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

Android Annotations proposal #1255

Merged
merged 7 commits into from
Jul 2, 2023
Merged

Conversation

fynngodau
Copy link
Collaborator

Fixes #1189 by planning a new and improved annotations API for Android.

Latest version for reading: https://github.com/fynngodau/maplibre-native/blob/annotations-proposal/design-proposals/2023-06-17-android-annotations.md

@RomanBapst
Copy link
Collaborator

@fynngodau Thanks for bringing up this proposal! I went through it today and liked the direction it takes a lot.
Actually it's quite funny because me and @JonasVautherin released ramani-maps today and many things you have in the proposal are quite similar to what we have done. E.g. managing the layering under the hood and just exposing a zIndex to the user is exactly what we have done. Also the fact that you propose to instantiate the annotation classes directly is pretty much what we do in compose. With compose we have the additional advantage that it's super declarative, e.g. you add a Circle directly to the UI code.

From what I understand the old workflow of adding annotations via the manager would still co-exists, right? That's important for us in our library as we currently make use of it.

What I like a lot are all the options that can be used to fine-tune the appearance of the elements, e.g. Padding, Transform, ...
And of course I am excited to see the as much "Kotlinification" as possible.

@fynngodau
Copy link
Collaborator Author

fynngodau commented Jun 22, 2023

@RomanBapst Your library sounds very exciting!

From what I understand the old workflow of adding annotations via the manager would still co-exists, right? That's important for us in our library as we currently make use of it.

Yes, this is what I had in mind. It would be necessary to create annotations using the changed API, but they could still be added to annotation managers as previously. As explained in the proposal, the two use main cases are

  • when you really can't afford the matching algorithm to take O(n log n) (and have additional knowledge about the desired z layers in advance)
  • when you want to call methods that are not available on the specific Annotation

@systemed
Copy link

As a general note, while porting cycle.travel's app from iOS to Android, I've been surprised how different the Android and iOS APIs are.

Obviously each platform has its own design patterns (e.g. iOS delegates, Android builders, etc.) so you wouldn't expect full convergence, but there's a lot that seems needlessly different between the two - annotations being just one example. It means a whole bunch of extra cognitive load when porting or developing in parallel. From looking through issues on the old Mapbox repos it looks like their two teams were working independently.

So perhaps an additional design goal would be appropriate: increased commonality with MapLibre iOS Native. That doesn't necessarily imply changes have to be made here in Androidland - it could equally mean changing the iOS API to match the new, better Android API. But I think it's worth considering as an aim.

@louwers
Copy link
Collaborator

louwers commented Jun 26, 2023

@systemed Very interesting point! Now that Swift is the language of choice for iOS development, I think it makes sense to offer a better Swift-first API in the future. Being a much more modern programming language with quite powerful features (in comparison to Objective-C), I think a lot is possible there. If you have specific ideas in mind or want to contribute, check out the general topic for iOS modernization #1248.

I gave you a follow in LinkedIn, let me know if you ever want to brainstorm about this.

@systemed
Copy link

Cheers!

Swift and Kotlin are broadly similar languages, each with a good async/await story and heavy use of blocks/closures. To my mind this gives the opportunity to remove some of the more idiomatic design patterns (Java-ish managers and builders, ObjC-ish delegates) and converge on a simpler, more direct API. So @fynngodau's suggestion of de-emphasising manager objects makes a lot of sense in this context.

Copy link
Collaborator

@louwers louwers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard my earlier comment, I think it is better to use maplibre-native since you are referring to the repo, not the project.

LGTM!

@louwers louwers merged commit 53d061d into maplibre:main Jul 2, 2023
@fynngodau fynngodau deleted the annotations-proposal branch July 3, 2023 07:25
stefankarschti pushed a commit to stefankarschti/maplibre-native that referenced this pull request Jul 5, 2023
@fynngodau fynngodau mentioned this pull request Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plan changes to Android annotation API
4 participants