Mapbox-gl-native: MarkerClick event is only raised once

[bobmeyers5]

The concept of a tap/click input _event_ and the default _response_ to that event (select the marker and show info window) seem conflated in the current implementation. This causes problems when the input events are important, but the "selection" state is not.

The most critical problem is that a tap/click on a marker _always_ moves the marker into the "selected" state (regardless of the MarkerClick event handler's return value), and once in this state, additional tap/click input no longer raises the MarkerClick event. When selection state is not important/visible to the user, it is not obvious to them how to fix this. In addition, there is no easy way at the code level to prevent it from happening.

Some specifics contributing to the confusion:

• A tap/click on an unselected marker _always_ moves it to the "selected" state, even if mOnMarkerClickListener.onMarkerClick returns false, indicating the input event was "handled".
• A tap/click on a selected marker does _not_ raise the MarkerClick event, even though the associated input event clearly occurred.
• Calling selectMarker raises the MarkerClick event, even though the method name doesn't say anything about simulating a particular input event, and the event name suggests a particular input event occurred (which it did not).
• The default visual indicator of a marker's selection state is the presence or absence of an info window. However, this is not reliable because an info window may not be defined, it may be prevented from appearing due to the MarkerClick event returning false, and it may be shown and hidden independently by calling methods on the Marker object directly, none of which have any effect on the marker's selection state.

Unfortunately, because the current implementation is so confusing, one cannot be sure exactly how the code is currently being used, and therefore almost any fix could be considered a breaking change for some scenarios.

In the long term, though:

• MapView.onSingleTapConfirmed should raise the MarkerClick event any time the input event occurs, regardless of the marker's selection state.
• If the MarkerClick event handler returns false, the _entire_ default response to that input, including any selection state change, should be skipped.
• A new MarkerSelectionChanged event should be introduced to allow input events and selection state changes to be handled separately.
• MapView.selectMarker should raise the new MarkerSelectionChanged event, and should _not_ raise MarkerClick.

[tobrun]

@bmeyers43
Thank you for reaching out and giving us such detailed feedback. The subject here is something that we are improving/documenting in #553. I'm going to close this one down in favour of that one. I linked this issue so we can pickup this up.

[bobmeyers5]

That's a pretty massive work item ("One does not simply imitate Google's robust gesture system!") and it sounds like it's entirely about gesture _recognition_, not about the API for handling gestures after they have been recognized. My issue is just about fixing the API for handling simple tap/click events and changes to marker selection state. You may want to follow a similar pattern when exposing other gesture events, but I _really_ don't want to wait for you to reimplement your whole gesture recognition system before I can get a reliable marker click event!

Could you please consider fixing this separately (and sooner)?

--Bob

---

From: Tobrunmailto:[email protected]
Sent: ‎1/‎19/‎2016 4:39 AM
To: mapbox/mapbox-gl-nativemailto:[email protected]
Cc: bmeyers43mailto:[email protected]
Subject: Re: [mapbox-gl-native] MarkerClick event is only raised once (#3176)

@bmeyers43
Thank you for reaching out and giving us such detailed feedback. The subject here is something that we are improving/documenting in #553. I'm going to close this one down in favour of that one. I linked this issue so we can pickup this up.

---

Reply to this email directly or view it on GitHub:
https://github.com/mapbox/mapbox-gl-native/issues/3176#issuecomment-172839447

[tobrun]

@bmeyers43
You are right about that being about recognition vs selection state. Reopening and putting this issue on my priority list. Thank again for writing up this issue. This type of feedback is very valuable!

I'm seeing 2 actionable things here:

• The general one is that onMarkerClick callback invocation is not correctly positioned.
  Instead of in selectMarker it should be moved to onSingleTapConfirmed (now tapping a selected marker also results in a callback + solves is this method name/event appropriate problem).
• The second one is introduce a new callback system around MarkerSelectionChanged

I'm not :100: on the second one though, is this API you would use or ideally want? Google Maps SDK is currently not exposing this. I have no problem adding it but I would like to support this with a use-case that isn't solvable with a correctly working onMarkerClick integration.

[bobmeyers5]

The biggest issue currently is that no event is raised in response to a tap once the marker is selected. This issue could be resolved by either (or preferably both) of the following changes:

• Call onMarkerClick in onSingleTapConfirmed so it gets raised regardless of selection state.
• Don't call selectMarker if onMarkerClick returns false (indicating the event was handled).

In addition, I think you should consider adding an onMarkerSelectionChanged callback, but this is up to you. If someone is allowing the default behavior of click -> select, then they can probably use the onMarkerClick callback as a proxy in most cases. However it is common in general for controls that maintain selection state for a list of subitems to expose events around changes to the selection state. These callbacks would be invoked both in response to user input (e.g. a tap/click) and programmatic changes to selection state. If Google Maps doesn't expose anything like this and no one else is clamoring for it, I understand if it's not a high priority anytime soon. I myself don't need it at the moment either.

[tobrun]

@bmeyers43:
I tried implementing as mentioned above but ran into some other click related issue that has priority over this one #3643. That one needs to be addressed before I can safely fix this one. When that is resolved, I will pick up this again.

[tobrun]

picking this up again, while I'm at it, going to pick up #3720.

[tobrun]

I will not be able to land #3720 due to missing functionality.
I'm going to PR current fixes for this issue and comment out anything related to #3720.

[nealsanche]

I'd be completely happy with the current marker click functionality in 4.1.0-SNAPSHOT if this:

                mMapboxMap.setOnMarkerClickListener(new MapboxMap.OnMarkerClickListener() {
                    @Override
                    public boolean onMarkerClick(@NonNull Marker marker) {
                        Toast.makeText(MainActivity.this, "Marker tapped: " + marker.getTitle(), Toast.LENGTH_LONG).show();
                        return true;
                    }
                });

Would pop the toast every time. Notice I returned 'true' there. That's the cue to say that I've handled this event myself and I don't want default behaviour. Thing is, the default behaviour is still happening (though not showing the info window) and so the second tap doesn't pop a toast, only every second tap pops a toast.

[SilkSama]

Ugly workaround but it work for me waiting a better solution

@Override
public boolean onMarkerClick(@NonNull Marker marker) {
...
new Handler().postDelayed(() -> mMapboxMap.deselectMarker(marker), 100);
return true;
}

[TostF]

@SilkSama hack is working.
Problem here is that the OnMarkerClickListener does not represent the task that it is doing.
At the moment OnMarkerClickListener should be called OnMarkerSelectedListener.

[jemshit]

@bobmeyers5 That is very nice description of problem 👍

[tobrun]

I focussed on resolving the business logic issues as set out in the OP:

• [x] MapView.onSingleTapConfirmed should raise the MarkerClick event any time the input event occurs, regardless of the marker's selection state.

• [x] If the MarkerClick event handler returns false true, the entire default response to that input, including any selection state change, should be skipped.

Related to the other items mentioned in the OP as OnMarkerSelectionChangeListener. I'm thinking of PR'ing current progress and handling that as a separate feature request.

[FreedApe770]

@SilkSama Why do I need to use the "Handler().postDelayed(() -> mMapboxMap.deselectMarker(marker), 100);" rather than the “mMapboxMap.deselectMarker(marker)”？ I know the latter is no effect, but I would like to know the reason.
Thank you very much!

[tobrun]

@jwen1989 the workaround is no longer applicable, you should be able to get it your use-case working without it. Feel free to add more information or create a separate ticket.

[FreedApe770]

@tobrun Thank you for your advice。