Fix invalid tile key issues when swapping rasters when region picker active

carbonplan / maps

interactive multi-dimensional data-driven web maps

https://maps.demo.carbonplan.org

Other

214 stars 18 forks source link

Fix invalid tile key issues when swapping rasters when region picker active #105

Closed Shane98c closed 9 months ago

Shane98c commented 9 months ago

~~Adds a couple checks to make sure we're not accessing the tile before it has been initialized. This solves errors when swapping raster properties (like variable) while a region picker is active.~~

Fixes zoom/center state initialization and makes sure dataset level is correctly calculated before querying.

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Shane98c commented 9 months ago

Ok new direction here. Traced this back to the camera center/zoom properties being undefined. This was due to the way useControls was set up.

Since the map was already loaded, map.on('load', ... was never (or at least not consistently) fired, so those values remained undefined until the map was moved. This change makes the the state initialization ask the map right away for the current values, removes the 'load' event listener, and also cleans up the listener on dismount.

I'm slightly worried that map.getZoom() and map.getCenter() could be undefined if useControls is raced against map initialization, but I don't think that's likely with its current usage.

I'm now seeing key issues (they look totally legit, but don't exist on this.tiles) only in cases where I zoom in somewhat far, which could be an unrelated to maps and instead have something to do with the zarr metadata for OAE not setting maxzoom correctly?

Shane98c commented 9 months ago

Ok I think I have a good solution to the timing thing we were diving into yesterday - we just need to update this.level during initialization since maxZoom is fine in there. We don't actually have to run the entire updateCamera function and worry about timing if we take this route. We also don't need to be in the updateCamera function for access to zoom because this.zoom is correct and accessible in the initialization function.