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

Support non-zero bearing when GeolocateControl is active #7783

Closed
andycalder opened this issue Jan 17, 2019 · 4 comments · Fixed by #7830
Closed

Support non-zero bearing when GeolocateControl is active #7783

andycalder opened this issue Jan 17, 2019 · 4 comments · Fixed by #7830

Comments

@andycalder
Copy link
Contributor

Motivation

When the GeolocateControl button is pressed, the map bearing always resets to zero, even if it was intentionally set otherwise. This behaviour is annoying when you want to locate the user on a map that has a natural 'up' direction. For example, I want to locate the user on a ski field that looks best with a certain orientation. Supporting non-zero bearing would also be useful if you want to align the map to a road or path when tracking user location.

Design

I propose we keep the current behaviour as default, but add a constructor option (called preserveBearing or something similar) that uses the current bearing when the camera transitions to the target. Usage:

map.addControl(new mapboxgl.GeolocateControl({
    positionOptions: {
        enableHighAccuracy: true
    },
    trackUserLocation: true,
    preserveBearing: true    // <-- new option
 }));

I'm happy to do a PR if the team thinks this is worth doing.

@andrewharvey
Copy link
Collaborator

The GeolocateControl used to reset the pitch and bearing, however with #4479 this changed to retain the pitch and only reset bearing. This was a side affect of changing the underlying camera operation from jumpTo to fitBounds.

I'd probably ask is it worth adding an option for this, or should it just be the default to retain pitch and bearing? If you ever want to reset the bearing on geolocate, then let the developer listen to the geolocate event and reset the bearing directly themselves. What does everyone think?

If we did introduce new options for this, I'd rather see it as more general camera options, which replaces the current fitBoundsOptions to the GeolocateControl https://docs.mapbox.com/mapbox-gl-js/api/#geolocatecontrol. Personally I was hopeful that the refactor planned in #2801 would provide such an option.

What would your implementation look like for a PR? Internally there is a cameraForBoxAndBearing which looks like you can pass a bbox + bearing and fit the map to that, retaining bearing.

@andycalder
Copy link
Contributor Author

andycalder commented Jan 20, 2019

Thanks @andrewharvey. I'm neutral on the default behaviour. I do see the value in resetting north when you're trying to orient yourself.

Yes, cameraForBoxAndBearing is already being used underneath, so all it takes is a simple change to the intermediate cameraForBounds and you can pass a bearing straight through from the GeolocateControl within fitBoundsOptions.

@andycalder
Copy link
Contributor Author

Actually, I think it would best to retain bearing as the default and avoid adding another option, just as you suggested @andrewharvey. Google Maps retains bearing on geolocate and it just seems like the most sensible behaviour to me. I'm interested to hear what others think about this.

I have a working prototype where GeolocateControl#_updateCamera queries the map bearing and passes it on as a property of fitBoundsOptions. Should I go ahead and open a PR so we can more easily discuss the implementation?

@andrewharvey
Copy link
Collaborator

I have a working prototype where GeolocateControl#_updateCamera queries the map bearing and passes it on as a property of fitBoundsOptions. Should I go ahead and open a PR so we can more easily discuss the implementation?

Awesome, that sounds good to me..

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

Successfully merging a pull request may close this issue.

3 participants