3003 autocomplete when in dialog intercepts escape keypresses and click outside #3087
Conversation
|
size-limit report 📦
|
green6erry
force-pushed
the
3003-autocomplete-when-in-dialog-intercepts-escape-keypresses-and-click-outside
branch
from
4be31e9
to
7e4ae8f
Compare
|
@green6erry bump on this one for when you're back in office. Let's work on getting this closed out please! |
|
@green6erry Unsubscribing from changes, feel free to re-request review when you're ready :) |
…e Overlay instead of visually hiding it
…escape-keypresses-and-click-outside
There was a problem hiding this comment.
I think this is great work.
-
Found a potential accessibility concern.
-
question (not blocker for this PR, something to consider after): What is the difference between Dialog/useDialog and Overlay/useOverlay? Should Dialog use Overlay underneath to get focus + keyboard features?
| ) : ( | ||
| // HACK: This ensures AutocompleteMenu is still mounted when closing the menu and all of the hooks inside of it are still called. | ||
| // A better way to do this would be to move the hooks to AutocompleteOverlay or somewhere that won't get unmounted. | ||
| <VisuallyHidden>{children}</VisuallyHidden> |
There was a problem hiding this comment.
Comparing the a11y tree both production and this branch, in production (left) the listbox is not in the a11y tree until the autocomplete is opened. While, in this branch, it's in the a11y tree even when the autocomplete is closed.
because Overlay uses the css property visibility, it removes the contents from the accessibility tree, which VisuallyHidden does not.
If we can rid of the hack, that would of course be perfect. But, if not, we probably would need to make it aria-hidden as well.
…n-in-dialog-intercepts-escape-keypresses-and-click-outside
…escape-keypresses-and-click-outside
…escape-keypresses-and-click-outside
…escape-keypresses-and-click-outside
green6erry
deleted the
3003-autocomplete-when-in-dialog-intercepts-escape-keypresses-and-click-outside
branch
Describe your changes here.
Closes #3003
Screenshots
Please provide before/after screenshots for any visual changes
If the Dialog component includes an Autocomplete in the body, you can still close the dialog by pressing the
escapekey.Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.