Prevent specific MGLAnnotation objects from being selected

[amphib]

using IOS 3.7 and I believe this issue is new to this release.

I have a MGLPolyline that shows a track that the user has taken. Along it I have some MGLPointAnnotations that are places the user marked along the way. I want the user to be able to tap the point annotation and see its callout. But the user sometimes has to tap on the point annotation several times before the callout will be shown. I monitored didSelectAnnotation and can see that the MGLPolyLine is 'stealing' the user taps and being selected as an annotation. Is there a way to tell MGLPolyline not to consume taps meant for point annotations?

[fabian-guerra]

Hi, @amphib. Thank you for using Mapbox. If MGLPolyline and MGLPointAnnotations are close the behavior is iterate over the ones that are closer as you tap the same area. We added support selection of shape and polyline annotations in #9984. Could you please post a test app or code snipet we could use to verify the issue?

[gmoretto]

I have the same problem: in didselect I expect to have a point annotation but I always find a MGLPolylineFeature and it crashes. No way to click only the annotation. Only in 3.7

[johnnewman]

I'm seeing this issue as well: polygons and polylines are both swallowing touches for point annotations. Here is some sample code to recreate the issue using Mapbox-iOS-SDK 3.7.6:

- (void)viewDidLoad {
    [super viewDidLoad];
    self.map = [[MGLMapView alloc] initWithFrame:self.view.bounds];
    self.map.delegate = self;
    [self.view addSubview:self.map];
    
    [self.map setCenterCoordinate:CLLocationCoordinate2DMake(35, -95) zoomLevel:8 animated:NO];
    [self addPolygon];
    [self addPolyline];
    [self addPoints];
}

- (void)addPolygon {
    CLLocationCoordinate2D *coordinates = malloc(sizeof(CLLocationCoordinate2D) * 5);
    coordinates[0] = CLLocationCoordinate2DMake(40, -90);
    coordinates[1] = CLLocationCoordinate2DMake(30, -90);
    coordinates[2] = CLLocationCoordinate2DMake(30, -100);
    coordinates[3] = CLLocationCoordinate2DMake(40, -100);
    coordinates[4] = CLLocationCoordinate2DMake(40, -90);
    [self.map addAnnotation:[MGLPolygon polygonWithCoordinates:coordinates count:5]];
    free(coordinates);
}

- (void)addPolyline {
    CLLocationCoordinate2D *coordinates = malloc(sizeof(CLLocationCoordinate2D) * 2);
    coordinates[0] = CLLocationCoordinate2DMake(35, -94);
    coordinates[1] = CLLocationCoordinate2DMake(35, -96);
    [self.map addAnnotation:[MGLPolyline polylineWithCoordinates:coordinates count:2]];
    free(coordinates);
}

- (void)addPoints {
    CLLocationCoordinate2D coord = CLLocationCoordinate2DMake(35, -95);
    for (int i = 0; i < 20; i++) {
        MGLPointAnnotation *annotation = [MGLPointAnnotation new];
        annotation.coordinate = coord;
        [self.map addAnnotation:annotation];
        coord.longitude += .01;
    }
}

#pragma mark - MGLMapView delegate

- (CGFloat)mapView:(MGLMapView *)mapView alphaForShapeAnnotation:(MGLShape *)annotation {
    return 0.25f;
}

- (void)mapView:(MGLMapView *)mapView didSelectAnnotation:(id<MGLAnnotation>)annotation {
    if ([annotation isKindOfClass:[MGLPolygon class]]) {
        NSLog(@"Polygon selected. << Swallowing a touch on a point annotation.");
        return;
    } else if ([annotation isKindOfClass:[MGLPolyline class]]) {
        NSLog(@"Polyline selected. << Swallowing a touch on a point annotation.");
        return;
    }
    NSLog(@"Selected a point annotation.");
}

• If I try tapping on any of those point annotations, the polys always take priority in didSelectAnnotation:.
• If I use the simulator and keep tapping without moving the cursor, I will eventually receive a point annotation.

[amphib]

Since I do not want MglPolyline participating in my callouts I ended up writing my own touch handler that searches my mglPointAnnotations for the closest and then displays my own 'callout'.

I think the proper solution is to modify the api so it supports enable/disable callouts on a per annotation basis. This would be similar to objective C's annotations.

[jmkiley]

We could possibly add an isEnabled flag to MGLAnnotation, then filter out any disabled annotations
somewhere in here:

mapbox-gl-native/platform/ios/src/MGLMapView.mm

Lines 4046 to 4055 in a721b74

	// Filter out any annotation whose image or view is unselectable or for which
	// hit testing fails.
	auto end = std::remove_if(nearbyAnnotations.begin(), nearbyAnnotations.end(), [&](const MGLAnnotationTag annotationTag) {
	id <MGLAnnotation> annotation = [self annotationWithTag:annotationTag];
	NSAssert(annotation, @"Unknown annotation found nearby tap");
	if ( ! annotation)
	{
	return true;
	}

[1ec5]

In the past, I think we’ve typically recommended tracking selection yourself using -mapView:didSelectAnnotation: and -mapView:didDeselectAnnotation:. It doesn’t require setting up a gesture recognizer, just responding to these two delegate methods and updating your own ivar.

Another consideration is that there’s already an -[MGLMapViewDelegate mapView:annotationCanShowCallout:] method for preventing an annotation from showing a callout view when tapped. By default, if this method is unimplemented, no annotations show callout views when tapped. If we add a -mapView:annotationRespondsToTapGesture: or somesuch, then the default value when unimplemented would have to be YES rather than NO. This inconsistency could be confusing.

We could possibly add an isEnabled flag to MGLAnnotation

There’s already an enabled property on MGLAnnotationImage for the same purpose. It’s sort of forgotten, and I’ve considered proposing that we deprecate it in favor of the custom selection tracking approach described above. I think it could be problematic to require classes to implement an enabled property in order to conform to MGLAnnotation, since such a variety of classes conform to that protocol (including classes that conform to MGLFeature).

[amphib]

Maybe I missed something, but I could not find an easy way to prevent my MglPolylines from consuming user taps and preventing a nearby point annotation from showing its callout. The user experience is terrible. They have to tap one or more times with no response from the UI before the point annotation finally shows its callout.

I have implemented many use cases for several clients that require some annotations to not consume taps (mostly in IOS and Google map objects but also other more obscure mapping APIs). This is a common use case so I think the API should easily support it. I don't have a preference as to how it is done, but I think it should be easy to do (for instance, as easy as setting a bool).

This is no longer holding me up. I have side stepped the API and implemented my own code for figuring out what was touched. But I really shouldn't have had to do that.

[jmkiley]

I think it could be problematic to require classes to implement an enabled property in order to conform to MGLAnnotation, since such a variety of classes conform to that protocol (including classes that conform to MGLFeature).

Good point! Would adding an optional enabled property to MGLShape be preferable? That seems like it would exclude MGLFeature. We can enable all subclasses of MGLShape by default, then allow developers to individually disable other annotations. I'm not sure if a delegate method would be the best approach in this case, for the inconsistency reasons you outlined.
Edited to add: Would still need to either exclude or document the limitations for the feature subclasses of MGLMultiPoint classes.

A drawback to checking for the type of annotation in -mapView:didSelectAnnotation: (which seems to have been implemented above) is that it can appear to the end user that the map is not responding. I’m not sure if there is a built-in way to move onto the next annotation, but it would be great for developers to have a way to prevent an annotation from being cycled through.

[1ec5]

Maybe I missed something, but I could not find an easy way to prevent my MglPolylines from consuming user taps and preventing a nearby point annotation from showing its callout. The user experience is terrible. They have to tap one or more times with no response from the UI before the point annotation finally shows its callout.

To have only point annotations show callouts, implement -mapView:annotationCanShowCallout: to return YES unless annotation is a kind of MGLPolyline or MGLPolygon. (Some of the map SDK’s examples suggest unconditionally returning YES; this is problematic if you intend to add noninteractive polyline or polygon annotations to the map.)

[amphib]

@1ec5 Minh, I tried implementing annotationCanShowCallout, it does not solve the problem. When this function returns false the annotation does not show its callout but it still consumes the user's tap. Therefore, the nearby annotation that I want to show the callout does not receive the tap and does not show its callout. So lets say I have a point annotation P that is situated on a polyline L. I disable L's callout using annotationCanShowCallout==false. The UX is that the user touches P and nothing happens because L has consumed the tap. Then he touches P again and see's P's callout (because mapbox can remember that L was recently touched and so for the next tap it skips L). In my use case the user often had to tap an annotation several times before the callout was shown (because other nearby annotations whose callouts were disabled were consuming the taps). Basically, what I need is a way to tell an annotation to ignore a user tap and pass it down the chain to the next annotation. This is a common thing to need and both Apple's and Google's IOS map objects provide this functionality. Anyway, I think your cohorts are already thinking about how to solve this issue.

[amphib]

I should mention that in my use case I am disabling callouts only for polylines. But a solution to this issue should allow for disabling any individual annotation regardless of type.

[1ec5]

When this function returns false the annotation does not show its callout but it still consumes the user's tap. … Basically, what I need is a way to tell an annotation to ignore a user tap and pass it down the chain to the next annotation. This is a common thing to need and both Apple's and Google's IOS map objects provide this functionality.

OK, thank you for clarifying the issue. In MapKit, MKAnnotationView.enabled determines whether tapping the point annotation view selects it. In this SDK, MGLAnnotationView.enabled already controls interactivity for view-backed point annotations, while MGLAnnotationImage.enabled controls interactivity thing for non-view-backed point annotations.

What’s missing is a way to opt shape annotations out of tap selection. That was admittedly a blind spot for me as I reviewed #9984. It isn’t a problem for MapKit, because shape annotations don’t respond to user interaction in MapKit. If MapKit were to make shape annotations respond to user interaction, there would likely be an enabled property on MKOverlayRenderer. Unfortunately, this SDK has no analogue to MKOverlayRenderer. Instead, we added various customization methods directly to MGLMapViewDelegate. #4392 proposes revisiting that decision. (The runtime styling API does have an analogue to MKOverlayRenderer in the form of MGLLineStyleLayer and MGLFillStyleLayer. #1734 (comment) proposed allowing MGLLineStyleLayer and MGLFillStyleLayer to be used for customizing shape annotations as well.)

The approach currently taken in #12352 has a few problems. It overlaps with the two existing enabled properties but it leaves custom MGLAnnotation classes unable to prevent selection. Meanwhile, feature querying and runtime styling both rely on MGLShape, but the proposed MGLShape.enabled property is meaningless in those contexts.

I’ve come around to the idea of adding a method to MGLMapViewDelegate, to be called whenever the user taps on a polyline or polygon annotation. I expressed some reservation in #10539 (comment) about its default value differing from that of -mapView:annotationCanShowCallout:. If that’s an issue, we could address it by instead naming the method -[MGLMapViewDelegate mapView:shapeAnnotationIsDisabled:] or -[MGLMapViewDelegate mapView:shapeAnnotationIgnoresUserInteraction:], preferably the latter.

/cc @jmkiley @julianrex @fabian-guerra

[fabian-guerra]

I’ve come around to the idea of adding a method to MGLMapViewDelegate, to be called whenever the user taps on a polyline or polygon annotation.

I agree with you, and also in this comment #12352 (review) I outlined that we have two "kind of" ways to disable an annotation. In one we "disable" the other checks if it can show a callout but both "eat" the touch which is not the ultimate goal according to what this issue outlines. Will it be possible to homogenize the behavior for the property enabled and clarify that it means no interactivity and it will not eat the touch, and leave -mapView:annotationCanShowCallout: as it is —which means it will check if the annotation can show a callout thus eating the touch—

/cc @1ec5 @jmkiley @julianrex

[jmkiley]

I’ve come around to the idea of adding a method to MGLMapViewDelegate, to be called whenever the user taps on a polyline or polygon annotation. I expressed some reservation in #10539 (comment) about its default value differing from that of -mapView:annotationCanShowCallout:. If that’s an issue, we could address it by instead naming the method -[MGLMapViewDelegate mapView:shapeAnnotationIsDisabled:] or -[MGLMapViewDelegate mapView:shapeAnnotationIgnoresUserInteraction:], preferably the latter.

My only concern with this is that some developers may need to allow individual shape annotations to be interactive. If we did go the delegate method route, we'd likely want to consider -[MGLMapViewDelegate mapView:polylineAnnotationIgnoresUserInteraction:].

The goal with the initial approach was to minimize the number of times a user touch is consumed, but that approach may be too granular. I'll explore the approach @fabian-guerra outlined above for now, and see if we can make the intention a little more clear.

Renaming enabled to ignoresUserInteraction could also be a good first step to make it clear, but, as @1ec5 pointed out, we're disabling interactions with the rendering of an annotation, not the annotation data itself.

[1ec5]

My only concern with this is that some developers may need to allow individual shape annotations to be interactive. If we did go the delegate method route, we'd likely want to consider -[MGLMapViewDelegate mapView:polylineAnnotationIgnoresUserInteraction:].

Yes, I think we’re on the same page. When the user taps on the map, the map view calls this method once for each nearby shape annotation – not only polylines but also polygons – and stops as soon as it finds one that accepts user input.

[julianrex]

I think it's worth considering that selection may not be via user interaction (though I see this as less of an issue for MGLPolyline) - but this sounds like it should just be a naming issue.

Could it be as simple as having an optional delegate method -[MGLMapViewDelegate mapView:canSelectAnnotation:] (matching existing can/did paradigm)?

We should also consider the performance cost of an additional delegate method check/call when there are many annotations.

If we wanted some kind of enabled property, this could be via a protocol that MGLPolyline, MGLAnnotationView and MGLAnnotationImage conform to (not MGLAnnotation). This might be the quickest and easiest approach, but I prefer a delegate method.

[1ec5]

I think it's worth considering that selection may not be via user interaction (though I see this as less of an issue for MGLPolyline) - but this sounds like it should just be a naming issue.

Do MGLAnnotationImage.enabled and MGLAnnotationView.enabled control whether an annotation can be selected at all, or only whether it can be selected by tapping?

We should also consider the performance cost of an additional delegate method check/call when there are many annotations.

That would be an argument for introducing a renderer-style object for overlays, analogous to MGLAnnotationImage, that could be home to an enabled property and a starting point for #1734 (comment). That would also enable us to deprecate MGLMapViewDelegate’s overlay-related properties. Effectively, instead of lots of redundant calls to delegate methods on tap, there would be fewer calls to a method like -mapView:rendererForOverlay: when the overlay comes into view. But that would be a larger effort; could a selection-specific delegate method serve as a stopgap?

[jmkiley]

Do MGLAnnotationImage.enabled and MGLAnnotationView.enabled control whether an annotation can be selected at all, or only whether it can be selected by tapping?

Annotations images and views with enabled set to NO can still be programmatically selected, as far as I know.

[julianrex]

Annotations images and views with enabled set to NO can still be programmatically selected, as far as I know.

Is this something that is worth considering changing?

[julianrex]

That would be an argument for introducing a renderer-style object for overlays, analogous to MGLAnnotationImage, that could be home to an enabled property and a starting point for #1734 (comment). That would also enable us to deprecate MGLMapViewDelegate’s overlay-related properties. Effectively, instead of lots of redundant calls to delegate methods on tap, there would be fewer calls to a method like -mapView:rendererForOverlay: when the overlay comes into view. But that would be a larger effort;

I like this approach - but is a big investment for us and for our users. I'd be in favour of this as part of a larger refactor.

could a selection-specific delegate method serve as a stopgap?

Definitely.

[1ec5]

Annotations images and views with enabled set to NO can still be programmatically selected, as far as I know.

Is this something that is worth considering changing?

These properties are named so similarly to MapKit properties that changing their semantics could create confusion. I think a change in their behavior would require a semver-major bump. In hindsight, we could’ve waited until a semver-major release for making shape annotations selectable in the first place; let’s avoid rushing a similar change out the door again. Instead, I think the delegate method approach is the right one, until we can refactor shape annotation options as described in #10539 (comment).

[fabian-guerra]

Fixed in #12352