Skip to content

Fix browser language detection for region specific languages (#8982)#9026

Merged
bramkragten merged 5 commits intohome-assistant:devfrom
rmogstad:fix-incorrect-language-detection
Apr 29, 2021
Merged

Fix browser language detection for region specific languages (#8982)#9026
bramkragten merged 5 commits intohome-assistant:devfrom
rmogstad:fix-incorrect-language-detection

Conversation

@rmogstad
Copy link
Copy Markdown
Contributor

Proposed change

Detect hyphenated languages (e.g. en-US) as we are looking through the list of browser languages, rather than as a separate step. This prevents it from picking a later language that is non-hyphenated instead of using the preferred language.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Comment on lines -97 to -102
if (navigator.language && navigator.language.includes("-")) {
language = findAvailableLanguage(navigator.language.split("-")[0]);
if (language) {
return language;
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think adding it to navigator.languages makes sense, but we should not remove it here? We should instead make a helper function for:

      language = findAvailableLanguage(locale);
      if (language) {
        return language;
      }
      if (locale.includes("-")) {
        language = findAvailableLanguage(locale.split("-")[0]);
        if (language) {
          return language;
        }
      }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, I see. I misunderstood what navigator.language was. I will make a method and call it in both places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made a change that is a little different. I couldn't think of a good name for the helper function, and it seemed like something we would want to always check when searching for an available translation, so I added it to that function.

I'll understand if you prefer not to have the recursive call there and want me to break it out differently.

This is my first time doing a CR on github, first time submitting to HA (or any OS project, for that matter), and first time working with TS, so I'm not super familiar with the industry best practices, nor with the project best practices. I am more than happy to make changes to make it

I am in the discord (@Zondebok) if you want to discuss directly.

@rmogstad rmogstad force-pushed the fix-incorrect-language-detection branch from 31b3348 to 25cd7c7 Compare April 28, 2021 22:09
rmogstad and others added 2 commits April 29, 2021 08:37
Co-authored-by: Bram Kragten <mail@bramkragten.nl>
Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@rmogstad
Copy link
Copy Markdown
Contributor Author

should I squash/rebase this at some point?

@bramkragten
Copy link
Copy Markdown
Member

should I squash/rebase this at some point?

No, we squash when we merge :-)

Co-authored-by: Bram Kragten <mail@bramkragten.nl>
@bramkragten bramkragten merged commit a28616d into home-assistant:dev Apr 29, 2021
@bramkragten bramkragten mentioned this pull request Apr 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When default language is set to a hyphenated locale (e.g. en-US), the frontend will choose the first non-hyphenated language in the translation list

3 participants