Skip to content

[Dialog] dismissible={false} doesn't prevent dismissing the modal on Escape key press #1688

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

Open
benface opened this issue Apr 8, 2025 · 1 comment
Labels
component: dialog This is the name of the generic UI component, not the React module! status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it

Comments

@benface
Copy link

benface commented Apr 8, 2025

Bug report

Current behavior

I noticed this behavior while writing #1687, and I'm not sure at all it's a bug, so maybe consider this a feature request... but I was really surprised to see that when passing dismissible={false} to Dialog.Root, the dialog was still dismissible when pressing the Escape key. That can of course be worked around by controlling the open state with open and onOpenChange, but it feels like something that should be handled automatically, since it already does prevent closing the dialog when clicking outside it / on the backdrop. I'm not sure what's different about pressing the Escape key, and I couldn't find anything in the ARIA spec about it.

Expected behavior

The behavior I expect, at least, is that dismissible={false} on Dialog.Root prevents closing the dialog when the Escape key is pressed (so if the open state is uncontrolled, the dialog stays open, and if it is controlled, onOpenChange is not called).

Reproducible example

https://codesandbox.io/p/devbox/wizardly-bohr-7h932q?file=%2Fsrc%2FApp.tsx%3A20%2C38-20%2C39&workspaceId=ws_VCLs2BtBqfm4WA73jXMWhM

Base UI version

1.0.0-alpha.7

Which browser are you using?

Chrome, Safari, and Firefox

Which OS are you using?

macOS

Which assistive tech are you using (if applicable)?

None

@benface benface added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 8, 2025
@mj12albert mj12albert added component: dialog This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 9, 2025
@mj12albert
Copy link
Member

mj12albert commented Apr 9, 2025

We do document that it only relates to outside clicks: https://github.com/mui/base-ui/blob/master/packages/react/src/dialog/root/useDialogRoot.ts#L234-L238

Though I agree the name dismissible could be easily be misinterpreted as "completely dismissable" 🤔

@mj12albert mj12albert added the status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it label Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it
Projects
None yet
Development

No branches or pull requests

2 participants