userlocationfocus and userlocationlostfocus events added to geolocate_control

[MeysamRT76]

Launch Checklist

Resolves #3846
Geolocate Control's OFF and BACKGROUND states both triggering the same trackuserlocationend event, and the WAITING_ACTIVE and ACTIVE_LOCK states both triggering the trackuserlocationstart event, two new events were introduced to separate theme:

1. userlocationfocus
2. userlocationlostfocus

This differentiation allows for a more precise control mechanism, enabling developers to accurately respond to specific geolocator state changes and implement more sophisticated and responsive geolocation functionalities.

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Document any changes to public APIs.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.70%. Comparing base (0a04d2a) to head (2299179).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3847      +/-   ##
==========================================
+ Coverage   86.55%   86.70%   +0.15%     
==========================================
  Files         242      242              
  Lines       33042    33044       +2     
  Branches     2027     2009      -18     
==========================================
+ Hits        28600    28652      +52     
+ Misses       3469     3423      -46     
+ Partials      973      969       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[HarelM]

Geolocate control is missing a lot of coverage in terms of tests and state management.
It would be great if you could add the relevant tests to cover all the flows related to state management.
When I tried to cover it with tests I had problems understanding all the state management logic.
Since you are deep into it, it works be great if you could spend some time adding the relevant tests.
Thanks!

[MeysamRT76]

I will try to write the related tests.

[HarelM]

This is a breaking change in terms of API, isn't it?
I think you could add the new events without removing the old event, document the old event is now deprecated and add a comment in v5 breaking change issue about it so we'll remove it in the future.

[MeysamRT76]

Do you want the old events to remain in their previous places and should I add new events beside them?

[HarelM]

Yes, I don't think there's another way to keep the old API and new at the same time, isn't it?

[MeysamRT76]

I made the code changes. How should I mark old events and functions as deprecated?

[HarelM]

The best thing I can think of is this one:
https://tsdoc.org/pages/tags/deprecated/

[MeysamRT76]

Please let me know if there are any changes needed.

[HarelM]

I'm not sure the added comments will be visible to users reading the geolocation API documentation here:
https://maplibre.org/maplibre-gl-js/docs/API/classes/GeolocateControl/
You can build the docs locally by running npm run generate-docs

Can you try and make sure the deprecation gets to this page somehow?

[andrewharvey]

When I tried to cover it with tests I had problems understanding all the state management logic.

I included a state diagram in my original implementation PR at mapbox/mapbox-gl-js#3781 which was eventually reworked into mapbox/mapbox-gl-js#4479.

I recently had to implement the Geolocate Control button external to the map with it's own design and ended up needing to access the private _watchState so 👍 to exposing more detailed events to allow custom buttons etc.

[HarelM]

@andrewharvey thanks, this is a great diagram!! I'll need to find a good way to incorporate this in the docs somehow so people can find this better, maybe using an image link to github user content or something, IDK...

In any case, this PR is missing the docs update and conflict resolution to be merged.

[HarelM]

Looking deeply into this code, I don't understand why the old events are considered deprecated when there's no alternative to the two places that are being fired now.
This PR still needs work and proper design, preferably a diagram with which events are fired when "before" and "after" this change.

[MeysamRT76]

State Transitions and Events Description Based On Diagram.

OFF -> ACTIVE WAITING
Event: trackUserLocationStart

ACTIVE WAITING -> OFF
ACTIVE ERROR -> OFF
BACKGROUND ERROR -> OFF
ACTIVE LOCK -> OFF
Event: trackUserLocationEnd

Explanation:
The above states and events are correct, indicating the start and end of user location tracking.

Proposed Changes:

BACKGROUND -> ACTIVE LOCK
Current Event: trackUserLocationStart

My Change: userLocationFocus (in addition to the existing event trackUserLocationStart)

ACTIVE LOCK -> BACKGROUND
Current Event: trackUserLocationEnd

My Change: userLocationLostFocus (in addition to the existing event trackUserLocationEnd)

Rationale:
When transitioning between ACTIVE LOCK and BACKGROUND, location tracking is not ending or starting; it is merely losing focus and refocusing on the user location.

Therefore, the old events trackUserLocationEnd and trackUserLocationStart are not deprecated. They are replaced in the two places mentioned above (Proposed Changes).

[HarelM]

So the events are not deprecated, but rather there are places where in the future, these event will not be fired.
In that case, I would recommend adding a comment to the following issue:

• Breaking changes MapLibre GL JS v5 #3834

That will remind us to remove the code that fire the events when they are not needed, when releasing version 5.

Please also fix the docs I changed as there's no real deprecation at this point.

[HarelM]

I've done the relevant change.
I'll merge this soon.

[MeysamRT76]

I was struggling with documentation changes due to personal issues, so I didn't have time to work on it. Thank you for completing this PR.

[andrewharvey]

When transitioning between ACTIVE LOCK and BACKGROUND, location tracking is not ending or starting; it is merely losing focus and refocusing on the user location.

Location tracking can mean different things, in this context it was likely talking about the map camera updating as the GPS location updates, not the "blue dot" tracking the live GPS location updates.

[HarelM]

I've added the diagram to the docs, hoping future reader will find it useful:
https://maplibre.org/maplibre-gl-js/docs/API/classes/GeolocateControl/#state-diagram

Thanks @andrewharvey!