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

Allow locale and language being set via URL param #5650

Merged
merged 2 commits into from Dec 23, 2018
Merged

Allow locale and language being set via URL param #5650

merged 2 commits into from Dec 23, 2018

Conversation

tordans
Copy link
Collaborator

@tordans tordans commented Dec 22, 2018

With the hash-url locale=en-US or locale=de-DE one can force a locale and language regardless of the given language from the osm-website-settings.

This is to solve #5644.

However, while looking into this I understood that the interface language is in fact pulled from the OSM-website-profile-language (not the browser language). So there is a way to change the language, by editing the preferred languages in the profile.

So while this PR makes changing the language a bit easier and quicker, I am not sure if it is worth the added complexity.

In case this should be continued, those thinks are todo IMO:

With the hash-url `locale=en-US` or `locale=de-DE` one can force a locale and language regardless of the given language from the osm-website-settings.
@bhousel
Copy link
Member

bhousel commented Dec 22, 2018

[ ] This line https://github.com/openstreetmap/iD/blob/master/modules/util/detect.js#L93 is duplicated now, should it be removed?

Yes, it's unnecessary to do it twice. You could put the line once at the top of the function if the q hash will be used in a few places.

Should there be specs for this? If so, I don't know enough about specs to write good ones.

I'm not sure we can test this feature, because I'm guessing window.location.hash from within the tests would cause issues. This is ok.

@tordans
Copy link
Collaborator Author

tordans commented Dec 23, 2018

@bhousel ready for review. Thanks.

@bhousel
Copy link
Member

bhousel commented Dec 23, 2018

Thanks @tordans - looks good!
Could you also add a short description of the parameter here: https://github.com/openstreetmap/iD/blob/master/API.md

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