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

Icon: Set up JSON for RTL icon list #3983

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rondevera
Copy link

@rondevera rondevera commented Feb 7, 2025

Summary

  • Move the list of icons to a JSON file, and import it into
    RTLIconList.tsx. Test manually via the docs site.
  • Update /foundations/international_design/rtl_guidelines/iconography
    to use this same icon list.

Why?

We define a list of icons in RTLIconList.tsx that we automatically
flip for right-to-left content. This works for web, but we need to make
this list available for iOS and Android.

Screenshots

Before (Classic theme)

Screenshot from /foundations/international_design/rtl_guidelines/iconography:

Original "Directional icons that need to be mirrored" section, Classic theme

After (Classic theme)

New "Directional icons" section with additional icons, Classic theme

After (VR theme)

New "Directional icons" section with additional icons, VR theme

Links

Checklist

  • Added unit tests → Used existing tests
  • Added documentation + accessibility tests → Used existing tests
  • Verified accessibility: keyboard & screen reader interaction
  • Checked dark mode, responsiveness, and right-to-left support
  • Checked stakeholder feedback (e.g. Gestalt designers, relevant feature teams)

Problem:

We define a list of icons in `RTLIconList.tsx` that we automatically
flip for right-to-left content. This works for web, but we need to make
this list available for iOS and Android.

Solution:

- Move the list of icons to a JSON file, and import it into
  `RTLIconList.tsx`. Test manually via the docs site.
- Update `/foundations/international_design/rtl_guidelines/iconography`
  to use this same icon list.

Jira: GESTALT-8856
@rondevera rondevera added the patch release Patch release label Feb 7, 2025
Copy link

netlify bot commented Feb 7, 2025

Deploy Preview for gestalt ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 53fbb14
🔍 Latest deploy log https://app.netlify.com/sites/gestalt/deploys/67a55b02940b25000865bf55
😎 Deploy Preview https://deploy-preview-3983--gestalt.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 site configuration.

@@ -1,38 +1,14 @@
import rtlIconListJson from 'gestalt-design-tokens/src/utils/rtlIconList.json';
Copy link
Author

Choose a reason for hiding this comment

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

I initially added this JSON file to gestalt/src/Icon/, but got error TS6307 from @rollup/plugin-typescript. This is potentially related to our use of "composite": true in packages/gestalt/tsconfig.json.

const swapOnRtlIconNames = rtlIconListJson.swapOnRtlIconNames as ReadonlyArray<
IconName | undefined
>;
const rtlIcons = [...flipOnRtlIconNames, ...swapOnRtlIconNames].sort();
Copy link
Author

Choose a reason for hiding this comment

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

Replaced the static icon list with a dynamic one so we can keep our docs up to date, and sorted the merged result to put list-numbered in the middle of the list.

One downside is that this version doesn't group icon pairs (e.g., arrow-back and arrow-forward) unless we sort the source JSON file manually.

@@ -0,0 +1,32 @@
{
Copy link
Author

Choose a reason for hiding this comment

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

Do we prefer camelcase, hyphens, or another format for JSON files like this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see why you put it here. I have the feeling I don't like this here as it doesn't relate to tokens.

We do need an icon repo... gestalt-design-icons.... and it should go there. One day!

Ideally we could handle the error TS6307. I'm ok keeping it here for now if we set a task for moving it out eventually.

I don't think we want to create a utils kitchen drawer within this repo.

@rondevera rondevera marked this pull request as ready for review February 7, 2025 02:57
@rondevera rondevera requested a review from a team as a code owner February 7, 2025 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch release Patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants