[DSRN] Added ButtonIcon #386
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
georgewrmarshall
requested changes
Jan 29, 2025
Comment on lines
+101
to
+115
| export const AllIcons: Story = { | ||
| render: () => ( | ||
| <ScrollView> | ||
| <View style={{ flexWrap: 'wrap', flexDirection: 'row', padding: 16 }}> | ||
| {Object.values(IconName).map((iconName) => ( | ||
| <ButtonIconStory | ||
| key={iconName} | ||
| iconName={iconName} | ||
| size={DEFAULT_BUTTONICON_PROPS.size} | ||
| /> | ||
| ))} | ||
| </View> | ||
| </ScrollView> | ||
| ), | ||
| }; |
There was a problem hiding this comment.
nit: This story seems more appropriate for icons.
| ButtonIconProps, | ||
| 'iconName' | 'size' | 'isDisabled' | 'isInverse' | 'isFloating' | ||
| > = { | ||
| iconName: IconName.Add, |
There was a problem hiding this comment.
nit: would close be a more appropriate default icon as it's likely the most used with ButtonIcon
|
|
||
| ### `size` | ||
|
|
||
| Optional prop to control the size of the `ButtonIcon`. |
There was a problem hiding this comment.
nit: add default size
Comment on lines
+23
to
+62
| export type ButtonIconProps = { | ||
| /** | ||
| * Optional prop to control the size of the icon | ||
| * Different sizes map to specific pixel dimensions | ||
| * @default IconSize.Md | ||
| */ | ||
| size?: ButtonIconSize; | ||
| /** | ||
| * Optional prop to specify an icon to show | ||
| */ | ||
| iconName: IconName; | ||
| /** | ||
| * Optional prop to pass additional properties to the icon | ||
| */ | ||
| iconProps?: Partial<IconProps>; | ||
| /** | ||
| * Optional prop that when true, disables the button | ||
| * @default false | ||
| */ | ||
| isDisabled?: boolean; | ||
| /** | ||
| * Optional prop to show the inverse state of the button, which is reserved for buttons on colored backgrounds. | ||
| * @default false | ||
| */ | ||
| isInverse?: boolean; | ||
| /** | ||
| * Optional prop to show the floating/contained state of the button, which is reserved for floating buttons. | ||
| * Note: This prop provides styling only. There is no positioning logic. | ||
| * @default false | ||
| */ | ||
| isFloating?: boolean; | ||
| /** | ||
| * Optional prop to add twrnc overriding classNames. | ||
| */ | ||
| twClassName?: string; | ||
| /** | ||
| * Optional prop to control the style. | ||
| */ | ||
| style?: StyleProp<ViewStyle>; | ||
| } & Omit<PressableProps, 'children'>; |
There was a problem hiding this comment.
possibly blocking: I noticed we don't have a required accessibilityLabel prop for the icon. I'm not very familiar with accessibility best practices for mobile, but I think labeling icon buttons is important. How should we handle the equivalent of aria-label in React Native, and how should it be applied here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds
ButtonIconto the dsrn libraryRelated issues
Fixes: #315
Manual testing steps
yarn storybook:iosfrom rootScreenshots/Recordings
Before
After
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2025-01-29.at.11.59.09.mp4
Pre-merge author checklist
Pre-merge reviewer checklist