-
Notifications
You must be signed in to change notification settings - Fork 196
feat: delete confirm modal #3033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a19a0ba
530876a
e7228cb
4b3cde8
fd2a23e
8ea5c83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import React from 'react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { IntlProvider } from '@edx/frontend-platform/i18n'; | ||
| import { render, screen } from '@testing-library/react'; | ||
|
|
||
| import TypeXToConfirmModal from './TypeXToConfirmModal'; | ||
|
|
||
| const defaultProps = () => ({ | ||
| label: 'Delete item', | ||
| bodyText: 'Dangerous action', | ||
| confirmLabel: 'Delete', | ||
| cancelLabel: 'Cancel', | ||
| X: 'DELETE', | ||
| confirmPayload: { id: 7 }, | ||
| isOpen: true, | ||
| onConfirm: jest.fn(), | ||
| onCancel: jest.fn(), | ||
| setConfirmPayload: jest.fn(), | ||
| }); | ||
|
|
||
| const renderModal = (props = defaultProps()) => | ||
| render( | ||
| <IntlProvider locale="en" messages={{}}> | ||
| <TypeXToConfirmModal {...props} /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| describe('TypeXToConfirmModal', () => { | ||
| it('renders the required confirmation phrase with strong emphasis', () => { | ||
| renderModal(); | ||
|
|
||
| expect(screen.getByText('DELETE', { selector: 'strong' })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('keeps the destructive confirm button disabled until the typed value exactly matches the required confirmation phrase', async () => { | ||
| const user = userEvent.setup(); | ||
| renderModal(); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| const confirmButton = screen.getByRole('button', { name: 'Delete' }); | ||
|
|
||
| expect(confirmButton).toBeDisabled(); | ||
| await user.type(input, 'DEL'); | ||
| expect(confirmButton).toBeDisabled(); | ||
| await user.type(input, 'ETE'); | ||
| expect(confirmButton).toBeEnabled(); | ||
| }); | ||
|
|
||
| it('does not enable confirmation for partial, differently cased, or whitespace-padded confirmation text', async () => { | ||
| const user = userEvent.setup(); | ||
| renderModal(); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| const confirmButton = screen.getByRole('button', { name: 'Delete' }); | ||
|
|
||
| for (const value of ['DEL', 'delete', ' DELETE', 'DELETE ', ' Delete ']) { | ||
| await user.clear(input); | ||
| await user.type(input, value); | ||
| expect(confirmButton).toBeDisabled(); | ||
| } | ||
| }); | ||
|
|
||
| it('requires explicit activation of the enabled destructive confirm button', async () => { | ||
| const user = userEvent.setup(); | ||
| const props = defaultProps(); | ||
| renderModal(props); | ||
|
|
||
| const input = screen.getByRole('textbox'); | ||
| const confirmButton = screen.getByRole('button', { name: 'Delete' }); | ||
|
|
||
| await user.click(input); | ||
| await user.keyboard('{Enter}'); | ||
| expect(props.onConfirm).not.toHaveBeenCalled(); | ||
|
|
||
| await user.type(input, 'DELETE'); | ||
| await user.keyboard('{Enter}'); | ||
| expect(props.onConfirm).not.toHaveBeenCalled(); | ||
|
|
||
| await user.click(confirmButton); | ||
| expect(props.onConfirm).toHaveBeenCalledWith(props.confirmPayload); | ||
| }); | ||
|
|
||
| it('resets confirmation state when the modal closes so a reopened dialog starts disabled again', async () => { | ||
| const user = userEvent.setup(); | ||
| const props = defaultProps(); | ||
| const { rerender } = renderModal(props); | ||
|
|
||
| await user.type(screen.getByRole('textbox'), 'DELETE'); | ||
| expect(screen.getByRole('button', { name: 'Delete' })).toBeEnabled(); | ||
|
|
||
| rerender( | ||
| <IntlProvider locale="en" messages={{}}> | ||
| <TypeXToConfirmModal {...props} isOpen={false} /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| rerender( | ||
| <IntlProvider locale="en" messages={{}}> | ||
| <TypeXToConfirmModal {...props} isOpen /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled(); | ||
| }); | ||
|
|
||
| it('clears the provided confirm payload when the modal is closed without confirming', () => { | ||
| const props = defaultProps(); | ||
| const { rerender } = renderModal(props); | ||
|
|
||
| rerender( | ||
| <IntlProvider locale="en" messages={{}}> | ||
| <TypeXToConfirmModal {...props} isOpen={false} /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| expect(props.setConfirmPayload).toHaveBeenCalledWith(null); | ||
| expect(props.onConfirm).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import React, { useEffect } from 'react'; | ||
| import { | ||
| ActionRow, | ||
| Button, | ||
| Card, | ||
| Form, | ||
| Icon, | ||
| ModalDialog, | ||
| } from '@openedx/paragon'; | ||
| import { WarningFilled } from '@openedx/paragon/icons'; | ||
| import { useIntl } from '@edx/frontend-platform/i18n'; | ||
| import messages from './messages'; | ||
|
|
||
| interface TypeXToConfirmModalProps { | ||
| label: string; | ||
| bodyText: string | React.ReactNode; | ||
| confirmLabel: string; | ||
| cancelLabel: string; | ||
| X: string; | ||
| confirmPayload?: Record<string, any> | null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addresses #3024 (comment) Thank you! |
||
| isOpen: boolean; | ||
| onConfirm: (confirmPayload?: Record<string, any> | null) => void; | ||
| onCancel: () => void; | ||
| setConfirmPayload?: (confirmPayload: Record<string, any> | null) => void; | ||
| } | ||
|
|
||
| const TypeXToConfirmModal: React.FC<TypeXToConfirmModalProps> = ({ | ||
| label, | ||
| X, | ||
| bodyText, | ||
| confirmLabel, | ||
| cancelLabel, | ||
| isOpen, | ||
| confirmPayload, | ||
| onConfirm, | ||
| onCancel, | ||
| setConfirmPayload, | ||
| }) => { | ||
| const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); | ||
| const intl = useIntl(); | ||
|
|
||
| const handleConfirm = () => { | ||
| if (!confirmedByTyping) { return; } | ||
| setConfirmedByTyping(false); | ||
| onConfirm(confirmPayload); | ||
| }; | ||
|
|
||
| const handleCancel = () => { | ||
| setConfirmedByTyping(false); | ||
| onCancel(); | ||
| }; | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.value === X) { | ||
| setConfirmedByTyping(true); | ||
| } else { | ||
| setConfirmedByTyping(false); | ||
| } | ||
| }; | ||
|
|
||
| // Don't remove. This is necessary to prevent an old state from erroneously enabling the confirm button | ||
| useEffect(() => { | ||
| if (!isOpen) { | ||
| setConfirmedByTyping(false); | ||
| if (setConfirmPayload) { | ||
| setConfirmPayload(null); | ||
| } | ||
| } | ||
| }, [X, isOpen, confirmPayload, setConfirmPayload]); | ||
|
|
||
| return ( | ||
| <ModalDialog | ||
| title={label} | ||
| isOpen={isOpen} | ||
| onClose={handleCancel} | ||
| isOverflowVisible | ||
| > | ||
| <ModalDialog.Header> | ||
| <ModalDialog.Title>{label}</ModalDialog.Title> | ||
| </ModalDialog.Header> | ||
| <ModalDialog.Body> | ||
| <Card className="bg-warning-100"> | ||
| <Card.Section> | ||
| <div className="d-flex align-items-start mb-2"> | ||
| <Icon src={WarningFilled} className="text-warning-500 mr-2" /> | ||
| <div className="small">{bodyText}</div> | ||
| </div> | ||
| </Card.Section> | ||
| </Card> | ||
| <div className="mt-3"> | ||
| <div> | ||
| {intl.formatMessage(messages.typeToConfirmInstruction, { | ||
| X, | ||
| strong: (chunks: React.ReactNode) => <strong>{chunks}</strong>, | ||
| })} | ||
|
Comment on lines
+92
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addresses #3024 (comment) Thank you! |
||
| </div> | ||
| <Form.Control | ||
| onChange={handleChange} | ||
| className="mt-4" | ||
| /> | ||
| </div> | ||
| </ModalDialog.Body> | ||
| <ModalDialog.Footer> | ||
| <ActionRow> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addresses #3024 (comment) Thank you! |
||
| <Button variant="tertiary" onClick={handleCancel}> | ||
| {cancelLabel} | ||
| </Button> | ||
| <Button onClick={handleConfirm} disabled={!confirmedByTyping} variant="danger"> | ||
| {confirmLabel} | ||
| </Button> | ||
| </ActionRow> | ||
| </ModalDialog.Footer> | ||
| </ModalDialog> | ||
| ); | ||
| }; | ||
|
|
||
| export default React.memo(TypeXToConfirmModal); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { defineMessages } from '@edx/frontend-platform/i18n'; | ||
|
|
||
| const messages = defineMessages({ | ||
| typeToConfirmInstruction: { | ||
| id: 'course-authoring.generic.type-to-confirm-instruction', | ||
| defaultMessage: 'Type <strong>{X}</strong> to confirm', | ||
| }, | ||
| }); | ||
|
|
||
| export default messages; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| import React from 'react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { IntlProvider } from '@edx/frontend-platform/i18n'; | ||
| import { render, screen, within } from '@testing-library/react'; | ||
|
|
||
| import DeleteModal from './DeleteModal'; | ||
|
|
||
| const createRow = (rowData) => | ||
| ({ | ||
| id: String(rowData.id), | ||
| original: rowData, | ||
| }) as any; | ||
|
|
||
| const leafRowData = { | ||
| id: 101, | ||
| value: 'leaf tag', | ||
| depth: 0, | ||
| childCount: 0, | ||
| subRows: [], | ||
| }; | ||
|
|
||
| const nestedRowData = { | ||
| id: 201, | ||
| value: 'parent tag', | ||
| depth: 0, | ||
| childCount: 1, | ||
| subRows: [ | ||
| { | ||
| id: 202, | ||
| value: 'child tag', | ||
| depth: 1, | ||
| childCount: 1, | ||
| subRows: [ | ||
| { | ||
| id: 203, | ||
| value: 'grandchild tag', | ||
| depth: 2, | ||
| childCount: 0, | ||
| subRows: [], | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| const defaultProps = (overrides = {}) => ({ | ||
| isOpen: true, | ||
| row: createRow(leafRowData), | ||
| setIsOpen: jest.fn(), | ||
| setRow: jest.fn(), | ||
| handleDeleteRow: jest.fn(), | ||
| ...overrides, | ||
| }); | ||
|
|
||
| const renderDeleteModal = (props = defaultProps()) => | ||
| render( | ||
| <IntlProvider locale="en" messages={{}}> | ||
| <DeleteModal {...(props as any)} /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| describe('DeleteModal', () => { | ||
| it('renders a singular delete title and "Delete Tag" action label when the selected row has no descendants', () => { | ||
| renderDeleteModal(); | ||
|
|
||
| const dialog = screen.getByRole('dialog'); | ||
| expect(dialog).toHaveTextContent('Delete "leaf tag"'); | ||
| expect(dialog).toHaveTextContent('Warning! You are about to delete 1 tag(s).'); | ||
| expect(dialog).toHaveTextContent('Type DELETE to confirm'); | ||
| expect(within(dialog).getByRole('button', { name: 'Delete Tag' })).toBeDisabled(); | ||
| expect(within(dialog).getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders a plural delete action label and descendant warning copy when the selected row has one or more descendants', () => { | ||
| renderDeleteModal(defaultProps({ row: createRow(nestedRowData) })); | ||
|
|
||
| const dialog = screen.getByRole('dialog'); | ||
| expect(dialog).toHaveTextContent('Delete "parent tag"'); | ||
| expect(dialog).toHaveTextContent('Warning! You are about to delete a tag containing sub-tags.'); | ||
| expect(dialog).toHaveTextContent('If you proceed, 3 tags will be deleted.'); | ||
| expect(dialog).toHaveTextContent('Type DELETE ALL 3 TAGS to confirm'); | ||
| expect(within(dialog).getByRole('button', { name: 'Delete Tags' })).toBeDisabled(); | ||
| }); | ||
|
|
||
| it('computes the required confirmation phrase from the recursive descendant count instead of only the immediate child count', () => { | ||
| renderDeleteModal(defaultProps({ row: createRow(nestedRowData) })); | ||
|
|
||
| const dialog = screen.getByRole('dialog'); | ||
| expect(dialog).toHaveTextContent('If you proceed, 3 tags will be deleted.'); | ||
| expect(dialog).toHaveTextContent('Type DELETE ALL 3 TAGS to confirm'); | ||
| expect(dialog).not.toHaveTextContent('DELETE ALL 2 TAGS'); | ||
| }); | ||
|
|
||
| it('calls handleDeleteRow with the dialog row context and then closes and clears the dialog state on confirm', async () => { | ||
| const user = userEvent.setup(); | ||
| const row = createRow(leafRowData); | ||
| const handleDeleteRow = jest.fn(); | ||
| const setIsOpen = jest.fn(); | ||
| const setRow = jest.fn(); | ||
|
|
||
| renderDeleteModal(defaultProps({ | ||
| row, | ||
| handleDeleteRow, | ||
| setIsOpen, | ||
| setRow, | ||
| })); | ||
|
|
||
| await user.type(screen.getByRole('textbox'), 'DELETE'); | ||
| await user.click(screen.getByRole('button', { name: 'Delete Tag' })); | ||
|
|
||
| expect(handleDeleteRow).toHaveBeenCalledWith(row); | ||
| expect(setIsOpen).toHaveBeenCalledWith(false); | ||
| expect(setRow).toHaveBeenCalledWith(null); | ||
| }); | ||
|
|
||
| it('closes and clears the dialog context on cancel without invoking deletion', async () => { | ||
| const user = userEvent.setup(); | ||
| const handleDeleteRow = jest.fn(); | ||
| const setIsOpen = jest.fn(); | ||
| const setRow = jest.fn(); | ||
|
|
||
| renderDeleteModal(defaultProps({ | ||
| handleDeleteRow, | ||
| setIsOpen, | ||
| setRow, | ||
| })); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: 'Cancel' })); | ||
|
|
||
| expect(handleDeleteRow).not.toHaveBeenCalled(); | ||
| expect(setIsOpen).toHaveBeenCalledWith(false); | ||
| expect(setRow).toHaveBeenCalledWith(null); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addresses #3024 (comment)
Thank you!