fix(ui-a11y-utils): fix focus region skipping items after Drilldown/scrollbars

matyasf · matyasf · commit 6d4a9837551e · 2025-09-05T17:12:37.000+02:00
The core of the issue is that findFocusable cannot detect everything that can be focused reliably,
e.g. browsers now focus on DOM elements that have scrollbars. The deleted code added the element
with the scrollbar to the end of the focus  region array, causing anything after it skipped when
tabbing. The downside of deleting this code is that focus order breaks when the last element in a
Popover/Modal is something that findFocusable does not detect e.g. a Menu (without a trigger,
 this is not detected because its tabIndex changes dynamically)

Fixes INSTUI-4694
diff --git a/packages/ui-a11y-utils/src/scopeTab.ts b/packages/ui-a11y-utils/src/scopeTab.ts
@@ -26,8 +26,7 @@ import {
   findDOMNode,
   findTabbable,
   isActiveElement,
-  containsActiveElement,
-  getActiveElement
+  containsActiveElement
 } from '@instructure/ui-dom-utils'
 import type { UIElement } from '@instructure/shared-types'
 
@@ -56,17 +55,13 @@ function scopeTab(
     return
   }
 
-  // Account for a changing tabindex of the active element
-  // (a case that happens with Menu for KO a11y)
-  if (containsActiveElement(element)) {
-    const activeElement = getActiveElement()
-    if (activeElement && tabbable.indexOf(activeElement) === -1) {
-      tabbable.push(activeElement)
-    }
-  }
-
   const finalTabbable = tabbable[event.shiftKey ? 0 : tabbable.length - 1]
   const leavingFinalTabbable =
+    // This can happen in Menu, where the first menu item becomes unfocusable
+    // when its focused and tab is pressed, leaving one element
+    // (the menu container) in the tabbable array, which is not the currently
+    // active element
+    tabbable.length < 2 ||
     isActiveElement(finalTabbable) ||
     // handle immediate shift+tab after opening with mouse
     isActiveElement(node) ||
diff --git a/regression-test/package-lock.json b/regression-test/package-lock.json
diff --git a/regression-test/src/app/calendar/page.tsx b/regression-test/src/app/calendar/page.tsx
@@ -30,9 +30,6 @@ const Calendar = cl as any
 export default function CalendarPage() {
   return (
     <main className="flex gap-8 p-8 flex-col items-start axe-test">
-      {/* Default config */}
-      <Calendar />
-
       {/* Default config with additional props */}
       <Calendar
         visibleMonth="2025-05"
diff --git a/regression-test/src/app/layout.tsx b/regression-test/src/app/layout.tsx
@@ -38,6 +38,9 @@ export default function RootLayout({
     // we need to make a new Map to reset counting on the server side
     // on each page refresh TODO fix
     <html lang="en">
+      <head>
+        <title>Component visual and regression test suite</title>
+      </head>
       <InstUISettingsProvider instanceCounterMap={new Map()}>
         <body className={inter.className}>{children}</body>
       </InstUISettingsProvider>
