fix tabbing into overflowing elements

KonnorRogers · Jan 10, 2024 · c917bde · c917bde
1 parent 61def41
commit c917bde
Show file tree

Hide file tree

Showing 4 changed files with 212 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.1.1
+
+- Fixed tabbing into overflowing elements, and reduced the chances of the focus getting "stuck"
+on unrecognized / unfocusable elements.
+
 ## 1.1.0
 
 - Improved the performance of tabbable checking by using `checkVisibility()`

diff --git a/exports/focus-hunter.js b/exports/focus-hunter.js
@@ -1,4 +1,5 @@
 // @ts-check
+import { activeElements } from './active-elements.js';
 import { deepestActiveElement } from './active-elements.js';
 import { getTabbableElements } from './tabbable.js';
 
@@ -230,47 +231,43 @@ export class Trap {
 
     const tabbableElements = [...getTabbableElements(this.rootElement)];
 
-    const start = tabbableElements[0]
-
     let currentFocusIndex = tabbableElements.findIndex((el) => el === currentFocus)
 
-    if (currentFocusIndex === -1) {
-      this.currentFocus = (/** @type {HTMLElement} */ (start));
-
-      event.preventDefault()
-      this.currentFocus?.focus?.({ preventScroll: this.preventScroll });
-      return;
-    }
-
     const addition = this.tabDirection === 'forward' ? 1 : -1;
 
-    if (currentFocusIndex + addition >= tabbableElements.length) {
-      currentFocusIndex = 0;
-    } else if (currentFocusIndex + addition < 0) {
-      currentFocusIndex = tabbableElements.length - 1;
-    } else {
-      currentFocusIndex += addition;
-    }
+    while(true) {
+      if (currentFocusIndex + addition >= tabbableElements.length) {
+        currentFocusIndex = 0;
+      } else if (currentFocusIndex + addition < 0) {
+        currentFocusIndex = tabbableElements.length - 1;
+      } else {
+        currentFocusIndex += addition;
+      }
 
-    this.previousFocus = this.currentFocus
-    const nextFocus = /** @type {HTMLElement} */ (tabbableElements[currentFocusIndex])
+      this.previousFocus = this.currentFocus
+      const nextFocus = /** @type {HTMLElement} */ (tabbableElements[currentFocusIndex])
 
+      // This is a special case. We need to make sure we're not calling .focus() if we're already focused on an element
+      // that possibly has "controls"
+      if (this.tabDirection === "backward") {
+        if (this.previousFocus && this.possiblyHasTabbableChildren(/** @type {HTMLElement} */ (this.previousFocus))) {
+          return
+        }
 
-    // This is a special case. We need to make sure we're not calling .focus() if we're already focused on an element
-    // that possibly has "controls"
-    if (this.tabDirection === "backward") {
-      if (this.previousFocus && this.possiblyHasTabbableChildren(/** @type {HTMLElement} */ (this.previousFocus))) {
-        return
+        if (nextFocus && this.possiblyHasTabbableChildren(nextFocus)) {
+          return
+        }
       }
 
-      if (nextFocus && this.possiblyHasTabbableChildren(nextFocus)) {
-        return
+      event.preventDefault()
+      this.currentFocus = nextFocus;
+      this.currentFocus?.focus({ preventScroll: this.preventScroll });
+
+      // @ts-expect-error
+      if ([...activeElements()].includes(this.currentFocus)) {
+        break
       }
     }
-
-    event.preventDefault()
-    this.currentFocus = nextFocus;
-    this.currentFocus?.focus({ preventScroll: this.preventScroll });
   }
 
 

diff --git a/exports/tabbable.js b/exports/tabbable.js
@@ -7,16 +7,11 @@ const computedStyleMap = /** @type {WeakMap<Element, CSSStyleDeclaration>} */ (n
 
 /**
  * @param {Element} el
+ * @returns {CSSStyleDeclaration}
  */
-function isVisible(el) {
-  // This is the fastest check, but isn't supported in Safari.
-  if (typeof el.checkVisibility === 'function') {
-    return el.checkVisibility({ checkOpacity: false, checkVisibilityCSS: true });
-  }
-
-  // Fallback "polyfill" for "checkVisibility"
+function getCachedComputedStyle(el) {
   /**
-   * @type {CSSStyleDeclaration | undefined}
+   * @type {undefined | CSSStyleDeclaration}
    */
   let computedStyle = computedStyleMap.get(el);
 
@@ -25,9 +20,60 @@ function isVisible(el) {
     computedStyleMap.set(el, computedStyle);
   }
 
+  return /** @type {CSSStyleDeclaration} */ (computedStyle);
+}
+
+/**
+ * @param {Element} el
+ */
+function isVisible(el) {
+  // This is the fastest check, but isn't supported in Safari.
+  if (typeof el.checkVisibility === 'function') {
+    return el.checkVisibility({ checkOpacity: false, checkVisibilityCSS: true });
+  }
+
+  // Fallback "polyfill" for "checkVisibility"
+  const computedStyle = getCachedComputedStyle(el);
+
   return computedStyle.visibility !== 'hidden' && computedStyle.display !== 'none';
 }
 
+/**
+ * While this behavior isn't standard in Safari / Chrome yet, I think it's the most reasonable
+ *   way of handling tabbable overflow areas. Browser sniffing seems gross, and it's the most
+ *   accessible way of handling overflow areas. [Konnor]
+ * @param {Element} el
+ * @return {boolean}
+ */
+function isOverflowingAndTabbable(el) {
+  const computedStyle = getCachedComputedStyle(el);
+
+  const { overflowY, overflowX } = computedStyle;
+
+  if (overflowY === 'scroll' || overflowX === 'scroll') {
+    return true;
+  }
+
+  if (overflowY !== 'auto' || overflowX !== 'auto') {
+    return false;
+  }
+
+  // Always overflow === "auto" by this point
+  const isOverflowingY = el.scrollHeight > el.clientHeight;
+
+  if (isOverflowingY && overflowY === 'auto') {
+    return true;
+  }
+
+  const isOverflowingX = el.scrollWidth > el.clientWidth;
+
+  if (isOverflowingX && overflowX === 'auto') {
+    return true;
+  }
+
+  return false;
+}
+
 
 /**
  * Determines if the specified element is tabbable using heuristics inspired by https://github.com/focus-trap/tabbable
@@ -37,17 +83,9 @@ function isVisible(el) {
 export function isTabbable(el) {
   const tag = el.tagName.toLowerCase();
 
-  // Elements that are hidden have no offsetParent and are not tabbable
-  // offsetParent() is added because otherwise it misses elements in Safari
-  if (
-    !isVisible(/** @type {HTMLElement} */ (el))
-  )
-  {
-    return false;
-  }
+  const tabindex = Number(el.getAttribute('tabindex'));
+  const hasTabindex = el.hasAttribute('tabindex');
 
-  const tabindex = Number(el.getAttribute('tabindex'))
-  const hasTabindex = el.hasAttribute("tabindex")
 
   // elements with a tabindex attribute that is either NaN or <= -1 are not tabbable
   if (hasTabindex && (isNaN(tabindex) || tabindex <= -1)) {
@@ -68,6 +106,13 @@ export function isTabbable(el) {
     return false;
   }
 
+  // Elements that are hidden have no offsetParent and are not tabbable
+  // offsetParent() is added because otherwise it misses elements in Safari
+  if (!isVisible(/** @type {HTMLElement} */ (el)))
+  {
+    return false;
+  }
+
   // Anchor tags with no hrefs arent focusable.
   // This is focusable: <a href="/">Stuff</a>
   // This is not focusable: <a href="">Stuff</a>
@@ -89,7 +134,14 @@ export function isTabbable(el) {
   }
 
   // At this point, the following elements are considered tabbable
-  return ['button', 'input', 'select', 'textarea', 'a', 'audio', 'video', 'summary', 'iframe', 'object', 'embed'].includes(tag);
+  const isNativelyTabbable = ['button', 'input', 'select', 'textarea', 'a', 'audio', 'video', 'summary', 'iframe', 'object', 'embed'].includes(tag);
+
+  if (isNativelyTabbable) {
+    return true;
+  }
+
+  // We save the overflow checks for last, because they're the most expensive
+  return isOverflowingAndTabbable(el);
 }
 
 /**

diff --git a/index.html b/index.html
@@ -228,6 +228,115 @@ <h2>
         <br><br>
         <audio controls allowfullscreen playsinline src="./media/video-1.mp4"></audio>
       </div>
+
+      <h2>
+        Handling overflow elements
+      </h2>
+
+      <button class="js-activate-trap">
+        Activate Trap
+      </button>
+
+      <div class="trap">
+
+        Non-overflowing w/ <code>overflow: auto;</code>
+        <div style="overflow: auto; max-height: 100px; display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 8px; padding: 4px;">
+          <button>Button</button>
+        </div>
+
+        <br><br>
+
+        <span>Overflowing w/ <code>overflow: auto;</code></span>
+        <br>
+        <div style="overflow: auto; max-height: 100px; display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 8px; padding: 4px;">
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+        </div>
+
+        <br><br>
+
+        <span><code>overflow scroll</code></span>
+        <br>
+        <div style="overflow: scroll; max-height: 100px; display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 8px; padding: 4px;">
+          <button>Button</button>
+        </div>
+
+        <br><br>
+
+        <span><code>overflow scroll</code></span>
+        <br>
+        <div style="overflow: scroll; max-height: 100px; display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 8px; padding: 4px;">
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+        </div>
+
+        <br><br>
+
+        <span><code>overflow: hidden;</code></span>
+          <br>
+        <div style="overflow: hidden; max-height: 100px; display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 8px; padding: 4px;">
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+          <button>Button</button>
+        </div>
+
+        <br><br>
+
+        <button class="js-deactivate-trap">Deactivate Trap</button>
+      </div>
     </main>
 
     <script type="module">