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

Use language string from common code instead of NumberSuiteCommonStrings.languageStringProperty #15

Closed
chrisklus opened this issue Jan 5, 2023 · 5 comments

Comments

@chrisklus
Copy link
Contributor

Before I knew about accessing the language name for each locale code, I added a 'language' string key so every translated locale could add the name of their language. But this information already exists in common code (think, the dynamic locale selector in the preferences menu). So the language key should be removed and replaced with the existing info, assuming that is accessible.

@chrisklus chrisklus self-assigned this Jan 5, 2023
@zepumph
Copy link
Member

zepumph commented Jan 12, 2023

@zepumph
Copy link
Member

zepumph commented Jan 12, 2023

Don't forget about the TODO I added in WordAccordionBox.

@zepumph
Copy link
Member

zepumph commented Jan 13, 2023

Ok! I kinda did a lot in the above commits, but I got the original issue taken care of. I tried to keep things separated to their own issues for clarity. Part of this work was getting all TODOs in all number suite repos having issues with them. There is now a lint rule for that.

As for the problem at hand, using localeInfoModule[ locale ].localizedName worked very well. In those cases, I was passing secondLocaleProperty through instead of the second locale stringMap. Ready for review.

@zepumph
Copy link
Member

zepumph commented Jan 13, 2023

I think there is some real trouble with how we get our secondary strings map. It may be related to #22, but currently on master I cannot load the built sim. I believe this has to do with how the string map we are using doesn't include fallbacks correctly. I don't have time to investigate tonight but will work on it more tomorrow.

@chrisklus
Copy link
Contributor Author

Thanks @zepumph! I really like these changes. LocaleSwitch and WordAccordionBox look great.

I think there is some real trouble with how we get our secondary strings map. It may be related to #22, but currently on master I cannot load the built sim. I believe this has to do with how the string map we are using doesn't include fallbacks correctly. I don't have time to investigate tonight but will work on it more tomorrow.

We fixed the first problem in #22, and fallbacks are going to be addressed in #13.

I think we are ready to close here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants