Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Move MarkerView click handling back to View#setOnClickListener #8272

Merged
merged 1 commit into from
Mar 10, 2017

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Mar 3, 2017

Closes #8236, refs # 8159 #5388 #5639

This PR moves touch handling of MarkerViews back to View#setOnClickListener and fixes the issue with the original implementation as shown in #5388.

This change is motivated by 2 items:

…kListener. Workaround panning issue by dispatching touch events to the parent ViewGroup.
@tobrun tobrun added the Android Mapbox Maps SDK for Android label Mar 3, 2017
@tobrun tobrun added this to the android-v5.0.0 milestone Mar 3, 2017
@tobrun tobrun self-assigned this Mar 3, 2017
@tobrun tobrun requested a review from zugaldia March 3, 2017 16:44
@mention-bot
Copy link

@tobrun, thanks for your PR! By analyzing this pull request, we identified @1ec5 to be potential reviewers.

@@ -503,6 +503,21 @@ public void invalidateViewMarkersInVisibleRegion() {
}
}

adaptedView.setOnClickListener(new View.OnClickListener() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought, we should refactor invalidateViewMarkersInVisibleRegion method. The longer a procedure is, the more difficult it’s to understand. It's an easy refactor, 99% of the times extract a method. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this has been on my list todo for a while, this method should be split up into smaller methods 👍

if (marker instanceof MarkerView) {
handledDefaultClick = markerViewManager.onClickMarkerView((MarkerView) marker);
} else {
if (!(marker instanceof MarkerView)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should refactor AnnotationManager class (actually I'm working on it 😅 ). It has a lot of responsibilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, looking forward to that PR ;)

@tobrun tobrun merged commit 1693b38 into release-ios-v3.5.0-android-v5.0.0 Mar 10, 2017
@tobrun tobrun deleted the tvn-markerview-click branch March 10, 2017 00:59
tobrun added a commit that referenced this pull request Mar 30, 2017
…etOnClickListener. Workaround panning issue by dispatching touch events to the parent ViewGroup. (#8272)"

This reverts commit 1693b38.
tobrun added a commit that referenced this pull request Mar 30, 2017
* Revert "[android] - only dispatch events if not handled by MarkerView (#8447)"

This reverts commit 09d7685.

* Revert "[android] - move touch handling of MarkerViews back to View#setOnClickListener. Workaround panning issue by dispatching touch events to the parent ViewGroup. (#8272)"

This reverts commit 1693b38.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants