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

Spike: i18n coverage/test cases in Storybook #3368

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rise-erpelding
Copy link
Collaborator

@rise-erpelding rise-erpelding commented Nov 5, 2024

Description

This work is part of a spike (CSS-849) to investigate how we might expand Storybook stories and test coverage to include more non-English examples. This draft PR builds upon some of the existing work done in #2916 but with more consistent, controlled language examples, which better allows for testing of text strings that are known to be problematic and also allows for visual regression testing using consistent, repeatable examples.

In order to explore how this might be done, a story ("With Locale Text") has been added to the text field component. This story makes use of an updated language picker and changes the text in the story to match the chosen locale. Translations are currently coming from a single .json file with all locales in the .storybook directory, although this can be adjusted, for instance if we decide that we want translations to be housed somewhere else, or if we want a different translation file for each locale (similar to the React team's approach).

When switching to testing preview, which gives an idea of how VRTs might look, sample text from each locale is displayed. This is an example of one way that we might expand i18 coverage and add test cases for components.

Text strings in this example were generated by AI and may not be accurate. We can hunt down more accurate test strings when we're in touch with the team that does translations.

Screenshots

Text field With Locale Text story with language picker set to Thai, as seen in the preview:
image

Storybook Testing Preview:
image

Considerations

I've asked in Slack to learn the specifics on which locales to cover, which components to cover, and how to obtain the best translations. Based on that, here are my thoughts on the best way forward:

  • Which locales: Cassondra's previous work included a handful of locales including RTL, CJK, and Thai, a language for which we've fixed a couple of bugs in picker and action button just in the last 6 months. It might be better to start with these while we're still refining our methods for adding internationalization to components, and expand to include other locales later. React has a whole list of supported locales.
  • Which components: The React team recommended that anything with internal strings be internationalized - this generally amounts to most components, but components where the text comes from another component that's already being tested have been left out. After testing our components with one of those Thai strings that was used for testing most recently, here are some components we might consider:
    • Higher priority (these have been fixed before due to cutoff issues, or are experiencing cutoff currently with the test string):
      • Asset list
      • Field label
      • Menu
      • Miller columns
      • Search
      • Tree view
      • Action button (has already had at least 1 fix)
      • Picker (has already had at least 1 fix)
      • Text field/text area (has already had at least 1 fix)
      • Typography (we display internationalized text on our Docs page here but it would be neat to build upon this)
    • Lower priority (these did mostly ok with the test string):
      • Asset card
      • Accordion
      • Action bar
      • Alert banner
      • Alert dialog
      • Badge
      • Button
      • Breadcrumbs
      • Calendar
      • Card
      • Checkbox
      • Coach mark
      • Combobox
      • Contextual help
      • Dial
      • Dialog
      • Help text
      • Illustrated message
      • Inline alert
      • Logic button (this had cutoff problems, but the text inside of this particular button is really limited so may be less of an issue)
      • Pagination
      • Radio
      • Side nav
      • Status light
      • Steplist
      • Switch
      • Table
      • Tabs
      • Tag
      • Toast
      • Tooltip
      • Well

Potential breakdown of work

Part 1: adding internationalization to higher-priority components

For each component, we would need to:

  • Obtain translations, either from React or from the translations team
  • Add the translations to the translations file
  • Add a story for the component to include different translations to be displayed for different locales
  • Add VRTs for each locale
  • If any component appears problematic, write a follow-up card to find a fix for the issue (for instance, to prevent cutoff, or wrapping, etc.)

We could split into multiple cards by component, perhaps 3-4 components per card - this would amount to something like 3 Jira tickets.

Part 2: expanding locales to include the ones it's recommended we support

This would likely be 1 Jira ticket

Part 3: adding internationalization to lower-priority components

This would need to be split into multiple Jira tickets, but given that there are many components here, it might be better to keep all of them listed on one Jira ticket for now with the intention of breaking them up when we're closer to being ready to pick up this work.

Copy link

changeset-bot bot commented Nov 5, 2024

⚠️ No Changeset found

Latest commit: 9ebee06

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@rise-erpelding rise-erpelding added i18n Things like `[dir]` that matter for i18n do not merge A flag for a branch indicating it should not be merged. research Flag for research needed or in progress wip This is a work in progress, don't judge. labels Nov 5, 2024
@rise-erpelding rise-erpelding self-assigned this Nov 5, 2024
Copy link
Contributor

github-actions bot commented Nov 5, 2024

File metrics

Summary

Total size: 4.26 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@rise-erpelding rise-erpelding added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Nov 5, 2024
@rise-erpelding rise-erpelding force-pushed the rise-erpelding/feat-localization-storybook branch from b69100b to c117928 Compare November 5, 2024 15:01
Copy link
Contributor

github-actions bot commented Nov 5, 2024

🚀 Deployed on https://pr-3368--spectrum-css.netlify.app

@rise-erpelding rise-erpelding force-pushed the rise-erpelding/feat-localization-storybook branch 3 times, most recently from 15b9603 to 4628945 Compare November 12, 2024 15:38
@rise-erpelding rise-erpelding force-pushed the rise-erpelding/feat-localization-storybook branch from 4628945 to 9ebee06 Compare November 14, 2024 12:17
{ value: "ko", title: "Korean", right: "한국어" },
{ value: "ar", title: "Arabic", right: "عربي" },
{ value: "zh", title: "Chinese", right: "中文" },
{ value: "fa", title: "Persian", right: "فارسی" },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not see Persian on the list of supported locales, we certainly still could support it though!

@rise-erpelding rise-erpelding added ready-for-review and removed wip This is a work in progress, don't judge. labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge A flag for a branch indicating it should not be merged. i18n Things like `[dir]` that matter for i18n ready-for-review research Flag for research needed or in progress skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants