maplibre-gl-js icon indicating copy to clipboard operation
maplibre-gl-js copied to clipboard

Remove finalize elevation when updating elevation on the fly

Open olsen232 opened this issue 10 months ago • 4 comments

Don't _finalizeElevation if freezeElevation has not been set.

This means that elevation logic has two distinct operating modes.

Mode one - freezeElevation is false - default:

  • updateElevation is called continuously during the movement
  • finalizeElevation is not called at the end.

Mode two - freezeElevation is true:

  • updateElevation is not called continuoulsy during the movement
  • finalizeElevation is called at the end.

Previously, both methods were called in mode one, which lead to issue #3878 if freezeElevation was not set.

Fixes #3878

Launch Checklist

  • [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • [x] Briefly describe the changes in this PR.
  • [x] Link to related issues.
  • [ ] Include before/after visuals or gifs if this PR includes visual changes.
  • [x] Write tests for all new functionality.
  • [ ] Document any changes to public APIs.
  • [ ] Post benchmark scores.
  • [x] Add an entry to CHANGELOG.md under the ## main section.

olsen232 avatar Apr 04 '24 23:04 olsen232

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 86.76%. Comparing base (aaeaf58) to head (9428223).

Files Patch % Lines
src/ui/camera.ts 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3944      +/-   ##
==========================================
+ Coverage   86.49%   86.76%   +0.27%     
==========================================
  Files         242      242              
  Lines       33041    33041              
  Branches     2036     2012      -24     
==========================================
+ Hits        28578    28669      +91     
+ Misses       3481     3404      -77     
+ Partials      982      968      -14     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 04 '24 23:04 codecov-commenter

This fix makes sense to me regardless of what I wrote in the issue.

HarelM avatar Apr 07 '24 17:04 HarelM

Can you add a test and a changelog item? Since elevation is constantly updated there's no need to finalize it, right? I think this should get in.

HarelM avatar May 03 '24 08:05 HarelM

Added a test, updated the changelog.

olsen232 avatar May 06 '24 01:05 olsen232