maplibre / maplibre-native

MapLibre Native - Interactive vector tile maps for iOS, Android and other platforms.
https://maplibre.org
BSD 2-Clause "Simplified" License
1k stars 295 forks source link

Plan changes to Android annotation API #1189

Closed fynngodau closed 1 year ago

fynngodau commented 1 year ago

When starting work on #1154, I noticed it can't be implemented as easily as we were hoping, for the following reasons:

Due to these considerations, we should make a plan for how we want the API to look like in the future. Some points that I think are important for this, regarding what aspects are nice about each of the APIs:

Goals of this issue:

JonasVautherin commented 1 year ago

I was asked my opinion on the two PRs above, so here it is :blush:.

I don't exactly know the implementation details, but this does not seem like a super quick task (this issue is 2 weeks old, and there are discussions about it from at least a month ago). Since the plugins seem to have been unmaintained for a while (last released artifact is from 2021, CI is failing, ...), I guess it makes sense to give them some attention :+1:.

Now personally, I have a couple concerns, which are:

I guess my point is that as long as those changes don't prevent us from using MapLibre and we somehow see a way to get our fixes upstream, then that is fine for us :innocent:.

louwers commented 1 year ago

@JonasVautherin MapLibre Native has a paid maintainer (me!) so I would not worry about not being able to upstream changes. Feel free to ping me on Slack at any time, I'll likely respond pretty quickly during office hours in Europe.

The reason why the plugins repo is stale is because bringing it to a good state again would require a lot of duplicate work that has been done for the main repo. As an example, we now have pretty good CI for this repo, including for example even running the UI tests for Android on AWS Device Farm.

If the plugins are actually re-designed/rewritten, then my concern is that we may lose features that we currently need

We will make sure no functionality is lost. This issue just exists to make a design proposal, you will be able to comment on the design when it is proposed at a later stage.

That said, would you want to help to make an official release of the plugins package?

fynngodau commented 1 year ago

@JonasVautherin I think the question this issue raises is: are there any specific design choices that have bothered you when working with the API for annotations in the past, something that could be made more user-friendly by designing it differently? Then we can consider it when writing the proposal in the first place.

JonasVautherin commented 1 year ago

I think the question this issue raises is: are there any specific design choices that have bothered you when working with the API for annotations in the past, something that could be made more user-friendly by designing it differently?

I think @RomanBapst can comment better on this, but to me it seems like it was pretty fine.

That said, would you want to help to make an official release of the plugins package?

Honestly I did not manage to build https://github.com/maplibre/maplibre-plugins-android, so instead I made the plugin we modify standalone (https://github.com/ramani-maps/maplibre-plugins-android/tree/main/plugin-annotation). So we can just go there and run ./gradlew build.

I could change it such that I can ./gradlew publish and it would push it to your Maven repo (I am used to Sonatype, not sure what you use). Either from your repo (if my PRs get in :innocent:) or directly from our fork, if you want.

Would that help? Or is it not what you were asking?

RomanBapst commented 1 year ago

@fynngodau

I think the question this issue raises is: are there any specific design choices that have bothered you when working with the API for annotations in the past, something that could be made more user-friendly by designing it differently? Then we can consider it when writing the proposal in the first place.

I have been working with the newer API (e.g. CircleManager) in the last couple of weeks and was quite happy with it. There are some really weird things like Text not being loaded on the map when you don't have an internet connection (I live somewhere in Tasania in the bush and that can happen :-p). But mostly I found that it gives you great flexibility (after some small mods in the annotation repo) to be creative about what you draw on the map. I also really like that fact that all annotations types expose drag events which is really really important.

I would be really careful to reduce this flexibility, let me try to explain with an example. In the past I have been using google-maps-compose for an app where I wanted to create an interactive polygon where the user should have been able to drag the vertices of the polygon around. The lib offered a Marker element which supported dragging but only after a long press. Additionally, the Marker internally implemented behavior which would make the icon jump out from below your finger to make it easier to place it. That may make sense for a standard marker on the map but not for that use case. I did not find a way around this and had to abandon the lib for that project. I think you can easily create convenience elements like Markers but that should not come at the cost of flexibility. I think the two things can easily co-exist.

fynngodau commented 1 year ago

@RomanBapst @JonasVautherin Thanks for your input.

@louwers Feel free to assign the issue to me, so I can start working on it.

fynngodau commented 1 year ago

@RomanBapst @JonasVautherin My proposal is here: https://github.com/maplibre/maplibre-native/pull/1255

Feel free to comment in that PR :slightly_smiling_face:

louwers commented 1 year ago

This bounty can be paid out.

fynngodau commented 1 year ago

Invoice: https://opencollective.com/maplibre/expenses/147683