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

Automatically find view controller for layout guides #1357

Merged
merged 1 commit into from
Apr 28, 2015

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Apr 27, 2015

We inherited an annoying, easy-to-overlook setup step from the iOS SDK: MGLMapView’s viewControllerForLayoutGuides outlet must be hooked up to the managing view controller in order for the ornaments (logo, ℹ️, compass) to be laid out properly.

Based on the -[UIResponder nextResponder] documentation and the fixes for ornament layout in #1328, this PR causes MGLMapView to compute the managing view controller on its own by traversing the responder chain. (If there is a legitimate need to lay out the ornaments based on some view controller not in the responder chain, that would be a good opportunity to subclass MGLMapView; in that case, we’d need to re-expose -viewControllerForLayoutGuides publicly, but there would still be no need for a stored property.)

Once this PR is merged and contained in a tag, I’ll remove the step from the installation guide.

/cc @incanus @friedbunny

Find the managing view controller by traversing the responder chain.
@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS refactor labels Apr 27, 2015
@incanus
Copy link
Contributor

incanus commented Apr 27, 2015

I'm torn here, because we autodetect in the iOS SDK but it felt like voodoo. I thought it better to leave this to the developer in GL. Unfortunately there is not a good precedent for doing this one way or the other that I can find — MapKit has access to private API which makes this solid from whatever autodetection they are doing internally. Can we search for another third-party similarity and compare before changing current behavior?

@incanus
Copy link
Contributor

incanus commented Apr 27, 2015

See also the behavior in RMMapView in the SDK for viewControllerPresentingAttribution, which isn't necessarily the map view's parent view controller.

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 27, 2015

Traversing the responder chain is a pretty common approach, and it’s based entirely on officially documented behavior. (On OS X, the responder chain is how you get UI niceties as you input text in an NSTextField for instance.)

Either way, we’re violating MVC by having the view know about its controller. The (pedantically) correct way to do it would be to support a delegate method -viewControllerForAligningOrnamentsInMapView: and requiring the developer to hook up a delegate that returns the view controller. I’m opposed to that approach because implementing a delegate should not be required just to set up the most basic map view. People are going to forget to do it and will wind up with missing compasses and attribution as a result.

As it turns out, -[RMMapView viewControllerPresentingAttribution] is initially set to -viewController, which is implemented by traversing the responder chain.

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 28, 2015

For those following along at home, the private API @incanus is referring to is +[UIViewController viewControllerForView:] / -[UIView viewDelegate]. -[MKMapView _updateInsets] uses it via -[MKMapView _findLayoutGuideVC] to update edge insets that it then uses to manually position the MKAttributionLabel. But given the given the fact that we’re only using knowledge of the view controller to position views internal to MGLMapView, I think traversing the responder chain is perfectly non-hacky.

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 28, 2015

As for prior art: MASTOrmmaSharedDelegate and I’m sure plenty of others.

@incanus
Copy link
Contributor

incanus commented Apr 28, 2015

Ok, cool. :shipit: 😄

@friedbunny
Copy link
Contributor

This seems like a very sensible abstraction for end-developers, I don't mind if there's a small voodoo aspect. 🐐

1ec5 added a commit that referenced this pull request Apr 28, 2015
Automatically find view controller for layout guides
@1ec5 1ec5 merged commit 67fdfba into master Apr 28, 2015
@1ec5 1ec5 deleted the 1ec5-view-controller branch April 28, 2015 05:28
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.

3 participants