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] Upgrade annotation plugin to v0.9 #381

Merged
merged 1 commit into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ android {
}
dependencies {
implementation "com.mapbox.mapboxsdk:mapbox-android-sdk:9.2.0"
implementation "com.mapbox.mapboxsdk:mapbox-android-plugin-annotation-v9:0.8.0"
implementation "com.mapbox.mapboxsdk:mapbox-android-plugin-annotation-v9:0.9.0"
implementation "com.mapbox.mapboxsdk:mapbox-android-plugin-localization-v9:0.12.0"
}
compileOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,27 +783,31 @@ public void onCameraTrackingDismissed() {
}

@Override
public void onAnnotationClick(Annotation annotation) {
public boolean onAnnotationClick(Annotation annotation) {
if (annotation instanceof Symbol) {
final SymbolController symbolController = symbols.get(String.valueOf(annotation.getId()));
if (symbolController != null) {
symbolController.onTap();

Choose a reason for hiding this comment

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

Shouldn't onTap method return a boolean whether or not the event should be consume ? Like in android plugin ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Ph0tonic I also thought about that, the problem is that we can have 0 to n listeners in flutter, so we'd need to decide which one makes the final decision. I haven't been able to come up with a good solution for this so far.

Copy link

Choose a reason for hiding this comment

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

For android plugin, they decided to return as soon as a listener consumes the event. In other word, the first listener in the order of registration, which consume the event prevent all other listeners to be triggered.

I think that this makes sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking about it again that definitely makes sense.
It'll probably be a bit until I find time to make the change, though, so if you'd like to, feel free to implement it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can tackle this as tailwork, thanks for 👀 @Ph0tonic

Choose a reason for hiding this comment

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

I'm not sure I'll have time to tackle this but in any case you're welcome. 👍

return true;
}
}

if (annotation instanceof Line) {
final LineController lineController = lines.get(String.valueOf(annotation.getId()));
if (lineController != null) {
lineController.onTap();
return true;
}
}

if (annotation instanceof Circle) {
final CircleController circleController = circles.get(String.valueOf(annotation.getId()));
if (circleController != null) {
circleController.onTap();
return true;
}
}
return false;
}

@Override
Expand Down