Add -mapView:didFinishLoadingStyle: delegate methods #6636

friedbunny · 2016-10-07T19:53:34Z

Fixes #6412 by implementing -mapView:didFinishLoadingStyle: delegate methods, made possible at the core level by #6371.

Also adds MGLStyle.description and a private MGLStyle.URL property, which are useful when debugging:

<MGLStyle: 0x60800001e0a0; name = "Mapbox Dark", URL = "mapbox://styles/mapbox/dark-v9”>

/cc @1ec5

mention-bot · 2016-10-07T19:53:39Z

@friedbunny, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1ec5, @incanus and @boundsj to be potential reviewers.

incanus

We should add some internal commentary around the fact that the setStyleJSON() functionality in core performs a reset on the internal impl->styleURL, which means this SDK call can come back empty (or whatever the equivalent is in the bindings).

1ec5 · 2016-10-07T20:07:44Z

platform/darwin/src/MGLStyle.mm

@@ -34,6 +34,7 @@

 @interface MGLStyle()
 @property (nonatomic, weak) MGLMapView *mapView;
+@property (readonly, copy, nullable) NSURL *URL;


Fine for now, but it would be nice to make this public at some point, especially once we have #6386.

1ec5 · 2016-10-07T20:09:29Z

platform/ios/src/MGLMapViewDelegate.h

+ methods. Changes to sources or layers of the current style do not cause this
+ method to be called.
+
+ This method can be used to initiate runtime styling changes that will be


We don’t use the term “runtime styling” in API or API documentation. How about “modify the layout or appearance of the current style before the map view is displayed to the user”?

Maybe also emphasize that this is the earliest opportunity to make those changes.

Went with your suggested wording in 335fa5f.

1ec5 · 2016-10-07T20:15:08Z

We should add some internal commentary around the fact that the setStyleJSON() functionality in core performs a reset on the internal impl->styleURL, which means this SDK call can come back empty (or whatever the equivalent is in the bindings).

We don’t currently expose any way for the developer to use setStyleJSON(). Once we do in #6386, we may want to allow style URLs to be nullable everywhere. But this PR publicly deals in MGLStyle objects, not style URLs, and the private URL property is safely nullable.

incanus · 2016-10-07T20:17:07Z

We don’t currently expose any way for the developer to use setStyleJSON().

Right, but my point is to catch things later, when we need to expose this and it looks like a simple core wiring but actually breaks the functionality in this PR.

1ec5 · 2016-10-07T20:21:07Z

OK, I’ve added a note to #6386 (comment) to track the expected fallout. I see all the necessary nullable qualifiers in the code added by this PR, in any case.

friedbunny · 2016-10-08T00:34:14Z

How does this look to y’all now, @incanus and @1ec5?

1ec5 · 2016-10-08T00:45:28Z

Update the iOS and macOS changelogs

incanus · 2016-10-08T00:55:44Z

platform/darwin/src/MGLStyle.mm

@@ -92,6 +93,10 @@ - (NSString *)name {
    return @(self.mapView.mbglMap->getStyleName().c_str());
 }

+- (NSURL *)URL {


What I was thinking of in #6636 (review) is just a comment here, something like:

// note that if `mbglMap->setStyleJSON()` is ever wired up, this can return unexpected results

Unexpected in what sense? The private definition of this property says it’s nullable. Is there an expectation that URL will end up returning something in the future that isn’t a URL or null?

Ah, misunderstood. Carry on! 👍

friedbunny · 2016-10-08T01:28:18Z

How’s this for changelogging?

Added the -mapView:didFinishLoadingStyle: delegate method, which offers the earliest opportunity to modify the layout or appearance of the current style before the map view is displayed to the user. (#6636)

1ec5 · 2016-10-08T02:05:36Z

There's more than one delegate protocol in the library, so best to mention MGLMapViewDelegate by name. Otherwise, that's perfect.

Example: <MGLStyle: 0x60800001e0a0; name = "Mapbox Dark", URL = "mapbox://styles/mapbox/dark-v9"> - Adds an internal `URL` property.

friedbunny · 2016-10-10T18:19:18Z

Added changelog entries and squashed back down to the basic commits.

1ec5

💯🗳

friedbunny added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Oct 7, 2016

friedbunny added this to the ios-v3.4.0 milestone Oct 7, 2016

friedbunny self-assigned this Oct 7, 2016

incanus suggested changes Oct 7, 2016

View reviewed changes

1ec5 suggested changes Oct 7, 2016

View reviewed changes

1ec5 mentioned this pull request Oct 7, 2016

Initializers for MGLStyle #6386

Open

incanus approved these changes Oct 8, 2016

View reviewed changes

friedbunny added 2 commits October 10, 2016 14:15

[ios, macos] Add -mapView:didFinishLoadingStyle: delegate method

d2cf652

[ios, macos] Add description string for MGLStyle class

695358b

Example: <MGLStyle: 0x60800001e0a0; name = "Mapbox Dark", URL = "mapbox://styles/mapbox/dark-v9"> - Adds an internal `URL` property.

friedbunny force-pushed the fb-style-loading-delegate branch from 335fa5f to 695358b Compare October 10, 2016 18:17

1ec5 approved these changes Oct 11, 2016

View reviewed changes

friedbunny merged commit 5f969e3 into master Oct 11, 2016

friedbunny deleted the fb-style-loading-delegate branch October 11, 2016 17:07

1ec5 mentioned this pull request Oct 11, 2016

[ios, macos] possibility to set custom images to style #6637

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add -mapView:didFinishLoadingStyle: delegate methods #6636

Add -mapView:didFinishLoadingStyle: delegate methods #6636

friedbunny commented Oct 7, 2016

mention-bot commented Oct 7, 2016

incanus left a comment

1ec5 Oct 7, 2016

1ec5 Oct 7, 2016

1ec5 Oct 7, 2016

friedbunny Oct 7, 2016

1ec5 commented Oct 7, 2016

incanus commented Oct 7, 2016 •

edited

Loading

1ec5 commented Oct 7, 2016

friedbunny commented Oct 8, 2016

1ec5 commented Oct 8, 2016 •

edited by friedbunny

Loading

incanus Oct 8, 2016

friedbunny Oct 8, 2016 •

edited

Loading

incanus Oct 8, 2016

friedbunny commented Oct 8, 2016

1ec5 commented Oct 8, 2016

friedbunny commented Oct 10, 2016

1ec5 left a comment

Add -mapView:didFinishLoadingStyle: delegate methods #6636

Add -mapView:didFinishLoadingStyle: delegate methods #6636

Conversation

friedbunny commented Oct 7, 2016

mention-bot commented Oct 7, 2016

incanus left a comment

Choose a reason for hiding this comment

1ec5 Oct 7, 2016

Choose a reason for hiding this comment

1ec5 Oct 7, 2016

Choose a reason for hiding this comment

1ec5 Oct 7, 2016

Choose a reason for hiding this comment

friedbunny Oct 7, 2016

Choose a reason for hiding this comment

1ec5 commented Oct 7, 2016

incanus commented Oct 7, 2016 • edited Loading

1ec5 commented Oct 7, 2016

friedbunny commented Oct 8, 2016

1ec5 commented Oct 8, 2016 • edited by friedbunny Loading

incanus Oct 8, 2016

Choose a reason for hiding this comment

friedbunny Oct 8, 2016 • edited Loading

Choose a reason for hiding this comment

incanus Oct 8, 2016

Choose a reason for hiding this comment

friedbunny commented Oct 8, 2016

1ec5 commented Oct 8, 2016

friedbunny commented Oct 10, 2016

1ec5 left a comment

Choose a reason for hiding this comment

incanus commented Oct 7, 2016 •

edited

Loading

1ec5 commented Oct 8, 2016 •

edited by friedbunny

Loading

friedbunny Oct 8, 2016 •

edited

Loading