hackgvl / open-map-data-multi-layers-demo

A bookmarkable map displaying all layers from HackGreenville Labs' Open Map Data
https://hackgvl.github.io/open-map-data-multi-layers-demo/
MIT License
2 stars 2 forks source link

Map doesn't navigate between tabs properly #25

Closed oliviasculley closed 8 months ago

oliviasculley commented 8 months ago

Steps to reproduce:

  1. Open the map
  2. Check some layers
  3. Switch to the "About" tab
  4. Switch back
  5. None of the old pins load, and trying to load more layers breaks :(

Ideal requirements to close this issue:

  1. Adding a test that tests the above scenario to make sure it works in the future
  2. Fixing the issue :yum:
ThorntonMatthewD commented 8 months ago

It seems like this is where the issue is manifesting: https://github.com/hackgvl/open-map-data-multi-layers-demo/blob/01e7b94895c99603b2d8ce3246893dd44c8c07d6/src/components/MainMap.vue#L107

If you have previously been on the map page and navigate to the About page, once you come back to the mapp all of the layers will still be in mapStore.loadedMaps. The conditional block that catches maps that are already in the store does not add layers to the layer control (like what happens here), so the layer pop-up remains blank. :scream:

To test this I threw the following into the MainMap.vue component:

onUnmounted(() => {
  mapStore.loadedMaps = {};
});

By wiping the loaded maps whenever the MainMap component is unmounted, upon return to the Map after roaming elsewhere I can add layers successfully. This is a pretty poor solution, as the data we are trashing would then just be repopulated, but it at least can be used to test the theory. It also causes the query params to not be rebuilt correctly here.

ThorntonMatthewD commented 8 months ago

This is a prototype showing a potential solution: https://github.com/hackgvl/open-map-data-multi-layers-demo/compare/develop...ThorntonMatthewD:open-map-data-multi-layers-demo:fix-retention-of-layers-prototype

I've not gone through and created a CFG yet. I'm hoping doing that will might help draw attention to maybe a better way to juggle the various layer states in addMapLayer.

This block is the trickiest since the same layer can hit this multiple times, and we can't just toss a line to add the layer to the control list there (unless we want multiples to pile up).

L.Control has a private _layers method that could be used to tell which layers are already present in the control list to help get around the above issue, but that seems a bit naughty. I can't see that interface changing too much, but I don't really want to take a chance.