add RemoveModal #95
add RemoveModal #95
Conversation
|
One styling thing - the title should be black like so |
| const useStyles = createUseStyles({ | ||
| rbacDeleteIcon: { | ||
| marginRight: 'var(--pf-v5-global--spacer--xs)' | ||
| }, | ||
| }) |
There was a problem hiding this comment.
this should not be needed
| const useStyles = createUseStyles({ | |
| rbacDeleteIcon: { | |
| marginRight: 'var(--pf-v5-global--spacer--xs)' | |
| }, | |
| }) | |
| const useStyles = createUseStyles({ | |
| rbacDeleteIcon: { | |
| marginRight: 'var(--pf-v5-global--spacer--xs)' | |
| }, | |
| }) |
| /** Custom message after confirmation */ | ||
| confirmCheckMessage: string, |
There was a problem hiding this comment.
| /** Custom message after confirmation */ | |
| confirmCheckMessage: string, | |
| /** Custom checkbox message */ | |
| checkboxMessage: string, |
| isOpen, | ||
| confirmButtonLabel, | ||
| withCheckbox=false, | ||
| confirmCheckMessage, |
There was a problem hiding this comment.
| confirmCheckMessage, | |
| checkboxMessage='I understand that this action cannot be undone.', |
...docs/content/extensions/component-groups/examples/RemoveModal/RemoveModalCheckboxExample.tsx
Outdated
Show resolved
Hide resolved
...docs/content/extensions/component-groups/examples/RemoveModal/RemoveModalCheckboxExample.tsx
Outdated
Show resolved
Hide resolved
...ternfly-docs/content/extensions/component-groups/examples/RemoveModal/RemoveModalExample.tsx
Outdated
Show resolved
Hide resolved
|
@InsaneZein just some nitpicking to be consistent with WarningModal as much as possible, thank you! 🙂 |
|
@fhlavac I've added the RemoveModal functionality in WarningModal like we talked about. |
| /** Custom message after confirmation */ | ||
| confirmCheckMessage?: string; |
There was a problem hiding this comment.
I would rename it to checkboxLabel since checkbox uses label as well
| /** Custom message after confirmation */ | |
| confirmCheckMessage?: string; | |
| /** Custom checkbox label */ | |
| checkboxLabel?: string; |
| /** Custom label for the cancel action button */ | ||
| cancelButtonLabel? : string; | ||
| cancelButtonLabel?: string; | ||
| /** Whether Modal requires a checkbox before confirming */ |
There was a problem hiding this comment.
| /** Whether Modal requires a checkbox before confirming */ | |
| /** Whether modal requires a checkbox before confirming */ |
| /** Whether confirm button will show up as red or not */ | ||
| dangerButtonVariant?: boolean; |
There was a problem hiding this comment.
It might be better to pass "confirmButtonVariant" of type ButtonVariant from PF
| variant = ModalVariant.small, | ||
| titleIconVariant = 'warning', | ||
| withCheckbox = false, | ||
| confirmCheckMessage='', |
There was a problem hiding this comment.
We should provide a default message - this is what we use in RBAC remove modals
| confirmCheckMessage='', | |
| checkboxLabel='I understand that this action cannot be undone.', |
| isChecked={checked} | ||
| onChange={() => setChecked(!checked)} | ||
| label={confirmCheckMessage} | ||
| id="remove-modal-check" |
There was a problem hiding this comment.
| id="remove-modal-check" | |
| id="warning-modal-check" |
There was a problem hiding this comment.
Could you please also enhance this example to use custom button labels?
Just to have them present somewhere
| title='Unsaved changes' | ||
| onClose={() => setIsOpen(false)} | ||
| onConfirm={() => setIsOpen(false)} | ||
| withCheckbox={true} |
There was a problem hiding this comment.
| withCheckbox={true} | |
| withCheckbox |
|
@InsaneZein looks great! |
There was a problem hiding this comment.
@InsaneZein looks great! Can you please just add a feat(): prefix to one of the commits so a release triggers? Feel free to merge it then
InsaneZein
force-pushed
the
add-removemodal
branch
from
557dda9 to
5227df7
Compare
|
🎉 This PR is included in version 5.1.0-prerelease.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #92
Add RemoveModal from rbac-ui