-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix: Update border radius of AvatarBase.XS to improve visual cohesion #841
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
base: main
Are you sure you want to change the base?
Conversation
📖 Storybook Preview |
georgewrmarshall
left a comment
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.
Well done on updating both react and react native instances. I've left a couple questions
| import { fileURLToPath } from 'url'; | ||
|
|
||
| /** | ||
| * This function is used to resolve the absolute path of a package. | ||
| * It is needed in projects that are set up within a monorepo. | ||
| * | ||
| * @param value | ||
| */ | ||
| function getAbsolutePath(value: string): any { | ||
| return dirname(require.resolve(join(value, 'package.json'))); | ||
| function getAbsolutePath(value: string): string { | ||
| return value; | ||
| } |
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.
What is the reason for this change?
| string | ||
| > = { | ||
| [AvatarBaseSize.Xs]: 'rounded-sm', // 4px | ||
| [AvatarBaseSize.Xs]: 'rounded-[5px]', // 5px |
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.
Is there any reason why we can't stay within the tailwind system? Is this the only place we should be using a 5px border radius?
Description
This PR refines the
borderRadiusof theAvatarBase.XSfrom4pxto5pxto improve visual cohesion with our typography. At4px, the icons looked too sharp. This change makes our icons feel like they scale more naturally.Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Figma file has been updated here
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Updates AvatarBase XS square radius from 4px to 5px in both web and React Native; simplifies Storybook getAbsolutePath config.
rounded-sm) to 5px (rounded-[5px]) in:packages/design-system-react/src/components/AvatarBase/AvatarBase.constants.tspackages/design-system-react-native/src/components/AvatarBase/AvatarBase.constants.tsapps/storybook-react/.storybook/main.ts: SimplifygetAbsolutePath()to return the value (typedstring) and addfileURLToPathimport.Written by Cursor Bugbot for commit d29beb9. This will update automatically on new commits. Configure here.