discord/focus-layers

Focus trap broken when first or last focusable element is hidden

voidloaf opened this issue · 0 comments

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,
  });
}