Callout-less annotation is deselected when user pans or zooms the map

[Ben305]

Platform: iOS
Mapbox SDK version: 3.4.1

I thought #4389 would be fixed in 3.4.0, but unfortunately the problem still exists. - (void)mapView:(MGLMapView *)mapView didDeselectAnnotation:(id<MGLAnnotation>)annotation is still called when the user pans or zooms the map. We don't use custom callout views but another info view that is placed below the map. This view is shown when an annotation is selected and hidden, when an annotation is deselected. Furtermore, we display tracks on the map when certain annotations are selected. If the user now zooms in or out to see the complete track, the annotation is deselected again and our logic removes the track from the map.

As a solution, I'd like to add a property called deselectOnRegionChange to MGLAnnotationView which defaults to true so that the current behavior is unchanged. If this property is set to false, the annotation is not deselected any longer when the user pans or zooms the map.

Steps to trigger behavior

1. Add annotation to map (also provide an annotation view)
2. Keep track of the annotations selection state (possible by implement setSelected:animated in the annotation view)
3. Select annotation
4. Change region by panning the map a little bit

Expected behavior

Annotation should still be selected

Actual behavior

Annotation is immediately deselected

[1ec5]

The logic in #4389 and #7646 assumes that a callout view is associated with the annotation, whether a custom callout view or the built-in MGLCompactCalloutView. If I understand your report correctly, you haven’t implemented -[MGLMapViewDelegate mapView:annotationCanShowCallout:] or are having it return NO. That inadvertently causes this line to deselect the annotation.

I think the intention there was to deselect if the callout view doesn’t respond to dismissesAutomatically, but to take the other code path if the callout view is absent. If that’s the case, I think a more targeted fix would be to check for dismissesAutomatically || (calloutView && ![calloutView respondsToSelector:@selector(dismissesAutomatically)]).

/cc @frederoni

[Ben305]

@1ec5 A callout view doesn't fit our needs. We have a bigger view at the bottom to show information for the selection annotation. Furthermore, the annotation view swaps its image when it is selected or deselected. There's no space left for showing a callout view.

So, I have not implemented -[MGLMapViewDelegate mapView:annotationCanShowCallout:]. That's why I've added another property which works without having a callout view.

[1ec5]

Makes sense. The solution proposed in #8021 (comment) would ensure that MGLMapView can track annotation selection consistently whether or not a callout is shown, making the additional property unnecessary.

[frederoni]

I think a more targeted fix would be to check for dismissesAutomatically || (calloutView && ![calloutView respondsToSelector:@selector(dismissesAutomatically)])

It would still get deselected when the annotation goes outside the viewport.
However, I don't see any reason to deselect annotations that are out of bounds.
@1ec5 Are you ok with removing these lines https://github.com/mapbox/mapbox-gl-native/blob/master/platform/ios/src/MGLMapView.mm#L4732-L4734 ?

[1ec5]

I’d prefer that we keep deselecting the selected annotation once it go out of view. If this isn’t suitable for a particular use case, it’s likely that MGLMapView’s concept of “selection” doesn’t match the concept of “selection” that the developer is trying to achieve. In that case, the developer should track the selected annotation some other way.

This raises another question: what if panning the map causes the selected annotation to slide underneath a custom info view like the one described in #8021 (comment)? (Perhaps it’s opaque with a margin on each side.) As a user, I might accidentally cause that to happen and get confused when the annotation appears to go missing. I think the correct solution might be to change the content insets when the view appears, but the existing deselection code doesn’t account for content insets.

[Ben305]

It would still get deselected when the annotation goes outside the viewport.
However, I don't see any reason to deselect annotations that are out of bounds.

I agree with @frederoni, I don't see any reason too when annotations that are (currently) not visible should be deselect. Think about a user that has selected an annotation which displays a bike trip on the map. When he zooms and pans to look the selected annotation may not be visible for some time and is deselected.

So, I would take the the solution proposed by @1ec5 and change the check to dismissesAutomatically || (calloutView && ![calloutView respondsToSelector:@selector(dismissesAutomatically)]) and also add a property deselectAnnotationWhenOutOfBounds to MGLMapView.

What do you think?

[frederoni]

I think the correct solution might be to change the content insets when the view appears, but the existing deselection code doesn’t account for content insets.

I see this as a different issue that most users will take for granted. I'd more likely pan the map causing the annotation to go outside the viewport by mistake than forgetting where it went when a static callout view appears.

Google Maps never deselects. MapKit does a partial deselection based on velocity but never fully deselect. MGLMapView always deselects. I don't know if we should aim for parity here because it's a different annotation and callout view API but it does feel odd to deselect automatically based on the viewport.

[Ben305]

@frederoni I've updated my PR accordingly, I think that would be a good compromise.

[1ec5]

Google Maps never deselects. MapKit does a partial deselection based on velocity but never fully deselect.

Thanks, I was unaware that Google Maps and MapKit behave that way. As it happens, the macOS implementation of MGLMapView also keeps annotations selected, so I’m not opposed to that behavior.