Remove need for developers to implement NavigationMapView.updateCourseTracking(location:animated:)

[bsudekum]

Closes: #1539

It's not very obvious developers need to update the user puck if they implement their own NavigationMapView and this view can easily handle update the puck when progress notifications occur.

This PR makes NavigationMapView.updateCourseTracking(location:animated:) private turns on the location manager sooner. This way, when NavigationMapView is inited, we already have a location and the view will be properly centered.

/cc @1ec5 @frederoni

[1ec5]

This method is already risky and could become moreso if we rely on notifications to drive course updates as opposed to manual calls to updateCourseTracking(location:animated:). NavigationMapView apparently listens for notifications coming from any RouteController, so every map view in the application will automatically respond to every route controller in the application. There’s a high potential for crosstalk. We’d need to pass in a specific route controller as the object here to avoid that problem:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Line 274 in ea468da

NotificationCenter.default.addObserver(self, selector: #selector(progressDidChange(_:)), name: .routeControllerProgressDidChange, object: nil)

[bsudekum]

@1ec5 this may require changing the initializer on NavigationMapview. We currently have no RouterController props to grab.

[bsudekum]

I guess making this an optional property is another option. If you only want to update a single map view, set this prop.

[1ec5]

When we implemented #402, the idea was that moving the course view was the responsibility of the view controller that contains the map view, not the map view itself, since the view controller knows about its route controller for sure. I think we could’ve made that more clear by making the course view a sibling of the map view instead of one of its views. Relying on a global notification is a poor way to make the map view aware of the view controller’s route controller. Changing the initializer is also problematic because map views are often created using the init(frame:) or init(coder:) initializer.

If you only want to update a single map view, set this prop.

I don’t think it should be something that you only use in some cases. If we make the route controller a property, then the view controller should always have to set this property on the map view in order to keep the course view in sync. Otherwise, how would the map view handle the route controller swapping that happens around tunnels?

[bsudekum]

How would someone add a NavigationMapView to a storyboard if they have not yet created a route controller to pass through?

[1ec5]

Should we leave this method public? Is there still a use case for manually updating the course view independently of a route controller?

[bsudekum]

🤔maybe. I think it might be wise to wrap these types of functions in a more specific helper function like recenterMap:

mapbox-navigation-ios/MapboxNavigation/NavigationMapView.swift

Lines 1064 to 1067 in 0b69c32

	@objc public func recenterMap() {
	tracksUserCourse = true
	enableFrameByFrameCourseViewTracking(for: 3)
	}

[bsudekum]

I'd be open to making this an assertion. Fail fast.

[1ec5]

tracksUserCourse is basically redundant to routeController now. We can move all the logic below into routeController’s setter, conditioned on whether routeController is non-nil.

[1ec5]

And we can rename routeController to something that makes its purpose clear, like userCourseController (which would suggest a protocol by that name based on RouteController’s interface).

[1ec5]

What about suspending notifications on the previous route controller?

untested changes

[bsudekum]

@1ec5 I'm not sure the road we're headed down is any better, in fact it could be worse. I'm going to close this out and move the good parts found in this pr to #1547.