Skip to content
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

Pressing ESC not working as expected with dialogs+popups #990

Closed
xdev1 opened this issue Nov 1, 2022 · 4 comments
Closed

Pressing ESC not working as expected with dialogs+popups #990

xdev1 opened this issue Nov 1, 2022 · 4 comments
Assignees
Labels
bug Things that aren't working right in the library. good first issue This bug or task is a good first issue for new contributors. help wanted Ready for a contributor to tackle.

Comments

@xdev1
Copy link
Contributor

xdev1 commented Nov 1, 2022

Please open the following MUI demo:
https://mui.com/material-ui/react-dialog/#optional-sizes

Now click on the "OPEN MAX-WIDTH DIALOG" button and after that open the popup of the "maxWidth" select field. Now you have an opened dialog plus an opened popup at the same time.
Now press Escape => Result: The popup of the select field closes while the dialog stays open.
Now press Escape again => Result: The dialog closes.

I'd say, this is the expected behavior.
Now try the same with the following Shoelace demo:
https://codepen.io/xdev1/pen/WNyrjgE?editors=1000
Result: Both dialog AND popup will close immediately when you press Escape for the first time.

I personally think, this should be considered a bug in Shoelace and not just somehow be fixed in userland.

@xdev1 xdev1 added the bug Things that aren't working right in the library. label Nov 1, 2022
@claviska claviska added good first issue This bug or task is a good first issue for new contributors. help wanted Ready for a contributor to tackle. labels Nov 2, 2022
@claviska
Copy link
Member

claviska commented Nov 2, 2022

Agree. Thanks for reporting this!

@claviska claviska added this to the 2.0.0 stable milestone Nov 8, 2022
@claviska
Copy link
Member

claviska commented Nov 9, 2022

Fixed in 5fd682d. This will be available in the next release.

@xdev1
Copy link
Contributor Author

xdev1 commented Nov 23, 2022

Unfortunately, this issue should be reopened, as this is still buggy.

Please open this demo, click on "Open dialog" and then click on the select box without selecting anything.
The popup is opened now. Then please press Escape => you'll see that the dialog will be closed immediately, instead of just closing the popup.

This change might do the trick:

image

@claviska
Copy link
Member

@xdev1 Thanks for the update. The previous fix solved pressing Escape when the menu has focus. You discovered another edge case that occurred when the menu has focus.

I appreciate the report and the suggested fix! It was patched in 7f9b0bd and will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library. good first issue This bug or task is a good first issue for new contributors. help wanted Ready for a contributor to tackle.
Projects
None yet
Development

No branches or pull requests

2 participants