Skip to content

feat: support nn locale #12079

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

Merged
merged 3 commits into from
May 2, 2025
Merged

feat: support nn locale #12079

merged 3 commits into from
May 2, 2025

Conversation

anveshmekala
Copy link
Contributor

@anveshmekala anveshmekala commented May 1, 2025

Related Issue: #11978

Summary

  • Add support for nn locale which maps to no
  • removes t9n context from locale util since it is handled by lumina controller.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label May 1, 2025
@anveshmekala anveshmekala marked this pull request as ready for review May 1, 2025 22:28
@anveshmekala anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label May 1, 2025
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

looks good!

also, I know that your t9n utils handle more advanced cases (including workarounds for bugs in browser intl) - we would be happy to host this code under @arcgis/components-utils so that other teams could also benefit.

there is also a todo for the future of sharing components-utls with @arcgis/core, so your intl utils could end up being used by many esri teams

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴
🇳🇴✨🇳🇴🇳🇴🇳🇴✨🇳🇴🇳🇴✨✨🇳🇴🇳🇴✨✨✨🇳🇴🇳🇴✨✨✨🇳🇴✨✨✨✨🇳🇴✨🇳🇴
🇳🇴✨✨🇳🇴🇳🇴✨🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴🇳🇴🇳🇴✨🇳🇴🇳🇴🇳🇴🇳🇴✨🇳🇴
🇳🇴✨🇳🇴✨🇳🇴✨🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴🇳🇴🇳🇴✨✨✨🇳🇴🇳🇴✨🇳🇴
🇳🇴✨🇳🇴🇳🇴✨✨🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴✨🇳🇴🇳🇴🇳🇴🇳🇴✨🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴
🇳🇴✨🇳🇴🇳🇴🇳🇴✨🇳🇴🇳🇴✨✨🇳🇴🇳🇴✨✨✨🇳🇴🇳🇴✨✨✨🇳🇴✨✨✨✨🇳🇴✨🇳🇴
🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴🇳🇴

@@ -162,43 +118,35 @@ export const getSupportedNumberingSystem = (numberingSystem: string): NumberingS
* @param locale – the BCP 47 locale code
* @param context - specifies whether the locale code should match in the context of CLDR or T9N (translation)
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this JSDoc param?

@jcfranco
Copy link
Member

jcfranco commented May 2, 2025

also, I know that your t9n utils handle more advanced cases (including workarounds for bugs in browser intl) - we would be happy to host this code under @arcgis/components-utils so that other teams could also benefit.

there is also a todo for the future of sharing components-utls with @arcgis/core, so your intl utils could end up being used by many esri teams

Happy to contribute, @maxpatiiuk! Is there an issue we can use to port some of these, similar to #11851?

@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 2, 2025
@anveshmekala anveshmekala merged commit 04b69f0 into dev May 2, 2025
14 checks passed
@anveshmekala anveshmekala deleted the anveshmekala/11978-map-nn-to-no branch May 2, 2025 19:37
@anveshmekala
Copy link
Contributor Author

One thing i observed with useT9n controller is the fallback value for pt is pt-PT where as calcite's fallback value is pt-BR prior to migration. I am not sure how much of deviation there would be among strings but something worth getting reviewed by t9n team if we hadn't. Thoughts ? @maxpatiiuk @jcfranco

@anveshmekala
Copy link
Contributor Author

I am not sure how much of deviation there would be among strings

diff check revealed some of carousel message strings are unique for both pt-PT & pt-BR. Applies to loading message string too.

@jcfranco
Copy link
Member

jcfranco commented May 2, 2025

One thing i observed with useT9n controller is the fallback value for pt is pt-PT where as calcite's fallback value is pt-BR prior to migration. I am not sure how much of deviation there would be among strings but something worth getting reviewed by t9n team if we hadn't. Thoughts ? @maxpatiiuk @jcfranco

Did some digging and I wonder if this was a bug related to an early implementation. These lines indicate that there was no t9n bundle for pt-PT, but it looks like we did have bundles for pt-BR and pt-PT by 1.0.3 (see https://github.com/Esri/calcite-design-system/pull/5426/files).

Based on an earlier conversation about pt's fallback, I think the Lumina behavior is correct, although I'm not sure if it was focused primarily on CLDR and/or T9N.

Adding @annierm18 for some expert9nise™.

benelan pushed a commit that referenced this pull request May 14, 2025
**Related Issue:** #11978 

## Summary

- Add support for `nn` locale which maps to `no`
- removes `t9n` context from locale util since it is handled by lumina
controller.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants