-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix: align ButtonIcon icon sizes and border radius with Figma design #873
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
Conversation
📖 Storybook Preview |
📖 Storybook Preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
packages/design-system-react-native/src/components/ButtonIcon/ButtonIcon.tsx
Show resolved
Hide resolved
📖 Storybook Preview |
63ad74e to
518c34d
Compare
📖 Storybook Preview |
| [ButtonIconSize.Lg]: IconSize.Lg, | ||
| [ButtonIconSize.Sm]: IconSize.Md, | ||
| [ButtonIconSize.Md]: IconSize.Lg, | ||
| [ButtonIconSize.Lg]: IconSize.Xl, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Icon size mapping updated to align with Figma design. ButtonIcon.Sm now uses Icon.Md (20px), ButtonIcon.Md uses Icon.Lg (24px), and ButtonIcon.Lg uses Icon.Xl (32px). Previously icons were one size smaller than intended.
| // Non-floating styles | ||
| !isFloating && [ | ||
| 'rounded bg-transparent', | ||
| 'rounded-lg bg-transparent', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Border radius updated from 4px to 8px for non-floating ButtonIcon variants. Floating variants remain fully rounded. This matches the updated Figma design specifications.
| [ButtonIconSize.Lg]: IconSize.Lg, | ||
| [ButtonIconSize.Sm]: IconSize.Md, | ||
| [ButtonIconSize.Md]: IconSize.Lg, | ||
| [ButtonIconSize.Lg]: IconSize.Xl, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same icon size mapping update for React Native to maintain consistency across platforms. Verified against Figma design using Figma MCP.
| items-center justify-center | ||
| ${TWCLASSMAP_BUTTONICON_SIZE_DIMENSION[size]} | ||
| ${isFloating ? 'rounded-full' : 'rounded-sm'} | ||
| ${isFloating ? 'rounded-full' : 'rounded-lg'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Border radius updated to 8px for React Native non-floating variants. This aligns with the React implementation and Figma design.
| 'h-8', | ||
| 'w-8', | ||
| 'rounded', | ||
| 'rounded-lg', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test updated to expect rounded-lg instead of rounded to match the new 8px border radius for non-floating ButtonIcon.
| const { result } = renderHook(() => useTailwind()); | ||
| const tw = result.current; | ||
| const expected = tw`items-center justify-center ${TWCLASSMAP_BUTTONICON_SIZE_DIMENSION[ButtonIconSize.Md]} rounded-sm bg-transparent opacity-100`; | ||
| const expected = tw`items-center justify-center ${TWCLASSMAP_BUTTONICON_SIZE_DIMENSION[ButtonIconSize.Md]} rounded-lg bg-transparent opacity-100`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test expectations updated throughout to reflect the new rounded-lg class for 8px border radius. All non-floating ButtonIcon tests now expect rounded-lg instead of rounded.
Description
This PR aligns the ButtonIcon component implementations in both React and React Native with the Figma design specifications. The icon sizes were misaligned across all three size variants, and the React Native implementation had an incorrect border radius for non-floating states.
What is the reason for the change?
The ButtonIcon component was using incorrect icon sizes that didn't match the Figma design. For example, ButtonIcon.Md was using Icon.Md (20px) when it should have been using Icon.Lg (24px) according to the Figma layers.
What is the improvement/solution?
Changes:
Related issues
Fixes: N/A (design alignment with Figma)
Manual testing steps
Screenshots/Recordings
Before
Icon sizes were smaller than designed:
React Native had 2px border radius (should be 4px)
beforemobile720.mov
before720.mov
After
Icon sizes match Figma:
mobileafter720.mov
after720.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes shared
ButtonIconvisuals (icon size mapping and non-floating corner radius) in both React and React Native, which can cause widespread UI/layout diffs in consuming apps. No security or data-handling logic is affected.Overview
Updates
ButtonIconin bothdesign-system-reactanddesign-system-react-nativeto better match design specs by increasing the renderedIconsize for eachButtonIconSize(Sm→Md,Md→Lg,Lg→Xl).Also adjusts non-floating
ButtonIconstyling to use larger rounding (rounded-lginstead of the previous smaller radius), and updates corresponding unit tests to assert the new Tailwind classes.Written by Cursor Bugbot for commit 518c34d. This will update automatically on new commits. Configure here.