[DSRN] Add ButtonBase component #384
Conversation
| @@ -40,7 +40,9 @@ const ButtonAnimated = ({ | |||
| }; | |||
|
|
|||
| return ( | |||
| <Animated.View style={scaleStyle}> | |||
| <Animated.View | |||
| style={[scaleStyle, { alignItems: 'center', justifyContent: 'center' }]} | |||
There was a problem hiding this comment.
Updating this to make sure the scale animation works as intended
There was a problem hiding this comment.
non-blocking: You could make this primitive more reusable by moving the scale animation to ButtonBase, depending on the values used in ButtonIcon.
| @@ -40,7 +40,9 @@ const ButtonAnimated = ({ | |||
| }; | |||
|
|
|||
| return ( | |||
| <Animated.View style={scaleStyle}> | |||
| <Animated.View | |||
| style={[scaleStyle, { alignItems: 'center', justifyContent: 'center' }]} | |||
There was a problem hiding this comment.
non-blocking: You could make this primitive more reusable by moving the scale animation to ButtonBase, depending on the values used in ButtonIcon.
| testID: 'end-icon', | ||
| }, | ||
| onPress: () => { | ||
| console.log('Button pressed'); |
There was a problem hiding this comment.
non-blocking: We shouldn't use a console.log as default props. You could consider using a no-op function (() => {}) or removing the default onPress handler entirely.
| export enum ButtonBaseSize { | ||
| /** | ||
| * Represents a small button size (32px). | ||
| */ | ||
| Sm = '32', | ||
| /** | ||
| * Represents a medium button size (40px). | ||
| */ | ||
| Md = '40', | ||
| /** | ||
| * Represents a large button size (48px). | ||
| */ | ||
| Lg = '48', | ||
| } |
There was a problem hiding this comment.
non-blocking: What is the reason to use strings instead of numbers here? Using numbers may be more appropriate for size values like this and it would prevent type coercion.
|
|
||
| ### `size` | ||
|
|
||
| Optional prop to control the size of the `ButtonBase`. |
There was a problem hiding this comment.
non-blocking: could add it defaults to size large here
| > | ||
| <View | ||
| style={tw`absolute inset-0 flex items-center justify-center opacity-${ | ||
| isLoading ? '100' : '0' |
There was a problem hiding this comment.
nit: it could be cleaner to use the whole classname here
isLoading ? 'opacity-100' : 'opacity-0'
Description
This PR adds the
ButtonBasecomponent to thePrimitivesfolderRelated issues
Fixes: #176
Manual testing steps
yarn storybook:iosfrom rootScreenshots/Recordings
Before
After
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2025-01-29.at.10.11.52.mp4
Pre-merge author checklist
Pre-merge reviewer checklist