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

add additional locale support to Carbon #6731

Merged
merged 11 commits into from
Jun 7, 2024
Merged

add additional locale support to Carbon #6731

merged 11 commits into from
Jun 7, 2024

Conversation

tomdavies73
Copy link
Contributor

@tomdavies73 tomdavies73 commented May 10, 2024

Proposed behaviour

Adds native locale support for de-DE, en-CA, en-US, es-ES, fr-FR and fr-CA. The only distinction betweenen-CA and en-US compared to en-GB currently is date formatting.

de-DE, es-ES and fr-FR are completely new locales. fr-CA has also been added which inherits the translations provided in fr-FR but some specific locale objects are overridden due to language variation.

PLEASE NOTE: Not all translations have been provided, you will find for some locale objects either the "NO TRANSLATION PROVIDED" string has been passed or the locale objects using string interpolation have been left empty. We have requested these translations, and they are in the process of being translated by the translations team. This PR will not be merged until we have all required translations.

Our i18n documentation has also been updated to represent these new changes, and to improve the general quality of the documentation overall.

Current behaviour

Currently, only the en-GB locale is supported in Carbon.

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

@@ -229,3 +229,4 @@ const enGB: Locale = {
};

export default enGB;
export { enGB };
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: lets just make the breaking change an update the default export in the index and remove this

Copy link
Contributor

@edleeks87 edleeks87 left a comment

Choose a reason for hiding this comment

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

Goof stuff @tomdavies73 I think we should make the breaking change I mention below as part of this PR, I also think we should tweak the commit message slightly (I've pinged you on slack about that). I think the rest of it looks fine, but perhaps we have a chat in office on Thursday with Will as well and iron out anything else we think needs to

docs/i18n.mdx Outdated
switch: {
on: () => "sur",
off: () => "de",
locale: () => "eo-EO",
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: believe for Esperanto it would just be eo

As there is no regional option? Nice flex though. Just not sure it's the ideal example, but shouldn't cause any problems.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggstion: add caveat that we only officially support enGB locale

We need to stipulate that we don't officially support the other locales and that they're provided as an open source implementation only. We aren't responsible for any mistakes and a PR should be raised for corrections

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: add info about fall back to enGB

@edleeks87 edleeks87 changed the title feat(de-de, en-ca, en-us, es-es, fr-fr, fr-ca): add native locale support chore(en-gb): change named export from EnGb to enGB May 17, 2024
@tomdavies73 tomdavies73 force-pushed the FE-6429 branch 3 times, most recently from 568c4be to 4297940 Compare May 17, 2024 16:36
nineteen88
nineteen88 previously approved these changes May 21, 2024
@tomdavies73 tomdavies73 changed the title chore(en-gb): change named export from EnGb to enGB add additional locale support to Carbon May 21, 2024
edleeks87
edleeks87 previously approved these changes May 22, 2024
nineteen88
nineteen88 previously approved these changes May 22, 2024
edleeks87
edleeks87 previously approved these changes Jun 3, 2024
docs/i18n.mdx Outdated

| Component | Link to translation keys table |
|-----------------------|-------------------------------------------------------------------------------------------|
| actions | [Table](https://carbon.sage.com/?path=/docs/pod--docs#translation-keys) |
Copy link
Contributor

@nineteen88 nineteen88 Jun 3, 2024

Choose a reason for hiding this comment

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

question(non-blocking): why are we using absolute links rather than relative?

nineteen88
nineteen88 previously approved these changes Jun 3, 2024
@tomdavies73 tomdavies73 merged commit 9aac9a5 into master Jun 7, 2024
21 checks passed
@tomdavies73 tomdavies73 deleted the FE-6429 branch June 7, 2024 13:55
@carbonci
Copy link
Collaborator

carbonci commented Jun 7, 2024

🎉 This PR is included in version 137.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

5 participants