Add support for non-HTML attribution #1163
Conversation
| @@ -7,7 +7,10 @@ extension MapView: AttributionDialogManagerDelegate { | |||
| } | |||
|
|
|||
| func attributionDialogManager(_ attributionDialogManager: AttributionDialogManager, didTriggerActionFor attribution: Attribution) { | |||
| let url: URL = attribution.isFeedbackURL ? mapboxFeedbackURL() : attribution.url | |||
| guard let url: URL = attribution.isFeedbackURL ? mapboxFeedbackURL() : attribution.url else { | |||
There was a problem hiding this comment.
Do we need an explicit URL type in this line?
There was a problem hiding this comment.
No need, indeed. It went away after a refactoring related to #1163 (comment)
Thanks for calling me out on this 😅 With the changes to A bit trickier with |
| window.makeKeyAndVisible() | ||
| window.rootViewController = viewController |
There was a problem hiding this comment.
If this ordering results in a warning logged to the console, you may want to try setting rootViewController before calling makeKeyAndVisible
| override func setUp() { | ||
| super.setUp() | ||
| Bundle.mapboxMapsMetadata.version = "AttributionTests" | ||
| } | ||
|
|
||
| override func tearDown() { | ||
| super.tearDown() | ||
| Bundle.mapboxMapsMetadata.version = sdkVersion | ||
| } |
There was a problem hiding this comment.
I was surprised that mapboxMapsMetadata was settable. That looks like a bug in my opinion. Could you avoid setting it here and use string interpolation to create the expected value below?
Currently there is no attribution in case the provided source attribution is in form of plain text(
{attribution: "OpenStreetMap contributors, CC-BY-SA"}. This PR adds support for plain text attributions.Initially I went with the same approach as on Android mapbox/mapbox-maps-android#1087 to disable alert's actions without URLs(see v1 image above), but it does not look good on iOS due to platform differences. Since there can only be a single plain text attribution I decided put it into alert's message instead(v2).
Fixes #1164
Pull request checklist:
## mainheading near the top).