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

date-picker locale attr/prop value should be case insensitive #3925

Closed
1 of 2 tasks
jcfranco opened this issue Jan 12, 2022 · 14 comments
Closed
1 of 2 tasks

date-picker locale attr/prop value should be case insensitive #3925

jcfranco opened this issue Jan 12, 2022 · 14 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. i18n-l10n issues dealing with internationalization/localization p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@jcfranco
Copy link
Member

jcfranco commented Jan 12, 2022

Actual Behavior

Stems from #3602 (comment)

When using locale="zh-CN" and locale="zh-cn", only the former works as it matches its localization file (the former defaults to English).

Note: this also applies to any supported locale with a language and region code

Expected Behavior

Using locale="zh-CN" or locale="zh-cn" should load the zh-CN.json localization file.

Reproduction Sample

https://codepen.io/jcfranco/pen/RwLqvme?editors=1000

Reproduction Steps

  1. Compare groups with different cased locale values.

Reproduction Version

@esri/calcite-components@1.0.0-beta.74

Relevant Info

Regression? Last working version: @esri/calcite-components@1.0.0-VERSION

Per https://tools.ietf.org/search/bcp47#section-2.1.1, the value of locale should be case-insensitive:

2.1.1. Formatting of Language Tags

At all times, language tags and their subtags, including private use
and extensions, are to be treated as case insensitive: there exist
conventions for the capitalization of some of the subtags, but these
MUST NOT be taken to carry meaning.

Thus, the tag "mn-Cyrl-MN" is not distinct from "MN-cYRL-mn" or "mN-
cYrL-Mn" (or any other combination), and each of these variations

Source

  • CDN
  • NPM package
@jcfranco jcfranco added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. p - high Issue should be addressed in the current milestone, impacts component or core functionality 0 - new New issues that need assignment. labels Jan 12, 2022
@jcfranco jcfranco added this to the Sprint 01/03 - 01/14 milestone Jan 12, 2022
@github-actions
Copy link
Contributor

More information is required to proceed with this issue:

  • Copy the appropriate issue template format and paste it into a comment, providing all of the requested items

This issue will be automatically closed in five days if the information is not provided. Thanks for your understanding.

@github-actions github-actions bot added the incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. label Jan 12, 2022
@y0n4 y0n4 self-assigned this Jan 12, 2022
@y0n4 y0n4 added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Jan 12, 2022
@benelan benelan removed the incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. label Jan 13, 2022
@AdelheidF
Copy link

AdelheidF commented Jan 13, 2022

Is it expected that "zh" or "ZH" doesn't work but "pt" does work?
Normally the language codes for those languages are more specific like "zh-ch" or "pt-br".

@jcfranco
Copy link
Member Author

Looking at the implementation, it is expected. pt works because we have the fallback available. @raje9276 In this case, should we always provide the fallback alongside the specific locale values or just the latter?

FWIW, pt-PT.json is available, but pt-BR.json is missing.

@y0n4 y0n4 added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Jan 14, 2022
@y0n4 y0n4 added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Jan 20, 2022
@github-actions github-actions bot assigned benelan and unassigned y0n4 Jan 20, 2022
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@AdelheidF
Copy link

AdelheidF commented Jan 20, 2022

FWIW, pt-PT.json is available, but pt-BR.json is missing.

We have a language code pt-BR. So what's the plan for that one? Currently it shows in English.

zh-cn, zh-hk, zh-tw work now. zh doesn't, but that seems to be expected.

@jcfranco
Copy link
Member Author

We have a language code pt-BR. So what's the plan for that one? Currently it shows in English.

I think we should do 2 things:

  1. add support for pt-BR
  2. enhance the locale util to fetch the base language file if the specific one cannot be found and show English if this one fails.

I'll create issues for these.

@jcfranco
Copy link
Member Author

Actually, the base language lookup worked before. We didn't have a test for this case, so this ended up introducing a regression. I'll create an issue for this. cc @y0n4

@jcfranco
Copy link
Member Author

Regression issue created.

Would we still need to add the pt-BR locale file or would the existing pt one suffice? cc @raje9276

@AdelheidF
Copy link

AdelheidF commented Jan 20, 2022

Would we still need to add the pt-BR locale file or would the existing pt one suffice?

Olga says pt and pt-BR are the same for calendar.

@raje9276
Copy link

@jcfranco pt is default for pt-PT (Portuguese Portugal) and it is different from pt-BR (Portuguese Brazilian). For Calendar the translations and formats are same but ESRI translation (resource files) are translated for pt-PT and pt-BR (different translations).

Did translation services team give you the translations?

@jcfranco
Copy link
Member Author

@raje9276 Thanks for the info!

We have calendar resource files for pt.json and pt-PT.json, but none for pt-BR. If I understood correctly, this isn't an issue for our calendar use case.

Long term, in terms of maintainability would you suggest:

  1. Drop pt-PT.json and rely on pt.json?
  2. Add pt-BR.json even if it introduces another redundant file?
  3. Leave it as is since it would work regardless? (this seems a bit broken to me)

All of the above are from the context of calendar.

@raje9276
Copy link

@jcfranco you need 2 files pt-PT.json (pt.json) and pt-BR.json even though they are the same.

We need 2 files because there is a possibility that the translations may change as we add more strings or the current translations change as part of language update.

@jcfranco
Copy link
Member Author

Got it. I'll submit a PR to add the missing file. Thanks again!

@jcfranco jcfranco added the i18n-l10n issues dealing with internationalization/localization label Jan 21, 2022
@benelan
Copy link
Member

benelan commented Feb 7, 2022

verified on beta.76

@benelan benelan closed this as completed Feb 7, 2022
@benelan benelan added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. i18n-l10n issues dealing with internationalization/localization p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests

5 participants