Skip to content

Release geolocation camera lock on zoom#7909

Open
timsluis wants to merge 1 commit into
maplibre:mainfrom
timsluis:geolocate-unlock-on-zoom
Open

Release geolocation camera lock on zoom#7909
timsluis wants to merge 1 commit into
maplibre:mainfrom
timsluis:geolocate-unlock-on-zoom

Conversation

@timsluis

Copy link
Copy Markdown
Contributor

Fixes #7908.

When GeolocateControl was configured with trackUserLocation: true, manually panning the map released the active camera lock, while manually zooming did not.

The PR ensures that zooming releases the active camera lock, just like panning, by transitioning the control from ACTIVE_LOCK to BACKGROUND.

Launch Checklist

  • 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.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.
  • Confirm you have read our AI policy here.

Assisted-By: Codex (GPT-5)

@HarelM

HarelM commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Can you try and see if you can find the cause that this test was added and the origin decision as to way the state was not changed to background for zoom, as the code suggests this is intentional and I want to see if we can understand why.
Otherwise this change looks good.
Thanks!

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (3bced3c) to head (4b7979e).
⚠️ Report is 50 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7909      +/-   ##
==========================================
- Coverage   93.21%   93.19%   -0.02%     
==========================================
  Files         290      290              
  Lines       24656    24656              
  Branches     6484     6484              
==========================================
- Hits        22983    22979       -4     
- Misses       1673     1677       +4     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@timsluis

timsluis commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@HarelM I did some research and found this:

The question is: how should we proceed?

My preference would be to release the lock when zooming. Panning and zooming are closely related and often combined, so having them produce different outcomes may be confusing. On mobile devices, it is also difficult to zoom without introducing some panning. Also, currently, a combined pan-and-zoom gesture does not release the lock (which leaves the location dot not in the middle of the screen).

We should make our own decision here, but for comparison: Mapbox does release the lock when the user zooms.

@timsluis

Copy link
Copy Markdown
Contributor Author

Another solution would be to preserve the current zoom level when the user re-enables the lock. So, when the state transitions from BACKGROUND to ACTIVE_LOCK.

@HarelM

HarelM commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Maybe we can introduce a flag to support both behaviors? IDK. As you can read from the linked issue I think zoom should release lock, but I think @tderflinger disagrees, and I can respect that.

Regarding keeping the zoom that an option as well.
I personally developed a small geolocation control of my own, which isn't a lot of code, but I can see why one might want to avoid this path.

Let's wait for @tderflinger response before we proceed.

@tderflinger

Copy link
Copy Markdown
Contributor

Thanks @HarelM. For me the change by @timsluis is fine if that is what the majority of users want. Maybe asking real-world users of MapLibre about this behavior and their expectations would make sense. For me, keeping a flag would be beneficial if users are split about the behavior and like to have both options available to them.
The original issue (#3504) was created by @michaelkirk, maybe he has an opinion on that, too.

@timsluis

timsluis commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@HarelM A small custom geolocation control might indeed be the best solution if someone wants custom behavior. Do still you still use the MapLibre geolocation events? Or is it fully custom?

About this solution: the current implementation fights the map user by zooming back after the user tries to zoom. So I think any of both is better than what we have now.

I might have a slight preference for locking on the current zoom level now. However, that would be a bigger change.

@michaelkirk

Copy link
Copy Markdown
Contributor

As I said in the original issue, I think you should look to what most popular mapping apps on the platform do. That is what users will expect.

@HarelM

HarelM commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

I don't have any way to know what most users would prefer unfortunately...
I have a bit more complicated case that also uses the compass to show the location direction when the user is not moving, so I have the logic here:
https://github.com/IsraelHikingMap/Site/blob/5b496828f50a5ff8c0fb436257fca26ef0f7748b/IsraelHiking.Web/src/application/services/geo-location.service.ts
https://github.com/IsraelHikingMap/Site/blob/5b496828f50a5ff8c0fb436257fca26ef0f7748b/IsraelHiking.Web/src/application/services/device-orientation.service.ts
https://github.com/IsraelHikingMap/Site/blob/5b496828f50a5ff8c0fb436257fca26ef0f7748b/IsraelHiking.Web/src/application/services/location.service.ts
https://github.com/IsraelHikingMap/Site/blob/5b496828f50a5ff8c0fb436257fca26ef0f7748b/IsraelHiking.Web/src/application/components/location.component.ts

It also uses some native capabilities, so I needed a more advanced solution to this. But if this is an important UX in your app I would consider writing your own. But that might just be me...

Bottom line, if both behaviors are needed a flag should be added. If we can agree on a default behavior we should change the current one to match it.

@HarelM

HarelM commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

I've added it to the monthly meeting agenda, feel free to join in and share your thoughts!

@HarelM HarelM added the need more info Further information is requested label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

need more info Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GeolocateControl resets the user-selected zoom on the next location update while camera tracking is active

4 participants