w3c / jlreq

Text Layout Requirements for Japanese
https://w3c.github.io/jlreq/
Other
100 stars 16 forks source link

Update URL when swithing the language in UI #262

Closed kojiishi closed 5 months ago

kojiishi commented 3 years ago

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 commented 2 years ago

Hello @r12a could you review this one? thanks!

kidayasuo commented 5 months ago

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

r12a commented 5 months ago

I haven't checked the code in detail, but i read https://github.com/w3c/jlreq/issues/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 commented 5 months ago

@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 commented 5 months ago

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.

kojiishi commented 5 months ago

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 ;-)