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

Focus trap broken when first or last focusable element is hidden #35

Open
voidloaf opened this issue Jan 31, 2023 · 0 comments
Open

Focus trap broken when first or last focusable element is hidden #35

voidloaf opened this issue Jan 31, 2023 · 0 comments

Comments

@voidloaf
Copy link

Issue
The focus layer keyboard trap can be broken if the first or last focusable element is hidden and therefore not considered tabbable/focusable by the browser. This is a result of the tree walker filtering elements with a tabindex, but not considering whether those elements may have display: none; or visiblity:hidden; applied.

A real-world scenario where this breaks is a modal with any kind of collapsed menu contained within it, such as a menu in a video player.

Test Case
Here is a working test case of the bug with repro steps: https://vigilant-cacti.surge.sh/.
My fork of this repository contains the source for the repro example.

Proposed Fix
Adding additional checks to the tree walker filter for height and visibility in src/util/wrapFocus.tsx to cover situations where an element has tabindex and is not disabled, but might be un-focusable due to display: none; or visiblity:hidden;.

function createFocusWalker(root: HTMLElement) {
  return document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
    acceptNode: (node: HTMLElement) =>
NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP,
      node.tabIndex >= 0 && !node.disabled && node.getBoundingClientRect().height > 0 && window.getComputedStyle(node).visibility === 'visible' ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP,
  });
}
@voidloaf voidloaf changed the title Focus trap broken when first of last focusable element is hidden Focus trap broken when first or last focusable element is hidden Jan 31, 2023
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

No branches or pull requests

1 participant