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 to override language metadata #11018

Merged
merged 2 commits into from
Apr 14, 2022
Merged

Allow to override language metadata #11018

merged 2 commits into from
Apr 14, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Apr 12, 2022

What it does

Closes #11010

  1. Fixes a bug in the LocalizationProvider which should've allowed to merge localization. It only merged the translations property and nothing else, leaving the languageName empty.
  2. Returns a bit more metadata in the getAvailableLanguages() method.

How to test

  1. Change the language and assert that the app works as expected.
  2. Set a breakpoint in this line and assert that the languageName and localizedLanguageName property is correctly set in the availableLanguages array (is undefined on master).

Review checklist

Reminder for reviewers

@msujew msujew added the localization issues related to localization/internalization/nls label Apr 12, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that getAvailableLanguages now contains additional information:

localization

However there is an error thrown when executing configure display language with a language-pack installed:

image

@msujew
Copy link
Member Author

msujew commented Apr 13, 2022

@vince-fugnitto I cannot reproduce the issue you experience (the error message in the console), but I've made the code a bit more error-safe (it now checks that the languageId is actually a string). Does it still happen for you?

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

It works well for me now 👍

For reference I had:

  • installed the french language pack
  • triggered the configure display language command
  • the quick-pick was empty, and the error was thrown

@msujew msujew merged commit 27535cc into master Apr 14, 2022
@msujew msujew deleted the msujew/i18n-language-info branch April 14, 2022 12:33
@github-actions github-actions bot added this to the 1.25.0 milestone Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localization issues related to localization/internalization/nls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get more detailed language info
2 participants