Dialog - add showClose #3192
Dialog - add showClose #3192
Conversation
src/incubator/Dialog/types.ts
Outdated
| /** | ||
| * The close button props | ||
| */ | ||
| closeProps?: DialogCloseButton; |
There was a problem hiding this comment.
'buttonProps' (or 'closeButtonProps') is a better name IMO
src/incubator/Dialog/types.ts
Outdated
| */ | ||
| visible?: boolean; | ||
| closeText?: string; |
There was a problem hiding this comment.
'label' is a better name IMO
There was a problem hiding this comment.
Just 'label' this is the type of the Dialog's 'closeButtonProps' prop (at the same time it can be ButtonProps type)...
src/incubator/Dialog/types.ts
Outdated
| */ | ||
| headerProps?: DialogHeaderProps; | ||
| textProps?: TextProps; |
There was a problem hiding this comment.
'labelProps' is a better name IMO
src/incubator/Dialog/types.ts
Outdated
| /** | ||
| * Whether to show the close button or not | ||
| */ | ||
| showClose?: boolean; |
There was a problem hiding this comment.
'visible' (or 'showCloseButton') is a better name IMO
| <TouchableOpacity paddingB-s2 row onPress={close}> | ||
| <Icon source={Assets.icons.xMedium} tintColor={Colors.white} {...closeProps?.iconProps}/> | ||
| <Text recorderTag={'unmask'} text70BO white {...closeProps?.textProps}> | ||
| {closeProps?.closeText || 'Close'} |
There was a problem hiding this comment.
This will need to be configured in private to pass an i18n string
There was a problem hiding this comment.
Yes, that's why I added the props (text + text style + icon)
| } else { | ||
| return ( | ||
| <> | ||
| {headerProps && <DialogHeader {...headerProps}/>} |
There was a problem hiding this comment.
This part can move to a separate render to remove duplications
src/incubator/Dialog/index.tsx
Outdated
| @@ -202,8 +213,7 @@ const Dialog = (props: DialogProps, ref: ForwardedRef<DialogImperativeMethods>) | |||
| <GestureDetector gesture={panGesture}> | |||
| {/* @ts-expect-error should be fixed in version 3.5 (https://github.com/software-mansion/react-native-reanimated/pull/4881) */} | |||
| <View {...containerProps} reanimated style={style} onLayout={onLayout} ref={setRef} testID={testID}> | |||
| {headerProps && <DialogHeader {...headerProps}/>} | |||
| {children} | |||
| <DialogContent headerProps={headerProps} children={children}/> | |||
There was a problem hiding this comment.
This is confusing... You're passing 'headerProps' as a prop to the DialogContent that you got from the hook that sets 'headerProps' in the 'DialogContent' it returns... Why not just pass it to the hook with the other props and set it there, only rendering the returned component in the Dialog?
Same as 'children'...
|
@Inbal-Tish ping 🙏 |
| close: () => void; | ||
| } | ||
|
|
||
| const useDialogCloseButton = (props: InternalDialogCloseButtonProps) => { |
There was a problem hiding this comment.
Please rename it to useDialogContent as you return the whole content and not just the Button
Description
Dialog - add showClose
Changelog
Additional info
None