Add support to show and track user location in the GeolocateControl #4479

andrewharvey · 2017-03-23T06:07:25Z

Fixes #4354
This PR builds upon #3781, please see that PR for background and design details.

This PR/branch uses a GL JS Marker (now that #4452 and #4452 are merged) to draw the user location dot rather than injecting the public style sheet. Rebased against master. Commits squashed.

From the original PR we did discuss some issues with the fitBoundsOptions option at #3781 (comment). There wasn't a resolution, my thoughts are to leave as is and change once the new Camera API lands (since there will be breaking changes with it already).

Breaking Changes

The option watchPosition has been replaced with trackUserLocation.
The camera operation has changed from jumpTo (not animated) to
fitBounds (animated). An effect of this is the map pitch is no longer
reset, although the bearing is still reset to 0.
The accuracy of the geolocation provided by the device is used to set
the view (previously it was fixed at zoom level 17). The maxZoom can
be controlled via the new fitBoundsOptions option.

New Features

New option showUserLocation to draw a "dot" as a Marker on the map at
the user's location.
An active lock and background state are introduced with
trackUserLocation. When in active lock the camera will update to follow
the user location, however if the camera is changed by the API or UI
then the control will enter the background state where it won't update
the camera to follow the user location.
New option fitBoundsOptions to control the camera operation.
New trackuserlocationstart and trackuserlocationend events.
New LngLat.toBounds method to extend a point location by a given
radius to a LngLatBounds object.

Launch Checklist

briefly describe the changes in this PR
write tests for all new functionality
document any changes to public APIs
N/A post benchmark scores
manually test the debug page

lucaswoj

This PR is shaping up very nicely! The implementation looks 👍 . I have a few thoughts about the public API for this feature.

lucaswoj · 2017-03-27T19:18:26Z

src/ui/control/geolocate_control.js

- * @param {Object} [options.positionOptions={enableHighAccuracy: false, timeout: 6000}] A [PositionOptions](https://developer.mozilla.org/en-US/docs/Web/API/PositionOptions) object.
- * @param {Object} [options.watchPosition=false] If `true` the map will reposition each time the position of the device changes and the control becomes a toggle.
+ * @param {Object} [options.positionOptions={enableHighAccuracy: false, timeout: 6000}] A Geolocation API [PositionOptions](https://developer.mozilla.org/en-US/docs/Web/API/PositionOptions) object.
+ * @param {Object} [options.fitBoundsOptions={maxZoom: 18}] A [`fitBounds`](#Map#fitBounds) options object to use when the map is panned and zoomed to the user's location. The default is to use a `maxZoom` of 18 to limit how far the map will zoom in for very accurate locations.


As we discussed in #3781 (comment), I'm interested in exploring some other names for this parameter that emphasize its relationship to "user location" and downplay its relationship to the soon-to-be-deprecated Map#fitBounds method (#2801).

From #3781 (comment).

Looking through the new camera api (at least based on the current API docs in #3583) the new API call would be:

map.fitBounds(bounds, cameraOptions, animationOptions)

It would make sense to have the GeolocateControl have and pass through options.cameraOptions and options.animationOptions. (and have cameraOptions support a maxZoom)

However at the moment fitBounds currently supports a few extra options which the AnimationOptions interface doesn't. linear, padding, maxZoom. So would you suggest I have an options.animationOptions and options.linear and options.padding and options.maxZoom? And then once the new camera api lands change the API to bring linear into AnimationOptions as type and padding,maxZoom into options.cameraOptions?

Either way there will be a public API change, but the new camera API already does that, so I think it's simpler to leave as is at the moment and then refactor for the new camera api.

I'm not sure the best way forward.

I think something userLocationViewOptions would work with what Lucas was suggesting (at the risk of extra verbosity 💭 )

😕 not sure exactly what that would look

Can we punt on this and keep the old jumpTo camera change instead of a fitBounds call? A user should be able to disable the default jumpTo and configure fitBounds independently if that's the behavior they want.

What's the reasoning behind keeping jumpTo? Is it related to the camera animation difference between fitBounds and jumpTo?

This was introduced in #3737 which I merged into this PR. The reason for fitBounds is to take into account the Geolocation API accuracy which I feel is a useful feature, animation of the camera is a separate issue but until the camera api refactor lands it is muddled into each api.

For example if the Geolocation accuracy is only city level, the map should only zoom to the size of that city.

lucaswoj · 2017-03-27T19:47:31Z

src/ui/control/geolocate_control.js

+/**
+ * Fired when the Geolocate Control changes to the background state, which happens when a user changes the camera during an active position lock. This only applies when trackUserLocation is true. In the background state, the dot on the map will update with location updates but the camera will not.
+ *
+ * @event background


What do you think about renaming these events to:

activeLock -> trackuserlocationstart

background -> trackuserlocationend

These names are consistent with existing event names (start and end suffixes) and more self-documenting (activeLock isn't a term of art AFIK). The all lowercase naming convention for events is unfortunate but consistent in our codebase so far.

I'm okay with that. Your reasoning makes sense.

andrewharvey · 2017-03-27T23:42:00Z

move test file from legacy to new test/unit directory
update tests based on using Marker
rename activeLock and background events
resolve fitBoundsOptions issue
remove ready event since Marker is synchronous (previous approach of using GL Layer was async)
fix broken tests

andrewharvey · 2017-04-06T22:46:50Z

From some testing, I've found that using

fitBoundsOptions: {
   linear: true
   duration: 0
}

seems to work better than the default easing. I feel it could be made the default but I'm not sure, anyone else have an opinion form a UX perspective?

andrewharvey · 2017-04-20T12:39:57Z

Is there anything else I can do to help move this PR forward?

mollymerp · 2017-04-21T23:31:15Z

@andrewharvey sorry for the delay here. @lucaswoj is hiking from Mexico to Canada right now so it looks like this slipped by other reviewers. I will plan to give it a proper review on Monday.

andrewharvey · 2017-05-09T10:38:58Z

@mollymerp rebased against master and updated to pass lint-css tests.

=== Breaking Changes === * The option watchPosition has been replaced with trackUserLocation. * The camera operation has changed from jumpTo (not animated) to fitBounds (animated). An effect of this is the map pitch is no longer reset, although the bearing is still reset to 0. * The accuracy of the geolocation provided by the device is used to set the view (previously it was fixed at zoom level 17). The maxZoom can be controlled via the new fitBoundsOptions option. === New Features === * New option showUserLocation to draw a "dot" as a Marker on the map at the user's location. * An active lock and background state are introduced with trackUserLocation. When in active lock the camera will update to follow the user location, however if the camera is changed by the API or UI then the control will enter the background state where it won't update the camera to follow the user location. * New option fitBoundsOptions to control the camera operation. * New trackuserlocationstart and trackuserlocationend events. * New LngLat.toBounds method to extend a point location by a given radius to a LngLatBounds object.

andrewharvey · 2017-05-19T23:55:15Z

rebased against master, squashed commits and updated commit message based on recent changes to event names.

pathmapper · 2017-05-23T19:02:24Z

@andrewharvey thanks a lot for your work on this!

It works fine - one thing I noticed while playing around with it:
With the latest release of Firefox for Android (53.0.2) there are all glyphs missing on the map (streets-v10). When using Chrome for Android or Firefox for Desktop the glyphs are there, also when using v0.37.0 of mapbox-gl.js with Firefox for Android (53.0.2).

I don't know whether this issue is related to your commit. It could be also that I was doing something wrong building mapbox-gl.js or it is a bug in Firefox or whatever - just wanted to let you know, maybe you are able to reproduce it.

andrewharvey · 2017-05-24T00:04:27Z

I don't know whether this issue is related to your commit. It could be also that I was doing something wrong building mapbox-gl.js or it is a bug in Firefox or whatever - just wanted to let you know, maybe you are able to reproduce it.

@pathmapper It sounds unrelated to this PR. Do you notice it in https://www.mapbox.com/mapbox-gl-js/examples ?

Not sure if it's #4607 but I would try upgrading to master and see if that fixes it.

pathmapper · 2017-05-24T07:15:43Z

@andrewharvey you're right, it is unrelated to this PR. Thanks for pointing me to the right direction.

Do you notice it in https://www.mapbox.com/mapbox-gl-js/examples ?

No

I would try upgrading to master and see if that fixes it.

It happens only with master, with latest release v0.37.0 everthing is fine.
Created an issue: #4747

mollymerp

Hi @andrewharvey – sorry for the delay on reviewing here. Added some comments/questions for you. Thanks again for this contribution!

Breaking Changes

The option watchPosition has been replaced with trackUserLocation.

The camera operation has changed from jumpTo (not animated) to fitBounds (animated). An effect of this is the map pitch is no longer reset, although the bearing is still reset to 0 (a limitation of fitBounds).

The accuracy of the geolocation provided by the device is used to set the view (previously it was fixed at zoom level 17). The maxZoom can be controled via the new fitBoundsOptions option.

I'm not sure any of these breaking changes are 100% necessary to add the functionality you're looking for here. Am I'm missing something? I think we can add the maxZoom option while remaining backwards compatible.

mollymerp · 2017-05-25T09:31:10Z

dist/mapbox-gl.css

@@ -303,6 +343,65 @@ a.mapboxgl-ctrl-logo {
 will-change: transform;
 }

+.mapboxgl-user-location-dot {
+ background-color: #1DA1F2;
+ width: 20px;


20px seems big, especially on mobile screens where this makes most sense to use – 12px is more reasonable I think. (same for height, and the :after sizes.

It feels a tad tiny at 12px, to the point where it almost disappears in the map at a glance, and compared to the size of other UI elements (like zoom buttons).

Noting at #3781 (comment) that the design came from @tristen .

Ah, I see. Maybe we could compromise at 16px? 😸

I think that works, at that size it's pretty much the same size as the icon in the control button which I think is good.

mollymerp · 2017-05-25T09:31:48Z

dist/mapbox-gl.css

+ height: 20px;
+ border-radius: 50%;
+ box-shadow: 0 0 2px rgba(0,0,0,0.25);
+ border: 3px solid #fff;


I think we should go down to 2px if changing the overall size of the marker.

mollymerp · 2017-05-25T09:32:22Z

dist/mapbox-gl.css

+}
+@-webkit-keyframes pulse {
+ 0% { -webkit-box-shadow: 0 0 0 0 rgba(29, 161, 242, 0.8); }
+ 70% { -webkit-box-shadow: 0 0 0 30px rgba(29, 161, 242, 0); }


should be sized down a bit as well

mollymerp · 2017-05-25T09:36:17Z

src/geo/lng_lat.js

+ * ll.toBounds(100).toArray(); // = [[-73.97501862141328, 40.77351016847229], [-73.97478137858673, 40.77368983152771]]
+ */
+ toBounds(radius: number) {
+ const latAccuracy = 360 * radius / 40075017,


Can you save 40075017 as a variable that explains its meaning (circ of the earth in meters) and/or add a comment w explanation

mollymerp · 2017-05-25T09:39:38Z

src/geo/lng_lat.js

+ const latAccuracy = 360 * radius / 40075017,
+ lngAccuracy = latAccuracy / Math.cos((Math.PI / 180) * this.lat);
+
+ const LngLatBounds = require('./lng_lat_bounds');


Is there a benefit of requiring this inside the function? 💭

Yeah explained at #3737 (comment) it was due to circular dependency issues which arose when it was outside the function 😉

👌 sorry I should have reviewed the comments in the old PR!

mollymerp · 2017-05-25T09:42:12Z

src/ui/control/geolocate_control.js


 const defaultGeoPositionOptions = { enableHighAccuracy: false, timeout: 6000 /* 6sec */ };
+const defaultFitBoundsOptions = { maxZoom: 18 };


this seems high. Z 15 would be a good default.

Actually I settled upon this after real world testing using it to navigate walking around, but I agree z15 would be a better default for desktop maps, so I'll change this.

The best value is going to depend a lot on the application/use case, device type (mobile or desktop), even if the user is moving and how fast (on a plane, or walking down the street would warrant different zooms) so being configurable is good.

mollymerp · 2017-05-25T09:49:03Z

src/ui/control/geolocate_control.js

- * @param {Object} [options.positionOptions={enableHighAccuracy: false, timeout: 6000}] A [PositionOptions](https://developer.mozilla.org/en-US/docs/Web/API/PositionOptions) object.
- * @param {Object} [options.watchPosition=false] If `true` the map will reposition each time the position of the device changes and the control becomes a toggle.
+ * @param {Object} [options.positionOptions={enableHighAccuracy: false, timeout: 6000}] A Geolocation API [PositionOptions](https://developer.mozilla.org/en-US/docs/Web/API/PositionOptions) object.
+ * @param {Object} [options.fitBoundsOptions={maxZoom: 18}] A [`fitBounds`](#Map#fitBounds) options object to use when the map is panned and zoomed to the user's location. The default is to use a `maxZoom` of 18 to limit how far the map will zoom in for very accurate locations.


Can we punt on this and keep the old jumpTo camera change instead of a fitBounds call? A user should be able to disable the default jumpTo and configure fitBounds independently if that's the behavior they want.

mollymerp · 2017-05-25T10:04:33Z

src/ui/control/geolocate_control.js

 * }));
 */
 class GeolocateControl extends Evented {

 constructor(options) {
 super();
 this.options = options || {};
+
+ // apply default for options.showUserLocation
+ this.options.showUserLocation = (this.options && 'showUserLocation' in this.options) ? this.options.showUserLocation : true;


could you use util.extend (see https://github.com/mapbox/mapbox-gl-js/blob/master/src/ui/map.js#L138 for example) to set the defaults?

I've done this, but also made a few other changes in other parts to simplify how default options are applied.

mollymerp · 2017-05-25T10:21:37Z

src/ui/control/geolocate_control.js

+
+ this._dotElement = DOM.create('div', 'mapboxgl-user-location-dot');
+
+ this._userLocationDotMarker = new Marker(this._dotElement, { offset: [-13, -13] });


can you add a comment explaining the offset constants?

Will do.

On that note, I'm in favour of #2900 which would eliminate offset here (and also make it much easier for people to override CSS styles to change the dot size for their app.

Yes, I agree that #2900 is something we should implement.

mollymerp · 2017-05-25T10:26:22Z

src/ui/control/geolocate_control.js

- * @param {Object} [options.watchPosition=false] If `true` the map will reposition each time the position of the device changes and the control becomes a toggle.
+ * @param {Object} [options.positionOptions={enableHighAccuracy: false, timeout: 6000}] A Geolocation API [PositionOptions](https://developer.mozilla.org/en-US/docs/Web/API/PositionOptions) object.
+ * @param {Object} [options.fitBoundsOptions={maxZoom: 18}] A [`fitBounds`](#Map#fitBounds) options object to use when the map is panned and zoomed to the user's location. The default is to use a `maxZoom` of 18 to limit how far the map will zoom in for very accurate locations.
+ * @param {Object} [options.trackUserLocation=false] If `true` the Geolocate Control becomes a toggle button and when active the map will receive updates to the user's location as it changes.


is trackUserLocation this just renaming watchPosition? I think we should avoid a breaking change if that's the case.

Yes, it was suggested over at #3781 (comment) to provide consistency with GL native.

I find showUserLocation and trackUserLocation go hand in hand. watchPosition fits in better with the HTML Geolocation API terminology, but I think trackUserLocation fits in better with what this option does in GL JS.

watchPositon was only recently introduced (well recently before the original PR for this) so I feel real world usage mightn't be that big.

andrewharvey · 2017-05-25T12:50:03Z

The option watchPosition has been replaced with trackUserLocation.

Comments inline above.

The camera operation has changed from jumpTo (not animated) to fitBounds (animated). An effect of this is the map pitch is no longer reset, although the bearing is still reset to 0 (a limitation of fitBounds).

The change from not animated to animated was necessary to get the extra options fitBounds provides, namely zoom to bounds (geolocation accuracy) and maxZoom. The fact that it's swapped from not animated to animated would only be fixed by the camera api changes to unbundle animation from camera parameters.

The accuracy of the geolocation provided by the device is used to set the view (previously it was fixed at zoom level 17). The maxZoom can be controled via the new fitBoundsOptions option.
I'm not sure any of these breaking changes are 100% necessary to add the functionality you're looking for here. Am I'm missing something? I think we can add the maxZoom option while remaining backwards compatible.

Because it now uses geolocation accuracy which affects maxZoom, it can't have both this functionality and always use a fixed zoom (as it did previously) hence the breaking change.

…I button

mollymerp · 2017-05-26T09:52:54Z

Thanks for the updates. I think I was just wondering if the added complexity of using geolocation accuracy at all is really necessary. I'm not sure it would be clear in this state to an end user that the bounds of the map view represent the accuracy of their geolocation, so I don't know if this is a value-add for the user.

andrewharvey · 2017-05-26T10:05:53Z

Thanks for the updates. I think I was just wondering if the added complexity of using geolocation accuracy at all is really necessary. I'm not sure it would be clear in this state to an end user that the bounds of the map view represent the accuracy of their geolocation, so I don't know if this is a value-add for the user.

It was more to prevent the map being on a street scale when the accuracy of the geocode is only city level, and vice versa allowing a street scale zoom when the accuracy of the geocode is within the meters.

mollymerp · 2017-05-29T10:02:22Z

ok – I would still like to remove the connection to fitBounds with this method before merging. Can you move maxZoom to its own property in options? It would also be great if you could document all the different _watchState values and their meanings in some way.

anandthakker

Echoing @mollymerp 's thanks for your work & patience on this @andrewharvey!

Adding a few notes inline.

anandthakker · 2017-05-30T16:55:09Z

dist/mapbox-gl.css

+ -ms-animation-iteration-count: infinite;
+ animation-iteration-count: infinite;
+}
+@-webkit-keyframes pulse {


I think this risks name collision w/ animations already defined on a site in which a GL JS is being embedded. Can we prefix the name? (Separately, we should probably also include animation names in our stylelint check)

anandthakker · 2017-05-30T17:20:45Z

src/ui/control/geolocate_control.js

@@ -40,27 +53,39 @@ function checkGeolocationSupport(callback) {
 * geolocation support is not available, the GeolocateControl will not
 * be visible.
 *
+ * The zoom level applied will depend on the accuracy of the geolocation provided by the device.
+ *
+ * The GeolocateControl has two modes. If `trackUserLocation` is `false` (default) the control acts as a button, which when pressed will set the map's camera to target the user location. If the user moves, the map won't update. This is most suited for the desktop. If `trackUserLocation` is `true` the control acts as a toggle button that when active the user's location is actively monitored for changes. In this mode there is a concept of an active lock and background. In active lock the map's camera will automatically update as the users's location changes until the user manually changes the camera (such as panning or zooming). When this happens the control is in background so the user's location dot still updates but the camera doesn't.


I think the explanation of trackUserLocation: true mode is still a bit hard to follow. Suggestion to clarify:

In this mode, the GeolocateControl has three states:

active - the map's camera automatically updates as the user's location changes, keeping the location dot in the center.

passive - the user's location dot automatically updates, but the map's camera does not.

disabled.

When a user first clicks the control, it becomes active. When the user manually pans or zooms the map, the geolocate control goes from active to passive mode. Clicking the control returns the control to the active state. (Is this true?)

anandthakker · 2017-05-30T17:25:58Z

src/ui/control/geolocate_control.js

+ default:
+ assert(false, `Unexpected watchState ${this._watchState}`);
+ }
+ this._geolocateButton.classList.remove('mapboxgl-ctrl-geolocate-waiting');


Nit: I think this line makes the equivalent lines in the cases above redundant.

anandthakker · 2017-05-30T17:37:41Z

src/ui/control/geolocate_control.js

+ if (this.options.trackUserLocation) {
+ this._map.on('movestart', (event) => {
+ if (!event.geolocateSource) {
+ if (this._watchState === 'ACTIVE_LOCK') {


Nit: change the nested ifs to if(!event.geolocateSource && this._watchState == 'ACTIVE_LOCK ')

anandthakker · 2017-05-30T17:47:05Z

src/ui/control/geolocate_control.js

+ }
+
+ if (this._watchState === 'ACTIVE_LOCK') {
+ this.fire('trackuserlocationstart');


I may be misunderstanding something here, but it looks to me like this will get fired each time the tracked location is updated, rather than only when we start tracking the location.

anandthakker · 2017-05-30T17:54:03Z

test/unit/ui/control/geolocate.test.js

+
+ let moveendCount = 0;
+ map.once('moveend', () => {
+ // moveend was being called a second time, this ensures that we don't run the tests a second time


The callback was being called a second time despite the use of map.once()?

yes that is what I observed. I couldn't work out why that was happening so I just worked around it with this count.

andrewharvey · 2017-06-01T01:12:13Z

ok – I would still like to remove the connection to fitBounds with this method before merging. Can you move maxZoom to its own property in options?

Are you saying you don't want the feature where it takes into account the geolocation accuracy at all? or you're happy with that but just don't want to use fitBounds?

I think it's important to let the user set all fitBounds options so they can tweak how the animation works, or remove it (which might even be the better default)?

It would also be great if you could document all the different _watchState values and their meanings in some way.

back in the original PR I had a state diagram:

however a few things have changed since then. so my question is does it make sense to improve the diagram and use that as the (internal) documentation or it needs to be documented in the js file directly?

mollymerp · 2017-06-01T13:37:12Z

Are you saying you don't want the feature where it takes into account the geolocation accuracy at all? or you're happy with that but just don't want to use fitBounds?

@andrewharvey the second one 😄. The only options exclusive to fitbounds (i.e. not represented in CameraOptions or AnimationOptions ) are padding and maxZoom, so I don't think that's too many to add to the root options object for the GeolocateControl

The diagram is awesome! I think it's great to have here on the PR for reference in the future, but I was thinking more just a short comment in-line explaining the difference between WAITING ACTIVE/ACTIVE LOCK/BACKGROUND and the difference between the two error types.

🙏 thanks for your patience on this! so close!!

andrewharvey · 2017-06-08T04:44:27Z

@andrewharvey the second one . The only options exclusive to fitbounds (i.e. not represented in CameraOptions or AnimationOptions ) are padding and maxZoom, so I don't think that's too many to add to the root options object for the GeolocateControl

@mollymerp Sorry I'm not convinced 😕. It feels messier than simply accepting a single fitBoundsOption parameter, which in the future would simply be replaced with cameraOptions when the new camera api lands. I'm not sure what you gain by having them as root options?

The fitBounds options at https://www.mapbox.com/mapbox-gl-js/api/#map#fitbounds I understand will accept all the options of AnimationOptions, CameraOptions + those described in fitBounds, so at the moment you could pass in:

fitBoundsOptions: {
   maxZoom: 19,
   linear: true,
   animate: false
}

Are you suggesting instead of this the GeolocateControl has options cameraMaxZoom, cameraLinear, cameraPadding, cameraOptions (CameraOptions|AnimationOptions), which are then all merged together anyway to pass to fitBounds?

The diagram is awesome! I think it's great to have here on the PR for reference in the future, but I was thinking more just a short comment in-line explaining the difference between WAITING ACTIVE/ACTIVE LOCK/BACKGROUND and the difference between the two error types.

Agreed, it's much easier to understand with the diagram, but it's outdated now as the states and class names have changed. I've tried to add in some internal docs on the states, plus I've removed the INIT state which wasn't doing anything (it was more important before when the user location dot was rendered as a Layer and we needed to wait for the map to load to add the Layer but now it's using Marker it's not doing anything).

michael8113 · 2017-06-14T02:07:30Z

Hi guys. Where may see the working example of geolocation work with the location point display? Thank you

andrewharvey · 2017-06-14T02:11:06Z

Hi guys. Where may see the working example of geolocation work with the location point display? Thank you

You should be able to do

yarn install
yarn run start-debug

and check out the main debug example http://localhost:9966/debug

michael8113 · 2017-06-14T03:02:03Z

Ок, If I just use external https://api.mapbox.com/mapbox-gl-js/v0.38.0/mapbox-gl.js, can not I just plug it in without having to install it?(

mollymerp · 2017-06-14T19:51:56Z

Thanks for your patience + contribution @andrewharvey! 🎉

mollymerp · 2017-06-15T00:03:48Z

@michael8113 this will go out in the next release v0.39.0

1updesign · 2017-07-18T13:20:55Z

hey @mollymerp - any idea when v0.39.0 is due? or perhaps the best way to get this feature working on a stable version in the meantime? thanks!

mollymerp · 2017-07-18T21:31:05Z

@1updesign We'll be doing a release this week!

nrako · 2017-08-09T09:29:47Z

@andrewharvey Thanks for this, this is great with a nice UX!

I'm just wondering if there's any reason for the control to not fire an event on any _watchState change?

As a simple example, I'd like to be able to know when the control is OFF or not (BACKGROUND, ACTIVE_LOCK). I feel having a statechange event would be more useful than trackuserlocationstart and trackuserlocationend

andrewharvey · 2017-08-10T13:45:36Z

I'm just wondering if there's any reason for the control to not fire an event on any _watchState change?
As a simple example, I'd like to be able to know when the control is OFF or not (BACKGROUND, ACTIVE_LOCK). I feel having a statechange event would be more useful than trackuserlocationstart and trackuserlocationend

@nrako I think you're right, a statechange event might be better. It would need to pass a value to indicate the new (and possible previous state) but I don't see that as an issue.

Do you think you need all the error states or just active, passive, disabled per https://www.mapbox.com/mapbox-gl-js/api/#geolocatecontrol.event:geolocate?

Could you open a new issue for this?

nrako · 2017-08-11T09:43:11Z

@andrewharvey – Thanks, I've created the issue #5136

I don't believe the previous state would need to be passed.
Maybe passing some states aren't useful and would just clutter the API. I leave this discussion for the issue => #5136

On the bonus side, I believe this could help refactor some similar logics which are spread in different handlers. I put some clue in the implementation part.

If the API and implementation design are agreed upon. I'll happily push a PR if my time allows it.

https://github.com/mapbox/mapbox-gl-js/releases/tag/v0.39.0 ⚠️ Breaking changes GeolocateControl breaking changes [DefinitelyTyped#4479](mapbox/mapbox-gl-js#4479) The option watchPosition has been replaced with trackUserLocation The camera operation has changed from jumpTo (not animated) to fitBounds (animated). An effect of this is the map pitch is no longer reset, although the bearing is still reset to 0. The accuracy of the geolocation provided by the device is used to set the view (previously it was fixed at zoom level 17). The maxZoom can be controlled via the new fitBoundsOptions option (defaults to 15).

⚠️ Breaking changes GeolocateControl breaking changes mapbox/mapbox-gl-js#4479 The option watchPosition has been replaced with trackUserLocation The camera operation has changed from jumpTo (not animated) to fitBounds (animated). An effect of this is the map pitch is no longer reset, although the bearing is still reset to 0. The accuracy of the geolocation provided by the device is used to set the view (previously it was fixed at zoom level 17). The maxZoom can be controlled via the new fitBoundsOptions option (defaults to 15). New option showUserLocation to draw a "dot" as a Marker on the map at the user's location

lucaswoj reviewed Mar 27, 2017

View reviewed changes

andrewharvey force-pushed the geolocate-track-show-user-location-html-marker2 branch from b24a7eb to 894b1a1 Compare March 28, 2017 23:30

andrewharvey force-pushed the geolocate-track-show-user-location-html-marker2 branch 2 times, most recently from d8fc8f1 to 208ed2d Compare April 13, 2017 13:20

andrewharvey force-pushed the geolocate-track-show-user-location-html-marker2 branch 2 times, most recently from c37fd55 to efbb106 Compare May 9, 2017 10:28

andrewharvey force-pushed the geolocate-track-show-user-location-html-marker2 branch from efbb106 to 299f9db Compare May 19, 2017 23:54

mollymerp suggested changes May 25, 2017

View reviewed changes

andrewharvey added 4 commits May 25, 2017 21:50

move magic number behind named constant

09961e0

reduce marker size

3975146

change default maxZoom to 15

fd77fc8

simplify how default options are applied

b88feb3

change marker size to 16 which makes it the same as the icon in the U…

811b6ba

…I button

anandthakker suggested changes May 30, 2017

View reviewed changes

andrewharvey added 5 commits June 1, 2017 10:46

prefix animation name

4199345

remove redundant classList remove

fe93af6

fix trackuserlocationstart to only fire when tracking starts

7065b6c

merge nested if

afb6f6c

improve trackUserLocation: true modes documentation

c11d54e

anandthakker approved these changes Jun 1, 2017

View reviewed changes

add watch state documentation

aae565c

mollymerp approved these changes Jun 14, 2017

View reviewed changes

andrewharvey merged commit 91ab217 into mapbox:master Jun 14, 2017

smmaurer mentioned this pull request Jun 29, 2017

Try updating GeolocateControl when Mapbox GL JS 0.39 is released smmaurer/geojson-web-viewer#1

Open

nrako mentioned this pull request Aug 11, 2017

Expose a "statechange" event on GeolocateControl #5136

Open

andrewharvey mentioned this pull request Jan 19, 2019

Support non-zero bearing when GeolocateControl is active #7783

Closed

andrewharvey mentioned this pull request May 16, 2024

userlocationfocus and userlocationlostfocus events added to geolocate_control maplibre/maplibre-gl-js#3847

Merged

4 tasks


		const defaultGeoPositionOptions = { enableHighAccuracy: false, timeout: 6000 /* 6sec */ };
		const defaultFitBoundsOptions = { maxZoom: 18 };


		this._dotElement = DOM.create('div', 'mapboxgl-user-location-dot');

		this._userLocationDotMarker = new Marker(this._dotElement, { offset: [-13, -13] });

Add support to show and track user location in the GeolocateControl #4479

Add support to show and track user location in the GeolocateControl #4479

Conversation

andrewharvey commented Mar 23, 2017 • edited Loading

Breaking Changes

New Features

Launch Checklist

lucaswoj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey May 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey commented Mar 27, 2017 • edited Loading

andrewharvey commented Apr 6, 2017

andrewharvey commented Apr 20, 2017

mollymerp commented Apr 21, 2017

andrewharvey commented May 9, 2017

andrewharvey commented May 19, 2017

pathmapper commented May 23, 2017

andrewharvey commented May 24, 2017 • edited Loading

pathmapper commented May 24, 2017 • edited Loading

mollymerp left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey May 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey May 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey commented May 25, 2017

mollymerp commented May 26, 2017

andrewharvey commented May 26, 2017

mollymerp commented May 29, 2017

anandthakker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewharvey commented Jun 1, 2017

mollymerp commented Jun 1, 2017

andrewharvey commented Jun 8, 2017

michael8113 commented Jun 14, 2017

andrewharvey commented Jun 14, 2017

michael8113 commented Jun 14, 2017

mollymerp commented Jun 14, 2017

mollymerp commented Jun 15, 2017

1updesign commented Jul 18, 2017

mollymerp commented Jul 18, 2017

nrako commented Aug 9, 2017

andrewharvey commented Mar 23, 2017 •

edited

Loading

andrewharvey May 25, 2017 •

edited

Loading

andrewharvey commented Mar 27, 2017 •

edited

Loading

andrewharvey commented May 24, 2017 •

edited

Loading

pathmapper commented May 24, 2017 •

edited

Loading

mollymerp left a comment •

edited

Loading

andrewharvey May 25, 2017 •

edited

Loading

andrewharvey May 25, 2017 •

edited

Loading

nrako commented Aug 11, 2017 •

edited

Loading