[@mantine/hooks] use-focus-trap: shift + tab bug #4679
Conversation
When using shift + tab to go back into a Radio Group, the last option will be selected, which doesn't line up with finalTabbable, and breaks out of the focus trap. To fix, we can check to see if the active element is a radio button, find it's radio group, and then check to see if the finalTabbable is in the same group. If so, set leavingFinalTabbable to true. Addresses mantinedev#4678
| userEvent.tab(); | ||
| expect(screen.getByRole('button')).toHaveFocus(); | ||
|
|
||
| userEvent.tab({ shift: true }); |
There was a problem hiding this comment.
This doesn't appear to be working as expected, though it does pass when I run as part of a different codebase. We use the following versions of testing library. Was there a breaking change?
"@testing-library/dom": "^8.19.0",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^12.1.5",
"@testing-library/user-event": "^13.5.0",| if (root.activeElement instanceof HTMLInputElement && root.activeElement.type === 'radio') { | ||
| const activeRadioGroup = tabbable.filter( | ||
| (element) => | ||
| element instanceof HTMLInputElement && | ||
| root.activeElement instanceof HTMLInputElement && | ||
| element.type === 'radio' && | ||
| element.name === root.activeElement.name | ||
| ); | ||
| leavingFinalTabbable = activeRadioGroup.includes(finalTabbable); | ||
| } |
There was a problem hiding this comment.
This, obviously, is only a solution for Radio Groups, but I suspect there's similar behavior for other elements.
Another idea I had was to get the next target, and check if it's in tabbable at all, but I wasn't sure if there was an API exposed to do that easily, or if we'd need to do a lot of work to calculate it on our own.
|
@Devon-Dickson are you planning to finish working on this PR? |
I believe this is an accessibility bug, and I'd like to see the fix merged. However I'm a bit busy at the moment and don't have the bandwidth to debug the RTL issue ( I may be able to revisit in a few weeks, unless you're familiar with the issue and can recommend an immediate solution? |
|
I believe it should be resolved by #4856 |
Looks like another route to the same solution 👍. No tests yet though, which is the blocker on this PR. |
|
I see #4856 has been merged, but is still missing tests. If I get the time in the next couple of weeks, I can look at this again and get some tests added and passing. In the meantime, we can close this PR. |
|
Thanks! |
|
Issue with the tests ended up being a small thing, but comes with a performance cost |
When using shift + tab to go back into a Radio Group, the last option will be selected, which doesn't line up with
finalTabbable, and breaks out of the focus trap.To fix, we can check to see if the active element is a radio button, find its radio group, and then check to see if the
finalTabbableis in the same group. If so, setleavingFinalTabbabletotrue.Addresses #4678