mapbox / mapbox-gl-native-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and OpenGL
https://www.mapbox.com/mobile/
Other
210 stars 122 forks source link

MGLMapView conformance to `UIGestureRecognizerDelegate` is not public #641

Closed groue closed 2 years ago

groue commented 2 years ago

Hello,

MGLMapView conforms to UIGestureRecognizerDelegate in a private way, and this conformance should be made public instead.

The conformance is declared in https://github.com/mapbox/mapbox-gl-native-ios/blob/ios-v6.4.1/platform/ios/src/MGLMapView.mm#L210. It should be declared in the .h header file, along with the implemented methods .

This private conformance creates a problem for applications that need to customise how MGLMapView deals with gesture recognizers. Because of the current private conformance, Swift won't let a subclass of MGLMapView override, for example, gestureRecognizerShouldBegin(_:). To do so, developers need to create a bridging header that exposes the private conformance, as in the example below:

// A bridging header that makes public what should have been public already
@import Mapbox;
@import UIKit;
@interface MGLMapView(Fix)<UIGestureRecognizerDelegate>
- (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer;
@end

I understand that making UIGestureRecognizerDelegate conformance public represents some work for you. But I also think this work is necessary.

Please consider that if MGLMapView had been written in Swift, the compiler would have required the UIGestureRecognizerDelegate conformance to be made public. In a way, this issue just exposes an ObjC atavism.

Mapbox SDK versions: 6.4.1 iOS/macOS versions: irrelevant Device/simulator models: irrelevant Xcode version: Version 13.1 (13A1030d)

groue commented 2 years ago

Hello @macdrevx, do you have any information about this topic?

ZiZasaurus commented 2 years ago

@groue thank you for raising this concern and contributing the request for this feature. We have recently launched v10, where this has been resolved. Can you please take a look at our GestureManager in v10 here?

groue commented 2 years ago

Thanks @ZiZasaurus. This looks great! I hope I can migrate to the new version of the framework soon. I have not yet evaluated the amount of work. Do you know if there is a migration guide?

ZiZasaurus commented 2 years ago

@groue you're most welcome! And absolutely, you can find our v10 migration guide here.

groue commented 2 years ago

Thank you very much, this is very helpful 🙇