Skip to content

chore: migrate IconSize to shared union type (ADR-0003 + ADR-0004)#1048

Draft
georgewrmarshall wants to merge 1 commit intomainfrom
feat/icon-size-shared-migration
Draft

chore: migrate IconSize to shared union type (ADR-0003 + ADR-0004)#1048
georgewrmarshall wants to merge 1 commit intomainfrom
feat/icon-size-shared-migration

Conversation

@georgewrmarshall
Copy link
Copy Markdown
Contributor

@georgewrmarshall georgewrmarshall commented Apr 7, 2026

Description

Migrates IconSize from platform-specific definitions to a shared const object in @metamask/design-system-shared, following ADR-0003 (string unions) and ADR-0004 (centralized types).

Why:

  • React and React Native had divergent IconSize definitions — React used semantic tokens ('xs'/'sm'/'md'/'lg'/'xl'), while React Native used pixel strings ('12'/'16'/'20'/'24'/'32'). This PR unifies them using the semantic token approach.
  • Part 1 of 3 planned Icon migration PRs (IconSize → IconColor → IconName).

What changed:

  • Added IconSize const object (semantic tokens xs/sm/md/lg/xl) to @metamask/design-system-shared
  • Added size?: string to shared IconPropsShared type
  • Removed platform-specific IconSize definitions from both src/types/index.ts files
  • Both platform Icon/index.ts files now export IconSize directly from @metamask/design-system-shared
  • Refactored RN Icon.tsx to use a TWCLASSMAP_ICON_SIZE_DIMENSION class map (same pattern as React) instead of arbitrary w-[${size}px] template interpolation
  • Created Icon.constants.ts in RN package (mirrors existing React pattern)
  • Updated BannerAlert in both platforms to import IconSize from '../Icon' instead of '../../types'
  • Updated AvatarIcon.test.tsx size assertions to use the class map

Related issues

Fixes:

Manual testing steps

  1. Run yarn build — should complete without TypeScript errors
  2. Run yarn test — all 499 React + 811 RN tests should pass
  3. Run yarn lint — should pass clean
  4. In Storybook, verify Icon renders at all 5 sizes (Xs/Sm/Md/Lg/Xl)

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs
  • I've completed the PR template to the best of my ability
  • I've included tests if applicable
  • I've documented my code using JSDoc format if applicable
  • I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Medium risk because it changes exported icon sizing/types across both React and React Native and updates how RN icons compute width/height, which could cause subtle styling or downstream type breakages.

Overview
Unifies IconSize across React and React Native by moving it to @metamask/design-system-shared as a string-union style token (xs/sm/md/lg/xl), removing the platform-specific IconSize definitions from both packages’ types exports.

Refactors icon sizing to use a shared class map: RN Icon now derives dimensions from a new TWCLASSMAP_ICON_SIZE_DIMENSION (mirroring React) instead of interpolating pixel-based tailwind classes, and related consumers/tests (AvatarIcon, BannerAlert, stories/tests) are updated to import IconSize from ../Icon and assert against the new mapping.

Adds shared icon types in design-system-shared (IconColor, IconName, IconSize, IconPropsShared) and updates platform IconProps to extend IconPropsShared.

Reviewed by Cursor Bugbot for commit 68ba9dc. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant