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

Update URL when swithing the language in UI #262

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

kojiishi
Copy link
Contributor

This patch changes to update the URL search parameters when
switching the language in UI. This change can ensure the URL
can navigate to what the user is seeing.

Fixes #236.

@kidayasuo
Copy link
Contributor

Hello @r12a could you review this one? thanks!

@kidayasuo
Copy link
Contributor

@r12a could you add your review? I would like to add this fix in the final (I hope) update of JLReq.
thank you!

@r12a
Copy link
Contributor

r12a commented Feb 14, 2024

I haven't checked the code in detail, but i read #236 and this seems like a useful feature, so long as the user is still presented with the buttons allowing them to choose an alternative language. If that's not the case, i won't approve this change. @kojiishi can you confirm?

@kojiishi
Copy link
Contributor Author

@r12a

so long as the user is still presented with the buttons allowing them to choose an alternative language. If that's not the case, i won't approve this change. @kojiishi can you confirm?

Yes, the latest editor's draft still has the buttons on the top-right corner.
image

@r12a
Copy link
Contributor

r12a commented Feb 14, 2024

But the latest ED presumably doesn't include these changes(?) since they haven't been merged yet.

To speed things up i'll mark this as accepted on the assumption that the buttons will still be there after merging. I took another look at the code, and it doesn't seem to affect the buttons.

This patch changes to update the URL search parameters when
switching the language in UI. This change can ensure the URL
can navigate to what the user is seeing.
@kojiishi kojiishi force-pushed the switch-lang-to-url-search-params branch from 10fdb5e to d109d3c Compare February 14, 2024 16:00
@kojiishi
Copy link
Contributor Author

Thanks for the approval. I rebased to the ToT and confirmed it's still working. I was actually a bit scary because the original patch was 3 years ago ;-)

@kojiishi kojiishi merged commit 4a8330e into w3c:gh-pages Feb 14, 2024
@kojiishi kojiishi deleted the switch-lang-to-url-search-params branch February 14, 2024 16:05
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.

Update URL when the language is switched
3 participants