fix: [issue-2868] after enabling terrain, the _update marker method was not called

[sebastianoscarlopez]

Fixes #2868.

We need to update the marker position after the terrain is loaded; when the map is fully loaded an idle event is fired.

So the approach was that after the terrain event is detected by the marker wait till an idle event is reached.

issue-2868.mp4

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.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (998533e) 75.03% compared to head (a6c549c) 75.03%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2996   +/-   ##
=======================================
  Coverage   75.03%   75.03%           
=======================================
  Files         240      240           
  Lines       19127    19131    +4     
  Branches     4316     4318    +2     
=======================================
+ Hits        14351    14355    +4     
  Misses       4776     4776           

Files Changed	Coverage Δ
src/ui/marker.ts	96.45% <100.00%> (+0.05%)	⬆️

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

[jashanbhullar]

I tested this, works fine on the turning terrain on and off but doesn't work on the first render of the map.

Screencast.from.20-08-23.09.22.32.AM.IST.webm

[sebastianoscarlopez]

I tested this, works fine on the turning terrain on and off but doesn't work on the first render of the map.

Screencast.from.20-08-23.09.22.32.AM.IST.webm

Thanks @jashanbhullar good catch. I'll try to fix the first render.

[sebastianoscarlopez]

First sorry for delaying it, I had little spare time.
I think it's ready, it always works, even in the first renders.

[HarelM]

Great!! Can you add some tests to it? Maybe a browser integration test, or at least a unit test?

[sebastianoscarlopez]

integration browser test was added. If this test is run in the current main branch it will throw this error.

note: I needed to use the idle event, my assumption is that is okay to use it in the test.

[sebastianoscarlopez]

Hi @HarelM, please let me know if this fix and its test code are good enough or how it can be improved.

[HarelM]

The code looks good, please add a change log entry, and a unit test if possible would be great so that the code in this class continues to be fully covered.

[sebastianoscarlopez]

@HarelM please take a look at it.

[HarelM]

I did, the location of the changelog entry is still incorrect and should be placed above the 3.3.1 heading.

[sebastianoscarlopez]

I did, the location of the changelog entry is still incorrect and should be placed above the 3.3.1 heading.

I'm so dumb 🤦

[HarelM]

Not sure why we keep this test, but there's a need to fix it. i.e. change the expected build size...