Skip to content

fix(Modal): replace deprecated maskClosable with mask.closable #481

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.

Sign up for GitHub

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

Open
hugetiny wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(Modal): replace deprecated maskClosable with mask.closable #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Modal/Modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const Modal = memo<ModalProps>(
>
<AntModal
closable
maskClosable
mask={{ closable: true }}
cancelText={cancelText}
className={cx(styles.content, className)}
closeIcon={<Icon icon={X} size={20} />}
Expand Down
2 changes: 1 addition & 1 deletion src/Modal/ModalStackItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const ModalStackItem = memo(
const stableOnCancel = useEventCallback(onCancel ?? noop);
const close = useEventCallback(() => onClose(id));
const setCanDismissByClickOutside = useEventCallback((value: boolean) =>
onUpdate(id, { maskClosable: value }),
onUpdate(id, { mask: { closable: value } }),
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onUpdate ultimately shallow-merges modal props in updateModal. Passing mask: { closable: value } here will overwrite any existing mask configuration on the imperative modal (dropping other mask fields). Consider updating the merge logic (or this update payload) so only mask.closable is changed while preserving other mask properties.

Copilot uses AI. Check for mistakes.
);
const stableContextValue = useMemo(
() => ({ close, setCanDismissByClickOutside }),
Expand Down
2 changes: 1 addition & 1 deletion src/Modal/RawModalStackItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const RawModalStackItem = memo(
const close = useEventCallback(() => stableOnClose(id));

const setCanDismissByClickOutside = useEventCallback((value: boolean) => {
onUpdate(id, { maskClosable: value });
onUpdate(id, { mask: { closable: value } });
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onUpdate shallow-merges raw props; sending mask: { closable: value } will replace any existing mask object and can drop other mask-related settings. Consider merging with the existing mask object (when it’s an object) so this toggle only affects mask.closable.

Suggested change
onUpdate(id, { mask: { closable: value } });
const currentMask = props['mask'];
const nextMask =
currentMask && typeof currentMask === 'object'
? { ...(currentMask as Record<string, unknown>), closable: value }
: { closable: value };
onUpdate(id, { mask: nextMask });

Copilot uses AI. Check for mistakes.
});
const contextValue: ModalContextValue = useMemo(
() => ({ close, setCanDismissByClickOutside }),
Expand Down
4 changes: 2 additions & 2 deletions src/Modal/imperative.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export const createModal = (props: ImperativeModalProps): ModalInstance => {
return {
close: () => closeModal(id),
destroy: () => destroyModal(id),
setCanDismissByClickOutside: (value) => updateModal(id, { maskClosable: value }),
setCanDismissByClickOutside: (value) => updateModal(id, { mask: { closable: value } }),
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateModal only does a shallow merge (props: { ...item.props, ...nextProps }). Updating mask with { closable: value } will overwrite any existing mask config (e.g., styles, className, motion) that may have been provided when creating/updating the modal. Consider merging with the existing props.mask (when it’s an object) so setCanDismissByClickOutside only changes mask.closable without dropping other fields.

Copilot uses AI. Check for mistakes.
update: (nextProps) => updateModal(id, nextProps),
};
};
Expand Down Expand Up @@ -271,7 +271,7 @@ export function createRawModal<P, OpenKey extends keyof P, CloseKey extends keyo
return {
close: () => closeModal(id),
destroy: () => destroyModal(id),
setCanDismissByClickOutside: (value) => updateRawProps(id, { maskClosable: value }),
setCanDismissByClickOutside: (value) => updateRawProps(id, { mask: { closable: value } }),
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateRawProps performs a shallow merge, so setting mask: { closable: value } will replace any existing mask object in the raw modal props. If callers provided additional mask options, setCanDismissByClickOutside will unintentionally drop them. Consider merging mask objects (when applicable) instead of overwriting.

Copilot uses AI. Check for mistakes.
update: (nextProps) => updateRawProps(id, nextProps as Record<string, unknown>),
};
}
Loading