Skip to content

Commit

Permalink
fix(Modal): call onClose on close (#2622)
Browse files Browse the repository at this point in the history
reported on slack
  • Loading branch information
Barsnes authored Oct 16, 2024
1 parent 41d5d23 commit 16b2988
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/kind-eyes-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@digdir/designsystemet-react": patch
---

Modal: Fix `onClose` not being called
2 changes: 1 addition & 1 deletion packages/react/src/components/Modal/Modal.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export const ModalWithForm: StoryFn<typeof Modal> = () => {
return (
<Modal.Context>
<Modal.Trigger>Open Modal</Modal.Trigger>
<Modal ref={modalRef} onClose={() => setInput('')}>
<Modal ref={modalRef} onClose={() => setInput('')} backdropClose>
<Heading size='xs' style={{ marginBottom: 'var(--ds-spacing-2)' }}>
Modal med skjema
</Heading>
Expand Down
18 changes: 18 additions & 0 deletions packages/react/src/components/Modal/Modal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,22 @@ describe('Modal', () => {
await render({ children, open: true });
expect(screen.getByText(children)).toBeInTheDocument();
});

it('should call onClose when the modal is closed with ESC', async () => {
const onClose = vi.fn();
await render({ open: true, onClose });
const dialog = screen.getByRole('dialog');
await act(async () => await userEvent.type(dialog, '{Escape}'));

expect(onClose).toHaveBeenCalledTimes(1);
});

it('should call onClose when the modal is closed with the close button', async () => {
const onClose = vi.fn();
await render({ open: true, onClose });
const closeButton = screen.getByRole('button', { name: CLOSE_LABEL });
await act(async () => await userEvent.click(closeButton));

expect(onClose).toHaveBeenCalledTimes(1);
});
});
8 changes: 8 additions & 0 deletions packages/react/src/components/Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>(function Modal(
};
}, [backdropClose]);

/* handle closing */
useEffect(() => {
const handleClose = () => onClose?.();

modalRef.current?.addEventListener('close', handleClose);
return () => modalRef.current?.removeEventListener('close', handleClose);
}, [onClose]);

return (
<Component className={cl('ds-modal', className)} ref={mergedRefs} {...rest}>
{closeButton !== false && (
Expand Down
15 changes: 15 additions & 0 deletions test/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ class ResizeObserver {
}
window.ResizeObserver = ResizeObserver;

/**
* TODO: Remove mock of Dialog element when jsdom supports it
* issue: https://github.com/jsdom/jsdom/issues/3294
*/

HTMLDialogElement.prototype.show = vi.fn(function mock(
this: HTMLDialogElement,
) {
Expand All @@ -31,6 +36,16 @@ HTMLDialogElement.prototype.close = vi.fn(function mock(
this: HTMLDialogElement,
) {
this.open = false;
/* dispatch close event */
this.dispatchEvent(new Event('close'));
});

/* add support for checking ESC key */
document.addEventListener('keydown', (event) => {
if (event.key === 'Escape') {
const dialog = document.querySelector('dialog');
dialog?.close();
}
});

// Add support for dialog form method
Expand Down

0 comments on commit 16b2988

Please sign in to comment.