Skip to content

Commit

Permalink
fix: make Modal's onRequestClose mandatory and call it on close (#602)
Browse files Browse the repository at this point in the history
BREAKING-CHANGE: Modal's and ConfirmModal's `onRequestClose` is now mandatory.
Note that a Modal has to always be closed (`isOpen` must be set to `false`) when `onRequestClose` is called.
This is driven by a change in the HTML spec that ensures a web page cannot indefinitely keep a dialog open.
See whatwg/html#9462 and https://bugs.chromium.org/p/chromium/issues/detail?id=1511166#c1
  • Loading branch information
targos authored Dec 13, 2023
1 parent 63f093b commit e3f753f
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/components/modal/ConfirmModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface ConfirmModalProps {
requestCloseOnBackdrop?: boolean;
onConfirm: () => void;
onCancel?: () => void;
onRequestClose?: () => void;
onRequestClose: () => void;
saveText?: string;
cancelText?: string;
headerColor: string;
Expand Down
2 changes: 1 addition & 1 deletion src/components/modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface ModalProps {
requestCloseOnEsc?: boolean;
hasCloseButton?: boolean;
requestCloseOnBackdrop?: boolean;
onRequestClose?: () => void;
onRequestClose: () => void;
maxWidth?: number;
width?: number;
height?: number;
Expand Down
20 changes: 10 additions & 10 deletions src/components/modal/useDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ import {
} from 'react';
import { useKbsDisableGlobal } from 'react-kbs';

import { useOnOff } from '../index';
import { useOnOff } from '../hooks/useOnOff';

interface UseDialogOptions {
dialogRef: RefObject<HTMLDialogElement>;
isOpen: boolean;
requestCloseOnEsc: boolean;
requestCloseOnBackdrop: boolean;
onRequestClose?: () => void;
onRequestClose: () => void;
}

interface UseDialogReturn {
dialogProps: {
onClick: MouseEventHandler<HTMLDialogElement>;
onCancel: ReactEventHandler<HTMLDialogElement>;
onClose: UseDialogOptions['onRequestClose'];
};
isModalShown: boolean;
}
Expand Down Expand Up @@ -50,12 +51,11 @@ export function useDialog({

const onCancel = useCallback<ReactEventHandler<HTMLDialogElement>>(
(event) => {
event.preventDefault();
if (requestCloseOnEsc && onRequestClose) {
onRequestClose();
if (!requestCloseOnEsc) {
event.preventDefault();
}
},
[onRequestClose, requestCloseOnEsc],
[requestCloseOnEsc],
);

const onClick = useCallback<MouseEventHandler<HTMLDialogElement>>(
Expand All @@ -77,15 +77,15 @@ export function useDialog({
// Since the dialog has no size of itself, this condition is only
// `true` when we click on the backdrop.
if (!isInDialog && requestCloseOnBackdrop) {
onRequestClose?.();
onRequestClose();
}
},
[dialogRef, requestCloseOnBackdrop, onRequestClose],
[requestCloseOnBackdrop, onRequestClose, dialogRef],
);

const dialogProps = useMemo<UseDialogReturn['dialogProps']>(
() => ({ onClick, onCancel }),
[onClick, onCancel],
() => ({ onClick, onCancel, onClose: onRequestClose }),
[onClick, onCancel, onRequestClose],
);

return {
Expand Down

0 comments on commit e3f753f

Please sign in to comment.