-
Notifications
You must be signed in to change notification settings - Fork 560
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
Escape key closes multiple components #395
Comments
After playing around with the code, I managed to get this fixed by adding either case "Escape": {
if (state !== IDLE) {
transition(ESCAPE);
+ event.stopPropagation();
}
break;
} Do you see any problems with this fix? If not, I'd like to create a pull request for this. |
Another solution would be to export |
It would be nice to get this fixed. I'd happily create a pull request for either solution. Could you let me know which one is preferred? |
Sorry @stephan281094, this is on my radar but I just haven't had time to think much about the problem to figure if your proposal is the right approach. This is an issue with many components when used in conjunction with one another, and there's also some ambiguity as far as WCAG is concerned. I'll need to do a little more research on the accessibility impact before making a decision here. |
Hey @chancestrickland have you had some time to think about the issue at hand? The expected behavior of @stephan281094 sounds very reasonable to me.
Same as this potential technical solution although this could require library consumers to render a
|
@CodingDive Yes, the expected behavior as described is correct. What I haven't had time to vet out is the suggested implementation. I think |
I also think |
As noted in the PR, the suggested fix doesn't make the tests pass. Is the test wrong somehow or is it the fix itself? I'll also look into it a bit more. |
Closing this, as the library is currently unmaintained. If you're running into this issue, Radix UI, React Aria and Headless UI are good alternatives. |
🐛 Bug report
Current Behavior
When you have an open combobox inside of an open dialog and you hit the escape key, it closes both the combobox and the dialog.
Expected behavior
Instead of closing both the combobox and the dialog, it should only close the combobox when hitting the escape key.
Reproducible example
https://codesandbox.io/embed/reach-ui-template-7uyon
Suggested solution(s)
Maybe use a global stack that keeps track of all dismissible UI elements that listen to "escape". Every time you hit "escape" it could call
pop()
to only call theonDismiss
of the "deepest" UI element.Additional context
Perhaps the same issue occurs with other combinations of UI elements.
Your environment
The text was updated successfully, but these errors were encountered: