carbon icon indicating copy to clipboard operation
carbon copied to clipboard

refactor: standardize translation functionality across components

Open adamalston opened this issue 5 months ago β€’ 3 comments

No issue.

Standardized translation functionality across components.

Changelog

New

  • Added types for all translation functions.

Changed

  • Updated style.md with TypeScript examples to reflect the standardized pattern.
  • Replaced non-standard translation key handling with consistent translationIds, TranslationKey types, and defaultTranslateWithId functions.
  • Replaced translateWithId props with TranslateWithId interface extensions.
  • Standardized all JSDoc comments for translation props since they're all referring to the same functionality.

Removed

  • Removed translationKeys PropTypes because they were inconsistently applied across components. Either all components should have them or none should.

Testing / Reviewing

Reference commits: https://github.com/carbon-design-system/carbon/compare/6d6150ce59fbbf0bb1e5ccb7751a84e58643ab89...3ab6359a7162986ee0aa64604016aa02c35ad866

The goal of these changes is to introduce consistency into the translation functionality. I began looking into these changes when I noticed many implementations weren't following what was listed in style.md.

yarn test packages/react

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

  • [x] Reviewed every line of the diff
  • [x] Updated documentation and storybook examples
  • [ ] ~Wrote passing tests that cover this change~
  • [x] Addressed any impact on accessibility (a11y)
  • [x] Tested for cross-browser consistency
  • [x] Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

adamalston avatar Jun 16 '25 13:06 adamalston

Deploy Preview for v11-carbon-web-components ready!

Name Link
Latest commit 61653a572531292c0a748aecaef171c76372cb4f
Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/68d42b100f7b6e00089f5c7d
Deploy Preview https://deploy-preview-19666--v11-carbon-web-components.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jun 16 '25 13:06 netlify[bot]

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
Latest commit 61653a572531292c0a748aecaef171c76372cb4f
Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/68d42b1003d80b00089df029
Deploy Preview https://deploy-preview-19666--v11-carbon-react.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jun 16 '25 13:06 netlify[bot]

Codecov Report

:x: Patch coverage is 96.25000% with 3 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 91.93%. Comparing base (30b4f87) to head (61653a5). :warning: Report is 225 commits behind head on main.

Files with missing lines Patch % Lines
...ges/react/src/components/DataTable/TableHeader.tsx 87.50% 1 Missing :warning:
...act/src/components/PaginationNav/PaginationNav.tsx 83.33% 1 Missing :warning:
packages/react/src/components/Slider/Slider.tsx 87.50% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #19666      +/-   ##
==========================================
+ Coverage   91.39%   91.93%   +0.54%     
==========================================
  Files         485      484       -1     
  Lines       31370    32771    +1401     
  Branches     5430     5578     +148     
==========================================
+ Hits        28670    30129    +1459     
+ Misses       2547     2495      -52     
+ Partials      153      147       -6     
Flag Coverage Ξ”
main-packages 85.25% <96.25%> (+0.31%) :arrow_up:
web-components 97.25% <ΓΈ> (+0.09%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jun 16 '25 13:06 codecov[bot]

Deploy Preview for carbon-elements ready!

Name Link
Latest commit 61653a572531292c0a748aecaef171c76372cb4f
Latest deploy log https://app.netlify.com/projects/carbon-elements/deploys/68d42b10fa94e600083d0f91
Deploy Preview https://deploy-preview-19666--carbon-elements.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jun 19 '25 00:06 netlify[bot]

It looks like these changes also address build warnings. https://github.com/carbon-design-system/carbon/actions/runs/17166355942/job/48707536559#step:6:218 from the latest merge to main:

> @carbon/react:build

@carbon/react: The exported identifier "TableHeaderTranslationKey" is not declared in Babel's scope tracker
@carbon/react: as a JavaScript value binding, and "@babel/plugin-transform-typescript"
@carbon/react: never encountered it as a TypeScript type declaration.
@carbon/react: It will be treated as a JavaScript value.
@carbon/react: This problem is likely caused by another plugin injecting
@carbon/react: "TableHeaderTranslationKey" without registering it in the scope tracker. If you are the author
@carbon/react:  of that plugin, please use "scope.registerDeclaration(declarationPath)".
@carbon/react: The exported identifier "TableHeaderTranslationArgs" is not declared in Babel's scope tracker
@carbon/react: as a JavaScript value binding, and "@babel/plugin-transform-typescript"
@carbon/react: never encountered it as a TypeScript type declaration.
@carbon/react: It will be treated as a JavaScript value.
@carbon/react: This problem is likely caused by another plugin injecting
@carbon/react: "TableHeaderTranslationArgs" without registering it in the scope tracker. If you are the author
@carbon/react:  of that plugin, please use "scope.registerDeclaration(declarationPath)".
@carbon/icons-vue: [BABEL] Note: The code generator has deoptimised the styling of /home/runner/work/carbon/carbon/packages/icons-vue/virtual:index.js as it exceeds the max of 500KB.
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/createTextComponent.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/Text.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/TextDirection.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/ListBox/index.ts -> src/components/ListBox/ListBox.tsx -> src/components/ListBox/index.ts
@carbon/react: Circular dependency: src/components/UIShell/index.ts -> src/components/UIShell/HeaderPanel.tsx -> src/components/UIShell/Switcher.tsx -> src/components/UIShell/index.ts
@carbon/react: The exported identifier "TableHeaderTranslationKey" is not declared in Babel's scope tracker
@carbon/react: as a JavaScript value binding, and "@babel/plugin-transform-typescript"
@carbon/react: never encountered it as a TypeScript type declaration.
@carbon/react: It will be treated as a JavaScript value.
@carbon/react: This problem is likely caused by another plugin injecting
@carbon/react: "TableHeaderTranslationKey" without registering it in the scope tracker. If you are the author
@carbon/react:  of that plugin, please use "scope.registerDeclaration(declarationPath)".
@carbon/react: The exported identifier "TableHeaderTranslationArgs" is not declared in Babel's scope tracker
@carbon/react: as a JavaScript value binding, and "@babel/plugin-transform-typescript"
@carbon/react: never encountered it as a TypeScript type declaration.
@carbon/react: It will be treated as a JavaScript value.
@carbon/react: This problem is likely caused by another plugin injecting
@carbon/react: "TableHeaderTranslationArgs" without registering it in the scope tracker. If you are the author
@carbon/react:  of that plugin, please use "scope.registerDeclaration(declarationPath)".
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/createTextComponent.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/Text.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/TextDirection.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/ListBox/index.ts -> src/components/ListBox/ListBox.tsx -> src/components/ListBox/index.ts
@carbon/react: Circular dependency: src/components/UIShell/index.ts -> src/components/UIShell/HeaderPanel.tsx -> src/components/UIShell/Switcher.tsx -> src/components/UIShell/index.ts

https://github.com/carbon-design-system/carbon/actions/runs/17023140054/job/48254986230?pr=19666#step:6:218 from https://github.com/carbon-design-system/carbon/commit/4704bc031caf4f780b4bd92220692842a4f3076c, the last commit pushed to this pull request:

> @carbon/react:build

@carbon/icons-vue: [BABEL] Note: The code generator has deoptimised the styling of /home/runner/work/carbon/carbon/packages/icons-vue/virtual:index.js as it exceeds the max of 500KB.
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/createTextComponent.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/Text.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/TextDirection.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/ListBox/index.ts -> src/components/ListBox/ListBox.tsx -> src/components/ListBox/index.ts
@carbon/react: Circular dependency: src/components/UIShell/index.ts -> src/components/UIShell/HeaderPanel.tsx -> src/components/UIShell/Switcher.tsx -> src/components/UIShell/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/createTextComponent.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/Text.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/Text/index.ts -> src/components/Text/TextDirection.tsx -> src/components/Text/index.ts
@carbon/react: Circular dependency: src/components/ListBox/index.ts -> src/components/ListBox/ListBox.tsx -> src/components/ListBox/index.ts
@carbon/react: Circular dependency: src/components/UIShell/index.ts -> src/components/UIShell/HeaderPanel.tsx -> src/components/UIShell/Switcher.tsx -> src/components/UIShell/index.ts

I have another pull request coming to address the circular dependency warnings.

adamalston avatar Aug 24 '25 16:08 adamalston

@adamalston I resolved your conflicts but was unable to update your branch due to permissions (not sure why, I do this all the time and never had an issue before). But once the conflicts are resolved this is good to go.

kennylam avatar Sep 24 '25 16:09 kennylam