feat(WarningModal): add text confirmation option #540
Conversation
| /** Whether modal requires a text confirmation */ | ||
| textConfirmation?: TextInputProps; | ||
| /** Label for the text confirmation */ | ||
| textConfirmationLabel?: (deleteName?: string) => ReactNode; | ||
| /** Text the user should type to confirm selection when using textConfirmation */ | ||
| deleteName?: string; |
There was a problem hiding this comment.
| /** Whether modal requires a text confirmation */ | |
| textConfirmation?: TextInputProps; | |
| /** Label for the text confirmation */ | |
| textConfirmationLabel?: (deleteName?: string) => ReactNode; | |
| /** Text the user should type to confirm selection when using textConfirmation */ | |
| deleteName?: string; | |
| /** Confirmation text input props */ | |
| confirmationInputProps?: TextInputProps; | |
| /** Label for the text confirmation input */ | |
| confirmationInputLabel?: (deleteName?: string) => ReactNode; | |
| /** Text the user should type to confirm selection when using confirmation input */ | |
| confirmationText?: string; |
how about renaming the props like so? Mabe it's a little bit more descriptive
| const isConfirmButtonDisabled = () => { | ||
| if(withCheckbox && textConfirmation) { | ||
| return !checked || !textConfirmed; | ||
| } | ||
| if(withCheckbox && !textConfirmation) { | ||
| return !checked; | ||
| } | ||
| if(!withCheckbox && textConfirmation) { | ||
| return !textConfirmed; | ||
| } | ||
| else {return false;} | ||
| } |
There was a problem hiding this comment.
| const isConfirmButtonDisabled = () => { | |
| if(withCheckbox && textConfirmation) { | |
| return !checked || !textConfirmed; | |
| } | |
| if(withCheckbox && !textConfirmation) { | |
| return !checked; | |
| } | |
| if(!withCheckbox && textConfirmation) { | |
| return !textConfirmed; | |
| } | |
| else {return false;} | |
| } | |
| const isConfirmButtonDisabled = React.useMemo(() => { | |
| if (withCheckbox) { | |
| return !checked || (textConfirmation && !textConfirmed); | |
| } | |
| return textConfirmation ? !textConfirmed : false; | |
| }, [checked, textConfirmed, withCheckbox, textConfirmation]); |
| setChecked(false); | ||
| setInputValue(''); |
There was a problem hiding this comment.
is this cleanup needed? it causes unnecessary re-renders.
|
|
||
| ### Warning modal with a text confirmation | ||
|
|
||
| To confirm that a user wishes to initiate a selected action, you can add a text confirmation. |
There was a problem hiding this comment.
| To confirm that a user wishes to initiate a selected action, you can add a text confirmation. | |
| To ensure the user intentionally initiates a selected action, you can add a confirmation text input. |
| cancelButtonLabel='No' | ||
| onClose={() => setIsOpen(false)} | ||
| onConfirm={() => setIsOpen(false)} | ||
| textConfirmation={{ type: 'text', isRequired: true }} |
There was a problem hiding this comment.
maybe these two props could be set as default and made overwritable using custom input props
There was a problem hiding this comment.
If we wanted to do this, I think we'd have to add something like a withTextConfirmation boolean prop for this to work.
|
@InsaneZein just some little suggestions. Otherwise, it looks great! |
| ouiaId={`${ouiaId}-confirmation-text-input`} | ||
| value={inputValue} | ||
| onChange={(_e, value) => setInputValue(value)} | ||
| {...confirmationInputProps} |
There was a problem hiding this comment.
@InsaneZein for the input default props mentioned above, we could do this.
What do you think?
| {...confirmationInputProps} | |
| {{ type: 'text', isRequired: true, ...confirmationInputProps}} |
| {confirmationInputProps ? ( | ||
| <Flex direction={{ default: 'column' }} spaceItems={{ default: 'spaceItemsSm' }}> | ||
| <FlexItem> | ||
| {confirmationInputProps && confirmationInputLabel(deleteNameSanitized)} |
There was a problem hiding this comment.
| {confirmationInputProps && confirmationInputLabel(deleteNameSanitized)} | |
| {confirmationInputLabel(deleteNameSanitized)} |
the check is probably not needed, since you have it above
| /** Label for the text confirmation input */ | ||
| confirmationInputLabel?: (deleteName?: string) => ReactNode; |
There was a problem hiding this comment.
not sure if this one has to be a function - if the user of our component uses this prop, he can pass the confirmation text directly, not using another prop. I would just make it a ReactNode
| <Stack hasGutter> | ||
| <StackItem>{children}</StackItem> | ||
| <StackItem> | ||
| {confirmationInputProps ? ( |
There was a problem hiding this comment.
| {confirmationInputProps ? ( | |
| {confirmationText ? ( |
we probably want to show the confirmation input only if the text is passed, not necessarily the input props
| confirmationInputLabel = 'Type to confirm', | ||
| confirmationText, |
There was a problem hiding this comment.
| confirmationInputLabel = 'Type to confirm', | |
| confirmationText, | |
| confirmationText, | |
| confirmationInputLabel = <>Type <strong>{confirmationText} </strong> to confirm the action:</>, |
I would keep your previous default there - it makes sense to have the confirmation text present
|
🎉 This PR is included in version 6.2.0-prerelease.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #411
for RHCLOUD-36638: adds a text confirmation option to the Warning Modal