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

On the web, modify the current URL as we change maps and scenarios. #509

Merged
merged 1 commit into from
Feb 12, 2021

Conversation

dabreegster
Copy link
Collaborator

This makes the web experience slightly nicer, letting somebody copy the URL and send it to a friend, and have them at least wind up on the right map or scenario. It doesn't work in general for jumping to a challenge mode or tutorial or arbitrary game state, but I think that's likely overkill -- this covers the common cases.

The simplest test: run locally, open localhost without any query params to get the title screen. Clicking sandbox brings you to the montlake scenario, and now your address bar should reflect this:
Screenshot from 2021-02-10 16-15-07

// Setting window.location.href may seem like the obvious thing to do, but that actually
// refreshes the page. This method just changes the URL and doesn't mess up history. See
// https://developer.mozilla.org/en-US/docs/Web/API/History_API/Working_with_the_History_API.
let history = window.history().map_err(|err| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL that web-sys Results have a JSValue as their Err case.

Some discussion to change that:
rustwasm/wasm-bindgen#1742

But seems like it's stalled out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, it would be so nice if this went through. In the meantime, I guess we can keep the boilerplate. And maybe figure out a macro/helper to make it a bit more ergonomic.

Copy link
Collaborator

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

LGTM - this seems handy!

@dabreegster dabreegster merged commit 8d0718d into master Feb 12, 2021
@dabreegster dabreegster deleted the change_urls branch February 12, 2021 23:14
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.

2 participants