Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose mapView(:ViewForAnnotation:) #498

Merged
merged 4 commits into from
Aug 16, 2017
Merged

Expose mapView(:ViewForAnnotation:) #498

merged 4 commits into from
Aug 16, 2017

Conversation

bsudekum
Copy link
Contributor

This exposes the ability to provide a view for any annotation on the map. In the example, I show how to provide a view for the user puck.

/cc @1ec5 @frederoni @ericrwolfe

@@ -242,6 +242,34 @@ class ViewController: UIViewController, MGLMapViewDelegate {
present(navigationViewController, animated: true, completion: nil)
}

func navigationMapView(_ mapView: MGLMapView, viewFor annotation: MGLAnnotation) -> MGLAnnotationView? {
guard annotation is MGLUserLocation else { return nil }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, I'm never getting an annotation with type MGLUserLocation .

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this has to do with some of the user location overriding that the navigation SDK does…

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason is that MGLMapView.showsUserLocation is set during storyboard initialization, causing MGLMapView to call MGLMapViewDelegate.mapView(_:viewFor:). Unfortunately, navigationMapView isn’t set until much later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in b35dfe1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed mapbox/mapbox-gl-native#9781 to address the root cause.

@bsudekum bsudekum changed the title Expose mapView(:imageForAnnotation:) Expose mapView(:ViewForAnnotation:) Aug 15, 2017
@@ -242,6 +242,34 @@ class ViewController: UIViewController, MGLMapViewDelegate {
present(navigationViewController, animated: true, completion: nil)
}

func navigationMapView(_ mapView: MGLMapView, viewFor annotation: MGLAnnotation) -> MGLAnnotationView? {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be implemented in the extension below that conforms to NavigationViewControllerDelegate.

@@ -101,6 +101,8 @@ public protocol NavigationViewControllerDelegate {
If this method is unimplemented, the navigation map view will represent the destination annotation with the default marker.
*/
@objc optional func navigationMapView(_ mapView: MGLMapView, imageFor annotation: MGLAnnotation) -> MGLAnnotationImage?

@objc optional func navigationMapView(_ mapView: MGLMapView, viewFor annotation: MGLAnnotation) -> MGLAnnotationView?
Copy link
Contributor

Choose a reason for hiding this comment

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

This method needs documentation.

@@ -242,6 +242,34 @@ class ViewController: UIViewController, MGLMapViewDelegate {
present(navigationViewController, animated: true, completion: nil)
}

func navigationMapView(_ mapView: MGLMapView, viewFor annotation: MGLAnnotation) -> MGLAnnotationView? {
guard annotation is MGLUserLocation else { return nil }
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this has to do with some of the user location overriding that the navigation SDK does…

@1ec5
Copy link
Contributor

1ec5 commented Aug 15, 2017

This PR would fix #213.

showsUserLocation preinitializes the user location annotation view very early on, but the delegate has yet to be set by that time, let alone the navigation map delegate. When each of the delegate properties is set, force the map view to reconsider everything it does as part of showing the user location.
1ec5
1ec5 previously approved these changes Aug 16, 2017
*/
func refreshShowsUserLocation() {
showsUserLocation = false
showsUserLocation = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This will result in -[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:] getting called a lot more frequently than before when setting up the map view. From a cursory glance, I don’t think it’ll be a problem in practice.

@1ec5 1ec5 dismissed their stale review August 16, 2017 09:56

#498 (review) still needs to be addressed.

/**
Returns a view object to mark the given point annotation object on the map.

The user location annotation view can also be customized via this method. When annotation is an instance of `MGLUserLocation` (or equal to the map view’s userLocation property), return an instance of `MGLUserLocationAnnotationView` (or a subclass thereof).
Copy link
Contributor

Choose a reason for hiding this comment

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

or equal to the map view’s userLocation property

The developer doesn’t have direct access to MGLMapView.userLocation, do they?

@bsudekum
Copy link
Contributor Author

@1ec5 updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants