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

Decompose useElementShouldClose into more targeted hooks #900

Closed
lyzadanger opened this issue Mar 3, 2023 · 0 comments · Fixed by #911
Closed

Decompose useElementShouldClose into more targeted hooks #900

lyzadanger opened this issue Mar 3, 2023 · 0 comments · Fixed by #911

Comments

@lyzadanger
Copy link
Contributor

useElementShouldClose conflates too many events into one hook, without the ability to configure it more specifically. It currently invokes the provided close callback for any external click, external focus or the keypress of Esc. You can't opt in or out of any of these.

Arguments:

  • With the re-implementation of Dialog, the away-focus handling won't make sense as the component will focus-trap.
  • Sometimes we don't want to close a modal on click-away (it becomes too easy, e.g., to accidentally dismiss the modal, as is the case in the LMS file pickers)
  • Sometimes we want to react to Esc, but not a click-away

We might consider decomposing this hook, e.g., maybe (just one option here):

  • useClickAway and useFocusAway, or useAwayEvent (hook that could take multiple events)
  • useKeyPress specifically for keyboard event handling

There are a number of plausible options here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant