Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Replace styleID property with styleURL__ inspectable #2632

Merged
merged 2 commits into from
Oct 22, 2015

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Oct 15, 2015

This PR replaces the styleID convenience property with an Interface Builder inspectable (normally inaccessible from code) that imitates a URL field. styleID is inconsistent with other GL-based APIs, and we never implemented a convenience initializer that took a styleID anyways, so it had limited usefulness as a separate property.

The new inspectable’s name is a horrible hack and is marked as such. I’ve filed <rdar://problem/23134321> against Interface Builder to request support for NSURL-typed inspectables.

I considered merely deprecating styleID but wound up cutting over in a manner similar to #1561. styleID is primarily used as an inspectable. Deprecating an inspectable leads to a horrible user experience, because the inspectable’s value continues to work even though it silently disappears from the Attributes inspector. (You’d have to know to go to the User Defined Runtime Attributes section of the Identity inspector.) If you ever tried to re-set the style, this time via the new “Style URL” inspectable, you might run into weirdness like #2626. This backwards-incompatible change requires bumping the version up to 3.0.0.

This PR also removes some APIs that have been marked unavailable for a few releases, since back before the iOS SDK left the 0.x series.

Fixes #2628.

/cc @jfirebaugh @incanus @bsudekum @friedbunny

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS refactor labels Oct 15, 2015
@1ec5 1ec5 added this to the ios-v2.2.0 milestone Oct 15, 2015
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 16, 2015

Before merging, I need to address invalid URLs by asserting.

@friedbunny
Copy link
Contributor

I'm on the record as being not un-in-favor of huge version numbers. Will give this a spin tomorrow!

1ec5 added 2 commits October 22, 2015 14:27
The new styleURL__ inspectable property is carefully named to masquerade as a URL property in Interface Builder, even though IB supports no such thing.

If the user enters an invalid URL into the “Style URL” inspectable, throw an error instead of silently setting the style to the default. This check is not required in -setStyleURL:, which takes a real NSURL.
@1ec5 1ec5 force-pushed the 1ec5-amazing-disappearing-underscore-inspectables-2628 branch from 96dcb92 to 4f50928 Compare October 22, 2015 21:28
@friedbunny
Copy link
Contributor

Looks good and works in practice. 👍

@1ec5 1ec5 merged commit 4f50928 into master Oct 22, 2015
@1ec5 1ec5 removed the in progress label Oct 22, 2015
@1ec5 1ec5 deleted the 1ec5-amazing-disappearing-underscore-inspectables-2628 branch October 22, 2015 21:56
@incanus
Copy link
Contributor

incanus commented Oct 23, 2015

Question: we are considering bumping to 3.x.x for this, correct? Is there any way we can not do that (just checking)?

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 23, 2015

As explained above, we could have a non-inspectable property with the old name, as a compatibility shim. However, that’s no way to deprecate a symbol: developers will never see any indication that styleID has gone away. If they even have a reason to look at MGLMapView’s inspectables, all they’ll see is that there’s a new “Style URL” field that’s blank. If they set that inspectable (since it was never their intention to use the default style), the result would be potential problems like #2626 (if the URL matches the hidden ID) or undefined behavior (if it doesn’t).

This is a unique case, and I think the set of inspectables we’ll have after landing this change will be pretty stable for awhile, modulo any Interface Builder enhancements to their inspectable support.

@tmcw
Copy link
Contributor

tmcw commented Nov 9, 2015

Is this change going to be reflected in the Android SDK?

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 9, 2015

The Android SDK has never had an independent styleID getter or setter: #1947 (comment).

1ec5 added a commit that referenced this pull request Nov 20, 2015
A hack atop the hack added in #2632. Pre-declare `styleURL__` with an attribute that prevents it from appearing in code completion suggestions, while leaving it available to Interface Builder, which is unable to parse attributes.
1ec5 added a commit that referenced this pull request Nov 20, 2015
A hack atop the hack added in #2632. Pre-declare `styleURL__` with an attribute that prevents it from appearing in code completion suggestions, while leaving it available to Interface Builder, which is unable to parse attributes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants