Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API redesign for platform specific behavior #217

Open
sargunv opened this issue Jan 2, 2025 · 2 comments
Open

API redesign for platform specific behavior #217

sargunv opened this issue Jan 2, 2025 · 2 comments
Labels
enhancement New feature or change to an existing feature good first issue Well scoped and good for newcomers
Milestone

Comments

@sargunv
Copy link
Owner

sargunv commented Jan 2, 2025

Gesture settings, ornament settings, and onClick APIs are currently designed around native, with JS support done in a best-effort way that sometimes doesn't match the name:

  • native has click and long click, while JS has click, double click, and secondary click
  • native has only pointer gestures, while JS also has keyboard shortcuts
  • native allows disabling tilt, zoom, rotate, and pan gestures individually, while some of these are coupled together on JS

Maybe the above issues will be obsolete once we reimplement all gestures and controls in Compose. But even then, there will inevitably be some platform specific behavior, for example JS doesn't allow setting an FPS cap, and Android has a "texture mode" renderer with improved animation interop at the cost of performance. Currently we don't allow configuring this at all.

We need some API suited to platform specific settings. Is it a data class in common, with parameter names denoting which settings apply to which platforms? Is it an expect/actual class, requiring callers to instantiate it in platform specific source sets? Leaning the former for simplicity, but unsure.

cc @westnordost for API thoughts

@sargunv sargunv added the enhancement New feature or change to an existing feature label Jan 2, 2025
@sargunv sargunv added the good first issue Well scoped and good for newcomers label Jan 2, 2025
@sargunv sargunv added this to the v0.6.0 milestone Jan 2, 2025
sargunv added a commit that referenced this issue Jan 3, 2025
Disabled texture mode again in MapLibre's Android map view to improve rendering performance. This has caveats; see #15. A future PR will make this an opt-in option instead, see #217 

previously enabled in #214
@westnordost
Copy link
Collaborator

Hmm, don't really have thoughts on that. In other multiplatform implementations I have seen, the features that are only available on certain platforms are handled this way, IIRC:

In common: expect MyThing { val a: Int; }
In e.g. Android: actual class MyThing { actual val a:Int; val b: Int; } (i.e. Android additionally has b)
etc.

But I am sure that's already how your are doing it, anyway.

@sargunv
Copy link
Owner Author

sargunv commented Jan 3, 2025

That's my impression as well, but I'd prefer for common use cases like disabling ornaments, capping fps, etc to be doable without the user of the library needing to write their own expect/actual to instantiate the options object.

For that reason, keeping all the options in common with clear naming for what applies where is appealing. But then, maybe some platform gains a feature it previously lacked, and now the name is incorrect and requires a breaking change to correct.

So expect/actual is probably the way to go. Perhaps some commonMain presets can mitigate the need for users to get into expect/actual themselves for simple cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or change to an existing feature good first issue Well scoped and good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants