Trap in open TOC at all sizes

openstax · Jan 23, 2024 · 55d625d · 55d625d
1 parent facf55c
commit 55d625d
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 41 deletions.
diff --git a/src/app/content/components/TableOfContents/index.tsx b/src/app/content/components/TableOfContents/index.tsx
@@ -15,7 +15,7 @@ import { CloseToCAndMobileMenuButton, TOCBackButton, TOCCloseButton } from '../S
 import { Header, HeaderText, SidebarPaneBody } from '../SidebarPane';
 import { LeftArrow, TimesIcon } from '../Toolbar/styled';
 import * as Styled from './styled';
-import { createTrapTab, useMatchMobileMediumQuery } from '../../../reactUtils';
+import { createTrapTab } from '../../../reactUtils';
 
 interface SidebarProps {
   onNavigate: () => void;
@@ -31,12 +31,8 @@ function TabTrapper({
   mRef: MutableRefObject<HTMLElement>;
   isTocOpen: boolean;
 }) {
-  const isMobile = useMatchMobileMediumQuery();
 
   React.useEffect(() => {
-    if (!isMobile) {
-      return;
-    }
     const listener = createTrapTab(mRef);
 
     if (isTocOpen) {
@@ -46,7 +42,7 @@ function TabTrapper({
     }
 
     return () => document?.removeEventListener('keydown', listener, true);
-  }, [mRef, isTocOpen, isMobile]);
+  }, [mRef, isTocOpen]);
 
   return null;
 }

diff --git a/src/app/reactUtils.spec.tsx b/src/app/reactUtils.spec.tsx
@@ -1,4 +1,4 @@
-import type { HTMLElement, KeyboardEvent, MediaQueryList } from '@openstax/types/lib.dom';
+import type { KeyboardEvent, MediaQueryList, HTMLElement as HTMLElementType } from '@openstax/types/lib.dom';
 import React from 'react';
 import renderer from 'react-test-renderer';
 import { resetModules, runHooks } from '../test/utils';
@@ -7,21 +7,21 @@ import { assertDocument, assertWindow } from './utils';
 import { dispatchKeyDownEvent } from '../test/reactutils';
 
 describe('onFocusInOrOutHandler focusout', () => {
-  let ref: React.RefObject<HTMLElement>;
-  let htmlElement: HTMLElement;
-  let childElement: HTMLElement;
-  let siblingElement: HTMLElement;
+  let ref: React.RefObject<HTMLElementType>;
+  let HTMLElementType: HTMLElementType;
+  let childElement: HTMLElementType;
+  let siblingElement: HTMLElementType;
   let addEventListener: jest.SpyInstance;
   let removeEventListener: jest.SpyInstance;
 
   beforeEach(() => {
-    htmlElement = assertDocument().createElement('div');
+    HTMLElementType = assertDocument().createElement('div');
     childElement = assertDocument().createElement('div');
-    htmlElement.appendChild(childElement);
+    HTMLElementType.appendChild(childElement);
     siblingElement = assertDocument().createElement('div');
     ref = {
-      current: htmlElement,
-    } as React.RefObject<HTMLElement>;
+      current: HTMLElementType,
+    } as React.RefObject<HTMLElementType>;
     addEventListener = jest.spyOn(ref.current!, 'addEventListener');
     removeEventListener = jest.spyOn(ref.current!, 'removeEventListener');
   });
@@ -84,19 +84,19 @@ describe('onFocusInOrOutHandler focusout', () => {
 });
 
 describe('onFocusInOrOutHandler focusin', () => {
-  let ref: React.RefObject<HTMLElement>;
-  let htmlElement: HTMLElement;
-  let childElement: HTMLElement;
-  let siblingElement: HTMLElement;
+  let ref: React.RefObject<HTMLElementType>;
+  let HTMLElementType: HTMLElementType;
+  let childElement: HTMLElementType;
+  let siblingElement: HTMLElementType;
 
   beforeEach(() => {
-    htmlElement = assertDocument().createElement('div');
+    HTMLElementType = assertDocument().createElement('div');
     childElement = assertDocument().createElement('div');
-    htmlElement.appendChild(childElement);
+    HTMLElementType.appendChild(childElement);
     siblingElement = assertDocument().createElement('div');
     ref = {
-      current: htmlElement,
-    } as React.RefObject<HTMLElement>;
+      current: HTMLElementType,
+    } as React.RefObject<HTMLElementType>;
   });
 
   it('clicking on child element triggers callback', () => {
@@ -183,14 +183,14 @@ describe('useOnDOMEvent', () => {
   });
 
   describe('on html element' , () => {
-    let ref: React.RefObject<HTMLElement>;
-    let htmlElement: HTMLElement;
+    let ref: React.RefObject<HTMLElementType>;
+    let HTMLElementType: HTMLElementType;
 
     beforeEach(() => {
-      htmlElement = assertDocument().createElement('div');
+      HTMLElementType = assertDocument().createElement('div');
       ref = {
-        current: htmlElement,
-      } as React.RefObject<HTMLElement>;
+        current: HTMLElementType,
+      } as React.RefObject<HTMLElementType>;
 
       addEventListener = jest.spyOn(ref.current!, 'addEventListener');
       removeEventListener = jest.spyOn(ref.current!, 'removeEventListener');
@@ -252,10 +252,17 @@ describe('useTrapTabNavigation', () => {
     window = assertWindow();
 
     addEventListener = jest.spyOn(window.document, 'addEventListener');
+    Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
+      configurable: true,
+      value: 200,
+    });
+    Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { 
+      configurable: true, value: 200 
+    });
   });
 
   function Component() {
-    const ref = React.useRef<HTMLElement | null>(null);
+    const ref = React.useRef<HTMLElementType | null>(null);
     utils.useTrapTabNavigation(ref);
 
     return <div />;
@@ -281,15 +288,15 @@ describe('useTrapTabNavigation', () => {
 describe('createTrapTab', () => {
   let trapTab: ReturnType<typeof utils.createTrapTab>;
   let querySelectorAll: ReturnType<typeof jest.spyOn>;
-  const htmlElement = assertDocument().createElement('div');
+  const HTMLElementType = assertDocument().createElement('div');
   const preventDefault = jest.fn();
   const d = assertDocument().createElement('div');
   const b = assertDocument().createElement('button');
   const i = assertDocument().createElement('input');
 
   beforeEach(() => {
-    const ref: React.MutableRefObject<HTMLElement> = {
-      current: htmlElement,
+    const ref: React.MutableRefObject<HTMLElementType> = {
+      current: HTMLElementType,
     };
 
     querySelectorAll = jest.spyOn(ref.current!, 'querySelectorAll');
@@ -300,14 +307,17 @@ describe('createTrapTab', () => {
     expect(querySelectorAll).not.toHaveBeenCalled();
   });
   it('handles Tab, no focusable elements', () => {
+    Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { 
+      configurable: true, value: 0 
+    });
     trapTab({key: 'Tab'} as KeyboardEvent);
     expect(querySelectorAll).toHaveBeenCalledTimes(1);
   });
   it('tabs forward (wrap around)', () => {
     b.focus = jest.fn();
-    htmlElement.appendChild(d); // not focusable
-    htmlElement.appendChild(b);
-    htmlElement.appendChild(i);
+    HTMLElementType.appendChild(d); // not focusable
+    HTMLElementType.appendChild(b);
+    HTMLElementType.appendChild(i);
     Object.defineProperty(document, 'activeElement', { value: i, writable: false, configurable: true });
     trapTab({key: 'Tab', preventDefault} as unknown as KeyboardEvent);
     expect(b.focus).toHaveBeenCalled();
@@ -329,16 +339,16 @@ describe('createTrapTab', () => {
 });
 
 describe('onKeyHandler', () => {
-  let ref: React.RefObject<HTMLElement>;
-  let htmlElement: HTMLElement;
+  let ref: React.RefObject<HTMLElementType>;
+  let HTMLElementType: HTMLElementType;
   let addEventListener: jest.SpyInstance;
   let removeEventListener: jest.SpyInstance;
 
   beforeEach(() => {
-    htmlElement = assertDocument().createElement('div');
+    HTMLElementType = assertDocument().createElement('div');
     ref = {
-      current: htmlElement,
-    } as React.RefObject<HTMLElement>;
+      current: HTMLElementType,
+    } as React.RefObject<HTMLElementType>;
     addEventListener = jest.spyOn(ref.current!, 'addEventListener');
     removeEventListener = jest.spyOn(ref.current!, 'removeEventListener');
   });

diff --git a/src/app/reactUtils.ts b/src/app/reactUtils.ts
@@ -18,6 +18,10 @@ export const useDrawFocus = <E extends HTMLElement = HTMLElement>() => {
   return ref;
 };
 
+function isHidden(el: HTMLElement) {
+  return el.offsetWidth === 0 && el.offsetHeight === 0;
+}
+
 // Exported to allow testing
 export function createTrapTab(ref: React.MutableRefObject<HTMLElement | null>) {
   return (event: KeyboardEvent) => {
@@ -28,7 +32,8 @@ export function createTrapTab(ref: React.MutableRefObject<HTMLElement | null>) {
 
     const focusableItemQuery =
       'button:not([disabled]), [href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])';
-    const focusableElements = el.querySelectorAll<HTMLElement>(focusableItemQuery);
+    const focusableElements = Array.from(el.querySelectorAll<HTMLElement>(focusableItemQuery))
+    .filter((e) => !isHidden(e));
 
     if (focusableElements.length === 0) { return; }
     const firstFocusableElement = focusableElements[0];