Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture browse layer state in the URL #508

Merged
merged 8 commits into from
Jun 6, 2024
Merged

Conversation

dabreegster
Copy link
Contributor

Demo: https://acteng.github.io/atip/browse_layer_urls/browse.html?style=dataviz&crossings=1&stats19=other%2F2019%2F2022&road_noise=1#5.69/53.021/-1.825

This syncs up the state of browse layers with URL query parameters. This makes URLs much more reproducible/shareable, and is a step towards linking directly to the browse page (or embedding in an iframe?) from route check.

Note:

  • If someone changes the URL, there's a full page refresh. We could squeeze everything into the URL fragment if we wanted to change this, but I don't think there's a use case for it
  • No change for the critical issues layer, because that requires a custom file input
  • We can't use a single query param (like layers=x,y,z) because some layers have more details
  • The URL as it is now isn't quite reproducible, because the camera viewport gets ignored. That was a choice made because of a bug in svelte-maplibre that's now fixed, so I'll followup separately and fix on all pages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of a nice and simple change

let show = false;
let opacity = 50;
let pollutant = "PM25_viridis";
type State = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of a more complex change. Any layer with more controls beyond show/hide has something like this. It's verbose, but I don't think it can really be much less code than this, without trying to encode as some very awkward JSON strings.

let param = new URLSearchParams(window.location.search).get(name);
let initialValue = param == null ? defaultValue : parse(param);
let store = writable(initialValue);
// TODO How do we avoid leaking this?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In prod, this won't be a problem -- each layer component is created only once, so there'll only be one store for the lifetime of the app. In dev mode with HMR, this will leak memory, because I couldn't figure out a clean way to call unsubscribe / teardown the stores. We could add onDestroy handlers to every single component I guess, but seems like overkill for a dev-mode mostly-invisible problem

Copy link
Contributor

@Pete-Y-CS Pete-Y-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool feature! I didn't fully understand the memory leak but if it only exists for dev users if you don't refresh for ages I think that's absolutely fine

@dabreegster
Copy link
Contributor Author

Thanks for the review

I didn't fully understand the memory leak

https://learn.svelte.dev/tutorial/auto-subscriptions

@dabreegster dabreegster merged commit 4a73c03 into main Jun 6, 2024
2 checks passed
@dabreegster dabreegster deleted the browse_layer_urls branch June 6, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants