mikebryant / ac-nh-turnip-prices

Price calculator/predictor for Turnip prices
https://turnipprophet.io
Apache License 2.0
1.46k stars 244 forks source link

Language selector does not match language when default is not English #289

Closed tigerplush closed 4 years ago

tigerplush commented 4 years ago

my page is in german although I selected english from the drop down menu 2020-04-27_17h16_56

mikebryant commented 4 years ago

That's odd - selecting the language works for me locally. Are there any useful messages in the browser console?

(ctrl+shift+J if you're on Windows/Linux and this is Chrome)

What browser, OS, what language is the OS set to?

tigerplush commented 4 years ago

Hey yes, there are some error messages in the browser 2020-04-27_17h25_27

I'm using chrome Version 81.0.4044.122 (Official Build) (64-bit) with windows 10 Version 10.0.18362 Build 18362 my OS is set to english but with german keyboard (which doesn't work in photoshop either, I have to switch to english keyboard for the keyboard shortcuts to work)

tigerplush commented 4 years ago

Changing to espanol and changing back to english has now set it to english

eliannelavoie commented 4 years ago

I've managed to get a similar bug: image

Page in french, but dropdown set to english. I need to click on another language, then click on english to select it.

Reproduction is quite simple: I just open the page for the first time (I use Incognito mode) with Google Chrome in french, and it's like that!

If I change Google Chrome's language to english, the page loads in english, and the dropdown isn't desynced.

eliannelavoie commented 4 years ago

This project seems to use i18next-browser-languageDetector.

I think there needs to be a check for when the detected language is not equal to the default language. In that case, the dropdown should be updated to show the user's detected language.

theRTC204 commented 4 years ago

Yeah, seems like we're just not setting the dropdown to the default language when it is not English. There's a bug there for sure.

DevSplash commented 4 years ago

This is because the language code detected is slightly different from the code in the language list. For example, the detected language code is de-DE but the code in the language list is only de. In this case the selector will select the default option en. I'll try to fix it.

theRTC204 commented 4 years ago

Good catch @DevSplash! I did notice that for myself it attempts to local an en-US file and then fails and falls back to en. I hadn't previously considered the same thing would happen for other languages that don't exactly line up by key name.

DevSplash commented 4 years ago

Good catch @DevSplash! I did notice that for myself it attempts to local an en-US file and then fails and falls back to en. I hadn't previously considered the same thing would happen for other languages that don't exactly line up by key name.

I didn't find how to get the actual language code in the i18next documentation.
Maybe we can modify here: https://github.com/mikebryant/ac-nh-turnip-prices/blob/d7667e1f8e0a3feeb248904277932d5bd61b7508/js/translations.js#L35 languageSelector.append(`<option value="${code}"${i18next.language == code || i18next.language.split('-')[0] == code ? ' selected' : ''}>${name}</option>`); Do you have a better way?

theRTC204 commented 4 years ago

The code itself shouldn't require any changes. We just have to rename the JSON files in the locales directory, and then update the language aray with the full region-language names.

https://github.com/mikebryant/ac-nh-turnip-prices/blob/d7667e1f8e0a3feeb248904277932d5bd61b7508/js/translations.js#L16-L32

Some of these names are already correct, while many still just have the language without region.

mikebryant commented 4 years ago

I'm not sure that's the right call. For a lot of these we want fallback to the general language, even if we don't have the specific one.

e.g. If the browser locale is set to en-US, it rightly falls back to en.

I don't want to change all the files in locale/ to be more specific, when it'll mean some people don't get a translation, but could have had a generic one that was workable.

If we change the code that looks for things in the select box to do the same sort of fallback, that would be quite nice.

theRTC204 commented 4 years ago

So the auto localization is correctly falling back to the target language, even without the region specific verion being available, and it's just the selector that doesn't match up?

mikebryant commented 4 years ago

Yes, that's my understanding