Maps: Hitbox feature default should be the feature at point instead of default arbitrary value 44

[bartolkaruza]

Describe the bug
The hitbox feature added to the VectorSource (and other sources) is always on, even when not set. The default value of 44 is arbitray compared to the feature size. If the hitbox is not set, it should default to triggering onPress on point instead of inside a Rect.

To Reproduce

1. Implement onPress on the source
2. Leave hitbox at default
3. Press right next to any feature

Expected behavior
onPress shouldn't be triggered

NSArray<id<MGLFeature>> *features = [mapView visibleFeaturesAtPoint:screenPoint inStyleLayersWithIdentifiers:[NSSet setWithArray:[touchableSource getLayerIDs]]];

Actual behavior
onPress is triggered outside of the feature bounds because of default Rect.

NSArray<id<MGLFeature>> *features = [mapView visibleFeaturesInRect:hitboxRect
                                                     inStyleLayersWithIdentifiers:[NSSet setWithArray:[touchableSource getLayerIDs]]
                                                     predicate:nil];

Versions (please complete the following information):

• Platfrom: iOS
• Device: any
• OS: any
• React Native Version: any

Additional context
Introduced by https://github.com/react-native-mapbox-gl/maps/commit/244830f2b8762d63e3632430f9813b6e5aa033d0

iOS:
https://github.com/react-native-mapbox-gl/maps/blob/04f789a665287578c929cbcbc8c6e651c710f379/ios/RCTMGL/RCTMGLMapViewManager.m#L325

Android:
https://github.com/react-native-mapbox-gl/maps/blob/04f789a665287578c929cbcbc8c6e651c710f379/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/mapview/RCTMGLMapView.java#L543

[kristfal]

I kinda with this being an issue.

44 pixels is based on human interface guidelines from Apple/Google. While hitting features precisely on a simulator is easy, doing it on device is much harder, and that is why we add a default buffer.

We’d rather have this work out of the box with sound defaults, that allow hitting LineStrings and narrow polys, than asking most people to override this value.

You can always override this if you don’t need this behavior. If this is not working for some reason, please let us know.

[bartolkaruza]

The hitbox is not a minimal size for the feature (which would relate to the interface guidelines, of making any touchable element at least 44px in size), but it relates to including any feature within 44px of the touch event. The problem with this is that features that are close to each other, but on different layers, make the underlying layer untouchable (because only the found feature at position 0 in the feature array is returned from native).

Below screenshot has 2 features outlined, top layer pin outlined in blue, and cluster layer below, outlined in green. The red dot is the touch event and the red outline the reach of that touch event as to inclusion of features.

In this case the pin receives the touch event and the cluster is almost impossible to reach by touch.

These are large features, that don't require enlarged buffers around them to make them easily touchable, but I imagine the exact same problem will occur when smaller features overlap.

Are you sure this is the default people are looking for? It took us a while to even figure out why that hitbox was causing the bug that the clusters weren't receiving touch events in many cases. I get that it makes sense in a map that has very few interactive features, but any map that has many interactive features will face the same problem we faced.

A viable workaround is setting hitbox to 1, which is the value we arrived at after debugging the native library code and after understanding the hitbox feature implementation (which we can maybe prevent for the next person facing this by explaining the hitbox default implication in the docs).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.