Fix refocus button bug #256
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have `outside click` behaviour implemented. Whenever the target element is focusable we make sure that the newly clicked/focused element stays focused. If it is not a focusable element we will make sure that the Menu/Listbox button is re-focused so that screenreader users don't get confused. This is all fine, but it turns out that when you have a button with a span, and you click on the span, then the event.target will be that span. The span itself is not focusable of course, but the button will get the focus. This results in the Menu/Listbox button being re-focused which is incorrect. For this we will introduce a FocusableMode on the `isFocusableElement`, we will have a `Strict` mode, which means the actual element should be focusable. And a `Loose` mode, which means that the actual element can be inside a focusable element. E.g.: A span within a button.
|
This pull request is being automatically deployed with Vercel (learn more). headlessui-react – ./packages/@headlessui-react🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-react/F3y47tZwCxzCn863zuB8DBExUift headlessui-vue – ./packages/@headlessui-vue🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-vue/AETPb4BETJKuqGcfar6k9zU4xZdR |
Copy paste can be fun sometimes
Merged
This was referenced Mar 15, 2021
Merged
RobinMalfait
added a commit
that referenced
this pull request
Mar 22, 2021
* fix outside click on span inside button works as expected We have `outside click` behaviour implemented. Whenever the target element is focusable we make sure that the newly clicked/focused element stays focused. If it is not a focusable element we will make sure that the Menu/Listbox button is re-focused so that screenreader users don't get confused. This is all fine, but it turns out that when you have a button with a span, and you click on the span, then the event.target will be that span. The span itself is not focusable of course, but the button will get the focus. This results in the Menu/Listbox button being re-focused which is incorrect. For this we will introduce a FocusableMode on the `isFocusableElement`, we will have a `Strict` mode, which means the actual element should be focusable. And a `Loose` mode, which means that the actual element can be inside a focusable element. E.g.: A span within a button. * rename menu to listbox Copy paste can be fun sometimes * update changelog
RobinMalfait
added a commit
that referenced
this pull request
Mar 26, 2021
* fix outside click on span inside button works as expected We have `outside click` behaviour implemented. Whenever the target element is focusable we make sure that the newly clicked/focused element stays focused. If it is not a focusable element we will make sure that the Menu/Listbox button is re-focused so that screenreader users don't get confused. This is all fine, but it turns out that when you have a button with a span, and you click on the span, then the event.target will be that span. The span itself is not focusable of course, but the button will get the focus. This results in the Menu/Listbox button being re-focused which is incorrect. For this we will introduce a FocusableMode on the `isFocusableElement`, we will have a `Strict` mode, which means the actual element should be focusable. And a `Loose` mode, which means that the actual element can be inside a focusable element. E.g.: A span within a button. * rename menu to listbox Copy paste can be fun sometimes * update changelog
RobinMalfait
added a commit
that referenced
this pull request
Apr 2, 2021
* fix outside click on span inside button works as expected We have `outside click` behaviour implemented. Whenever the target element is focusable we make sure that the newly clicked/focused element stays focused. If it is not a focusable element we will make sure that the Menu/Listbox button is re-focused so that screenreader users don't get confused. This is all fine, but it turns out that when you have a button with a span, and you click on the span, then the event.target will be that span. The span itself is not focusable of course, but the button will get the focus. This results in the Menu/Listbox button being re-focused which is incorrect. For this we will introduce a FocusableMode on the `isFocusableElement`, we will have a `Strict` mode, which means the actual element should be focusable. And a `Loose` mode, which means that the actual element can be inside a focusable element. E.g.: A span within a button. * rename menu to listbox Copy paste can be fun sometimes * update changelog
RobinMalfait
added a commit
that referenced
this pull request
Apr 2, 2021
* fix outside click on span inside button works as expected We have `outside click` behaviour implemented. Whenever the target element is focusable we make sure that the newly clicked/focused element stays focused. If it is not a focusable element we will make sure that the Menu/Listbox button is re-focused so that screenreader users don't get confused. This is all fine, but it turns out that when you have a button with a span, and you click on the span, then the event.target will be that span. The span itself is not focusable of course, but the button will get the focus. This results in the Menu/Listbox button being re-focused which is incorrect. For this we will introduce a FocusableMode on the `isFocusableElement`, we will have a `Strict` mode, which means the actual element should be focusable. And a `Loose` mode, which means that the actual element can be inside a focusable element. E.g.: A span within a button. * rename menu to listbox Copy paste can be fun sometimes * update changelog
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have
outside clickbehaviour implemented. Whenever the targetelement is focusable we make sure that the newly clicked/focused element
stays focused. If it is not a focusable element we will make sure that
the Menu/Listbox button is re-focused so that screenreader users don't
get confused.
This is all fine, but it turns out that when you have a button with a
span, and you click on the span, then the event.target will be that
span. The span itself is not focusable of course, but the button will
get the focus. This results in the Menu/Listbox button being re-focused
which is incorrect.
For this we will introduce a FocusableMode on the
isFocusableElement,we will have a
Strictmode, which means the actual element should befocusable. And a
Loosemode, which means that the actual element canbe inside a focusable element. E.g.: A span within a button.