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

Allow updating location puck bottom offset #323

Closed
ericrwolfe opened this issue Jun 30, 2017 · 4 comments · Fixed by #402
Closed

Allow updating location puck bottom offset #323

ericrwolfe opened this issue Jun 30, 2017 · 4 comments · Fixed by #402
Milestone

Comments

@ericrwolfe
Copy link
Contributor

This is an upstream issue in mapbox-gl-native (@1ec5 I can't recall the ticket), but based on the release cycles of mapbox-gl-native, we should consider adding a workaround to the navigation SDK that allows developers to customize the bottom location puck offset while in course tracking.

This is a big hurdle for implementing custom navigation apps from scratch.

cc @bsudekum

@ericrwolfe ericrwolfe added this to the v0.5.0 milestone Jun 30, 2017
@1ec5
Copy link
Contributor

1ec5 commented Jul 1, 2017

The map SDK currently provides an option, userLocationVerticalAlignment, with three options: center (the default), top, and bottom (what the navigation SDK currently uses). It suited our needs at the time, but these days it feels like either the options result in poor placement or that there are too few options.

One fix would be to publicly expose the MGLMapView method that positions the user puck (and thus the point on the screen around which the camera is fixed), allowing a subclass to override the logic, with mapbox/mapbox-gl-native#5302 as a design precedent. This is tracked in mapbox/mapbox-gl-native#6867 (comment).

Before we go down that route, however, is there an opportunity to improve the existing “bottom” vertical alignment? As I understand it, the problem is that the puck positioning code doesn’t account for the map view’s content insets. Is it possible that mapbox/mapbox-gl-native#6566 exacerbated this problem?

/cc @boundsj

@1ec5 1ec5 modified the milestones: v0.5.0, v0.6.0 Jul 9, 2017
@ericrwolfe ericrwolfe modified the milestones: v0.6.0-2, v0.6.0-1 Jul 21, 2017
@1ec5
Copy link
Contributor

1ec5 commented Aug 16, 2017

Once #213 is fixed, it would also be possible to adjust the offset by building some padding into the user location annotation view’s frame.

@1ec5
Copy link
Contributor

1ec5 commented Oct 6, 2017

Once #213 is fixed, it would also be possible to adjust the offset by building some padding into the user location annotation view’s frame.

#498 made it possible to customize the user location annotation view. This made it possible to pad the view’s frame to force the view to appear at a different position on screen.

#402, which we hope to land next week after some testing, overhauls the user puck implementation, disabling the solution in #498 but making it much easier to implement the feature proposed here. There are a couple possible designs, either of which can be implemented in short order:

  • We could make NavigationMapView.userCourseViewCenter public so that the developer can override it with a custom CGPoint. This approach is simple and enables the developer to reposition the puck between location updates, but it assumes the puck’s location is based on fixed values, unlike for instance Apple Maps’s puck, which shifts to either side based on route data.
  • We could add a delegate method, NavigationViewControllerDelegate.navigationViewController(_:mapView:userPointForRouteProgress:) that polls the delegate for the position upon each location update, providing the route progress as context. This approach requires slightly more application code but enables a more dynamic puck.

@1ec5
Copy link
Contributor

1ec5 commented Oct 7, 2017

We could add a delegate method, NavigationViewControllerDelegate.navigationViewController(_:mapView:userPointForRouteProgress:) that polls the delegate for the position upon each location update, providing the route progress as context. This approach requires slightly more application code but enables a more dynamic puck.

I wound up implementing a delegate method along these lines as part of #402. It doesn’t have the route progress as an argument, but it’s pretty straightforward to get ahold of the route progress via navigationViewController.routeController.routeProgress.

@1ec5 1ec5 added this to the v0.9.0-1 milestone Oct 7, 2017
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 a pull request may close this issue.

2 participants