Skip to content

Add MarkerView plugin - #652

Merged
tobrun merged 1 commit into
masterfrom
tvn-markerview-plugin
Sep 7, 2018
Merged

tobrun merged 1 commit into
masterfrom
tvn-markerview-plugin

Conversation

@tobrun

@tobrun tobrun commented Sep 6, 2018

Copy link
Copy Markdown
Member

Upstream in the Mapbox Maps SDK for Android, we have deprecated MarkerView for almost a year. While this API can unblock certain use-cases, it isn't ideal and has no place in the main SDK. We plan to remove support of this API in the next major version (v7.0.0). To not block users still using this deprecated API, we are moving this feature inside a plugin instead.

ezgif com-video-to-gif 39

@tobrun tobrun self-assigned this Sep 6, 2018
@tobrun
tobrun force-pushed the tvn-markerview-plugin branch from 20b35b4 to 40a3008 Compare September 6, 2018 06:10
this.mapboxMap = mapboxMap;
}

public void onDestroy() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This call is missing in the example. This method should probably be limited to the main thread as well.


@UiThread
public void addMarker(@NonNull MarkerView markerView) {
if (markers.contains(markerView)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should check here for MapView#isDestroyed, not to hook the listener into a destroyed map.

@tobrun
tobrun force-pushed the tvn-markerview-plugin branch from f137c8c to 98d3e7b Compare September 6, 2018 09:44

@LukasPaczos LukasPaczos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!

@tobrun
tobrun merged commit f150ee3 into master Sep 7, 2018
@tobrun
tobrun deleted the tvn-markerview-plugin branch September 7, 2018 11:41
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.

2 participants