Marker animation mostly fixed #1257
Conversation
| consolidatedMarkers = updateMarkers(markers, prevMarkers, hashDif); | ||
| // Get array of new markers with old positions | ||
| const cloneArray = updateMarkers(markers, prevMarkers, hashDif); | ||
| consolidatedMarkers = [...prevMarkers, ...cloneArray]; |
There was a problem hiding this comment.
Reversing the order of the spreads here does not fix the "old marker hidden under new markers" problem.
Did you try setting a higher z-index value for the old markers? |
There was a problem hiding this comment.
Can you explain how removing enqueueZoom fixes the issue?
There was a problem hiding this comment.
To be honest, not entirely sure. I suspect React 18-related timing changes messed something up. There was nothing else to "blame" as far as I could see in the animation-related source files.
Although there's a remaining niggle with the z-index ordering of old vs new markers during animation, I am happy to remove code.
The enqueued setTimeout isn't necessary because on the next render (which is now debounced to the same milliseconds as animation (300ms)) the animation function will basically be a no-op because the geohash level is the same for prevRecenteredMarkers and recenteredMarkers - so it returns recenteredMarkers which will replace the previous concatenated old+new markers.
Yes, I tried |
That surprising. If the markers are in the same stacking context, z-index should work. Might look into it later 🙂 |
Fixes #1227
The only problem I see is that when zooming in, it would be nice to have the old aggregated marker on top, and have the new disaggregated markers animate out from underneath it. I couldn't make this happen :-( (see the comments in updateMarkers.tsx) It's not a deal-breaker for me.
Needs a lot more heavy testing though.