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

[ios] Deactivate MGLMapView IBDesignable #13802

Closed
wants to merge 2 commits into from

Conversation

fabian-guerra
Copy link
Contributor

Fixes #13424

@fabian-guerra fabian-guerra added iOS Mapbox Maps SDK for iOS needs changelog Indicates PR needs a changelog entry prior to merging. labels Jan 25, 2019
@fabian-guerra fabian-guerra added this to the release-java milestone Jan 25, 2019
@fabian-guerra fabian-guerra self-assigned this Jan 25, 2019
@fabian-guerra fabian-guerra requested a review from a team January 25, 2019 21:46
@friedbunny
Copy link
Contributor

IBInspectable is separate from IBDesignable (see this short article from NSHipster) — correct me if I’m wrong, but I don’t believe we’ve had any issues with our inspectables.

Removed the MGLMapView's IBDesignable attribute. The map's view render attributes
uses GL, making changes through the inspectables were not reflected in the storyboard
and were causing IB crashes.
@fabian-guerra fabian-guerra force-pushed the fabian-deactivate-inspectables-13424 branch from c060996 to 9922073 Compare January 25, 2019 22:39
@fabian-guerra
Copy link
Contributor Author

@friedbunny you are right. I got confused, I changed accordingly.

@friedbunny friedbunny changed the title [ios] Deactivate MGLMapView IBInspectables. [ios] Deactivate MGLMapView IBDesignable Jan 25, 2019
@fabian-guerra fabian-guerra removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Jan 25, 2019
@@ -159,7 +159,7 @@ FOUNDATION_EXTERN MGL_EXPORT MGLExceptionName const MGLResourceNotFoundException
See the <a href="https://www.mapbox.com/ios-sdk/maps/examples/simple-map-view/">
Simple map view</a> example to learn how to initialize a basic `MGLMapView`.
*/
MGL_EXPORT IB_DESIGNABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing IB_DESIGNABLE will stop it from working, but evidently doesn’t remove any code from the shipped framework — are the opportunities here to conditionally remove code (and thus marginally reduce binary size)?

@@ -288,7 +288,6 @@ @implementation MGLMapView
/// True if a willChange notification has been issued for shape annotation layers and a didChange notification is pending.
BOOL _isChangingAnnotationLayers;
BOOL _isWaitingForRedundantReachableNotification;
BOOL _isTargetingInterfaceBuilder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this, but set it to NO (rather than calling NSProcessInfo.processInfo.mgl_isInterfaceBuilderDesignablesAgent) on an #ifdef (like you had previously).

Then you could keep the existing code that uses _isTargetingInterfaceBuilder.

Copy link
Contributor Author

@fabian-guerra fabian-guerra Jan 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason to remove this is that IBDesignable does provide a little value to MGLMapView, the only changes you can see in IB is the ornaments. Removing this code is a sign that we won't support it and we remove the unusable code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

We should still periodically check to see if the issue has been fixed on Xcode's side. Perhaps this is better done with a sample project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only changes you can see in IB is the ornaments

As illustrated in #929, the main reason we added a designable was because it’s very awkward to work with a custom view in IB: it ends up as an invisible rectangle. We also took advantage of the designable to remind the developer to set the access token; it’s gentler than asserting at launch.

We should still periodically check to see if the issue has been fixed on Xcode's side. Perhaps this is better done with a sample project.

Yes, especially now that Xcode 10.2 is in beta. Make sure to test both CocoaPods and Carthage. For what it’s worth, in Xcode 10.1 with the iOS navigation SDK installed via Carthage, the designable appears in a split-second, even in a launch storyboard.

@friedbunny friedbunny modified the milestones: release-java, release-k Jan 30, 2019
@friedbunny friedbunny added the needs changelog Indicates PR needs a changelog entry prior to merging. label Jan 30, 2019
@friedbunny friedbunny removed this from the release-kombucha milestone Feb 20, 2019
@julianrex julianrex added the upstream Blocked by an issue in a dependency label Mar 11, 2019
@fabian-guerra
Copy link
Contributor Author

For some reason github didn't let me update this branch. Following up on #14379

@fabian-guerra fabian-guerra deleted the fabian-deactivate-inspectables-13424 branch April 9, 2019 21:31
@friedbunny friedbunny removed needs changelog Indicates PR needs a changelog entry prior to merging. labels Jun 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS upstream Blocked by an issue in a dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove IB_DESIGNABLE from MGLMapView
4 participants