search

mapbox / mapbox-maps-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and Metal
https://www.mapbox.com/mapbox-mobile-sdk
Other
462 stars 151 forks source link

Attribution crashes because parentViewController is nil #1911

Open proggeramlug opened 1 year ago

proggeramlug commented 1 year ago

Environment

Observed behavior and steps to reproduce

The attribution buttons tries to open the dialog on the parentViewController here: https://github.com/mapbox/mapbox-maps-ios/blob/192437b31f46860d10900e227dfc36ef30aaa130/Sources/MapboxMaps/Foundation/MapView%2BAttribution.swift#L6

However, this can be nil (SwiftUI, Ionic/cordova etc.) and leads to a crash.

Expected behavior

To not crash 😇

Additional links and references

A quick workaround for me is:

 func viewControllerForPresenting(_ attributionDialogManager: AttributionDialogManager) -> UIViewController {
        return (UIApplication.shared.delegate?.window??.rootViewController!)!
    }

This could serve as a fallback in case the parentViewController is actually nil.

OdNairy commented 1 year ago

/cc @persidskiy

persidskiy commented 1 year ago

Hi @proggeramlug, thank you for highlighting this. I've created an internal issue to fix it.

Can you please specify in which cases you observe parentViewController to be nil. I've tried to put MapView into SwiftUI-only app on iOS 15-16 , but don't observe such crash, the parentViewController is UIHostingController in that case.

proggeramlug commented 1 year ago

@persidskiy Thanks!

In our case we are rendering a native extension on top of an Ionic/Cordova app. So the review is created and put on top of a WKWebview.

We do this by adding the newly created MapView (created via frame) to the stack where the WKWebview is, like this:

self.webView?.superview?.addSubview(self.mapView!)
persidskiy commented 1 year ago

@proggeramlug Can I ask you to create a minimal sample where we can debug and reproduce it?

This parentViewController should find a UIViewController eligible for presenting the attribution as long as MapView is presented inside a ViewController. https://github.com/mapbox/mapbox-maps-ios/blob/main/Sources/MapboxMaps/Foundation/Extensions/UIView.swift#L2-L11

Your case seem to be the edge case parentViewController doesn't cover, it would be nice to fix it.