Merge branch 'main' into focus-on-toc-when-toc-opens

src/app/content/components/TableOfContents/index.spec.tsx

@@ -15,11 +15,13 @@ import * as actions from '../../actions';
 import { initialState } from '../../reducer';
 import { formatBookData } from '../../utils';
 import * as domUtils from '../../utils/domUtils';
+import * as reactUtils from '../../../reactUtils';
 
 const book = formatBookData(archiveBook, mockCmsBook);
 
 describe('TableOfContents', () => {
   let store: Store;
+  let Component: React.JSX.Element; // tslint:disable-line:variable-name
 
   beforeEach(() => {
     const state = {
@@ -29,29 +31,27 @@ describe('TableOfContents', () => {
       },
     } as any as AppState;
     store = createTestStore(state);
+    Component =
+      <TestContainer store={store}>
+        <ConnectedTableOfContents />
+      </TestContainer>;
   });
 
   it('mounts and unmounts without a dom', () => {
-    const component = renderer.create(<TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>);
+    const component = renderer.create(Component);
     expect(() => component.unmount()).not.toThrow();
   });
 
   it('mounts and unmmounts with a dom', () => {
-    const {root} = renderToDom(<TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>);
+    const {root} = renderToDom(Component);
     expect(() => unmountComponentAtNode(root)).not.toThrow();
   });
 
   it('expands and scrolls to current chapter', () => {
     const scrollSidebarSectionIntoView = jest.spyOn(domUtils, 'scrollSidebarSectionIntoView');
     const expandCurrentChapter = jest.spyOn(domUtils, 'expandCurrentChapter');
 
-    renderer.create(<TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>);
+    renderer.create(Component);
 
     expect(expandCurrentChapter).not.toHaveBeenCalled();
     expect(scrollSidebarSectionIntoView).toHaveBeenCalledTimes(1);
@@ -65,11 +65,14 @@ describe('TableOfContents', () => {
   });
 
   it('opens and closes', () => {
+    jest.spyOn(reactUtils, 'useMatchMobileQuery')
+      .mockReturnValue(true);
     jest.spyOn(reactUtils, 'useMatchMobileMediumQuery')
       .mockReturnValue(true);
-    const component = renderer.create(<TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>);
+    const component = renderer.create(Component);
+
+    // To exercise ref code
+    renderToDom(Component);
 
     expect(component.root.findByType(TableOfContents).props.isOpen).toBe(null);
     renderer.act(() => {
@@ -117,9 +120,7 @@ describe('TableOfContents', () => {
   it('resets toc on navigate', () => {
     const dispatchSpy = jest.spyOn(store, 'dispatch');
 
-    const component = renderer.create(<TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>);
+    const component = renderer.create(Component);
 
     renderer.act(() => {
       component.root.findAllByType('a')[0].props.onClick({preventDefault: () => null});
@@ -133,11 +134,7 @@ describe('TableOfContents', () => {
       return expect(document).toBeTruthy();
     }
 
-    const render = () => <TestContainer store={store}>
-      <ConnectedTableOfContents />
-    </TestContainer>;
-
-    const {node} = renderToDom(render());
+    const {node} = renderToDom(Component);
     const spy = jest.spyOn(node.style, 'setProperty');
 
     const event = document.createEvent('UIEvents');

src/app/content/components/TableOfContents/index.tsx

@@ -16,6 +16,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, useMatchMobileQuery, useMatchMobileMediumQuery } from '../../../reactUtils';
 
 interface SidebarProps {
   onNavigate: () => void;

src/app/content/components/Toolbar/index.spec.tsx

@@ -12,6 +12,7 @@ import { receiveFeatureFlags } from '../../../featureFlags/actions';
 import { MiddlewareAPI, Store } from '../../../types';
 import { assertWindow } from '../../../utils';
 import { closeMobileMenu } from '../../actions';
+import { openToc } from '../../actions';
 import { practiceQuestionsFeatureFlag } from '../../constants';
 import { clearSearch, openSearchInSidebar } from '../../search/actions';
 import * as searchSelectors from '../../search/selectors';
@@ -54,15 +55,20 @@ describe('toolbar', () => {
 
   it('has a button that closes mobile menu', () => {
     const dispatchSpy = jest.spyOn(store, 'dispatch');
+    const sidebar = assertWindow().document.createElement('div');
     const component = renderer.create(<TestContainer store={store}>
       <Toolbar />
-    </TestContainer>);
+    </TestContainer>, { createNodeMock: () => sidebar });
 
     renderer.act(() => {
       component.root.findByType(CloseToCAndMobileMenuButton).findByType(PlainButton).props.onClick();
     });
 
     expect(dispatchSpy).toHaveBeenCalledWith(closeMobileMenu());
+
+    // exercise teardown of useEffect
+    store.dispatch(openToc());
+    expect(selectors.tocOpen(store.getState())).toEqual(true);
   });
 
   describe('print button', () => {

src/app/content/components/Toolbar/index.tsx

@@ -4,7 +4,7 @@ import { FormattedMessage } from 'react-intl';
 import { useSelector } from 'react-redux';
 import * as pqSelectors from '../../practiceQuestions/selectors';
 import { searchInSidebar } from '../../search/selectors';
-import { mobileMenuOpen } from '../../selectors';
+import { mobileMenuOpen, tocOpen } from '../../selectors';
 import { setSidebarHeight } from '../../utils/domUtils';
 import { nudgeStudyToolsTargetId } from '../NudgeStudyTools/constants';
 import { NudgeElementTarget } from '../NudgeStudyTools/styles';
@@ -20,6 +20,7 @@ import PracticeQuestionsButton from './PracticeQuestionsButton';
 import PrintButton from './PrintButton';
 import StudyGuidesButton from './StudyGuidesButton';
 import * as Styled from './styled';
+import { createTrapTab } from '../../../reactUtils';
 
 // tslint:disable-next-line: variable-name
 const VerticalNav = () => {
@@ -29,6 +30,7 @@ const VerticalNav = () => {
   );
   const sidebarRef = React.useRef<HTMLElement>(null);
   const showSearchInSidebar = useSelector(searchInSidebar);
+  const isTocOpen = useSelector(tocOpen);
 
   React.useEffect(() => {
     const sidebar = sidebarRef.current;
@@ -39,7 +41,22 @@ const VerticalNav = () => {
     callback();
 
     return deregister;
-  }, [sidebarRef]);
+  }, []);
+
+  React.useEffect(
+    () => {
+      if (isMobileMenuOpen && sidebarRef.current && !isTocOpen) {
+        const listener = createTrapTab(sidebarRef.current);
+
+        document?.addEventListener('keydown', listener, true);
+
+        return () => {
+          document?.removeEventListener('keydown', listener, true);
+        };
+      }
+    },
+    [isMobileMenuOpen, isTocOpen]
+  );
 
   return (
     <Styled.ToolbarWrapper

src/app/reactUtils.spec.tsx

@@ -1,4 +1,4 @@
-import type { HTMLElement, KeyboardEvent, MediaQueryList } from '@openstax/types/lib.dom';
+import type { KeyboardEvent, MediaQueryList, HTMLElement } from '@openstax/types/lib.dom';
 import React from 'react';
 import renderer from 'react-test-renderer';
 import { resetModules, runHooks } from '../test/utils';
@@ -255,7 +255,11 @@ describe('useTrapTabNavigation', () => {
   });
 
   function Component() {
-    const ref = React.useRef<HTMLElement | null>(null);
+    const container = assertDocument().createElement('div');
+    const b = assertDocument().createElement('button');
+
+    container.appendChild(b);
+    const ref = React.useRef<HTMLElement | null>(container);
     utils.useTrapTabNavigation(ref);
 
     return <div />;
@@ -280,34 +284,33 @@ describe('useTrapTabNavigation', () => {
 
 describe('createTrapTab', () => {
   let trapTab: ReturnType<typeof utils.createTrapTab>;
-  let querySelectorAll: ReturnType<typeof jest.spyOn>;
   const htmlElement = 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,
-    };
+  htmlElement.appendChild(d); // not focusable
+  htmlElement.appendChild(b);
+  htmlElement.appendChild(i);
 
-    querySelectorAll = jest.spyOn(ref.current!, 'querySelectorAll');
-    trapTab = utils.createTrapTab(ref);
+  Object.defineProperty(b, 'offsetHeight', {
+    value: 1000,
+    writable: false,
   });
-  it('ignores non_Tab events', () => {
-    trapTab({key: 'a'} as KeyboardEvent);
-    expect(querySelectorAll).not.toHaveBeenCalled();
+  Object.defineProperty(i, 'offsetWidth', {
+    value: 1000,
+    writable: false,
   });
-  it('handles Tab, no focusable elements', () => {
-    trapTab({key: 'Tab'} as KeyboardEvent);
-    expect(querySelectorAll).toHaveBeenCalledTimes(1);
+
+  beforeEach(() => {
+    trapTab = utils.createTrapTab(htmlElement);
+  });
+  it('ignores non-Tab events', () => {
+    trapTab({key: 'a'} as KeyboardEvent);
   });
   it('tabs forward (wrap around)', () => {
     b.focus = jest.fn();
-    htmlElement.appendChild(d); // not focusable
-    htmlElement.appendChild(b);
-    htmlElement.appendChild(i);
     Object.defineProperty(document, 'activeElement', { value: i, writable: false, configurable: true });
     trapTab({key: 'Tab', preventDefault} as unknown as KeyboardEvent);
     expect(b.focus).toHaveBeenCalled();
@@ -324,8 +327,21 @@ describe('createTrapTab', () => {
    it('tabs normally otherwise', () => {
     preventDefault.mockClear();
     trapTab({key: 'Tab', preventDefault} as unknown as KeyboardEvent);
+    Object.defineProperty(document, 'activeElement', { value: i, writable: false, configurable: true });
+    trapTab({key: 'Tab', shiftKey: true, preventDefault} as unknown as KeyboardEvent);
     expect(preventDefault).not.toHaveBeenCalled();
    });
+   it('brings focus back from outside', () => {
+    const outsideEl = assertDocument().createElement('button');
+    b.focus = jest.fn();
+    expect(b.focus).not.toHaveBeenCalled();
+    Object.defineProperty(document, 'activeElement', { value: outsideEl, writable: false, configurable: true });
+    preventDefault.mockClear();
+    trapTab({key: 'Tab', shiftKey: true, preventDefault} as unknown as KeyboardEvent);
+    expect(b.focus).toHaveBeenCalled();
+    expect(preventDefault).toHaveBeenCalled();
+   });
+
 });
 
 describe('onKeyHandler', () => {

src/app/reactUtils.ts

@@ -1,4 +1,4 @@
-import { Document, Element, FocusEvent, HTMLElement, Event,
+import type { Document, Element, FocusEvent, HTMLElement, Event,
   HTMLElementEventMap, KeyboardEvent, MediaQueryListEvent } from '@openstax/types/lib.dom';
 import React from 'react';
 import { addSafeEventListener } from './domUtils';
@@ -18,27 +18,69 @@ export const useDrawFocus = <E extends HTMLElement = HTMLElement>() => {
   return ref;
 };
 
+function isHidden(el: HTMLElement) {
+  return el.offsetWidth === 0 && el.offsetHeight === 0;
+}
+
+const focusableItemQuery = [
+  'button', 'input', 'select', 'textarea', '[href]', '[tabindex]:not([tabindex="-1"]',
+].map((s) => s.includes('[') ? s : `${s}:not([disabled])`).join(',');
+
+function ringAdd(arr: unknown[], a: number, b: number) {
+  return (arr.length + a + b) % arr.length;
+}
+
 // Exported to allow testing
-export function createTrapTab(ref: React.MutableRefObject<HTMLElement | null>) {
+export function createTrapTab(...elements: HTMLElement[]) {
+  const focusableElements = elements
+  .filter(c => c  && 'querySelectorAll' in c) // in some tests, this gets garbage
+  .map(
+    (container) => {
+      const contents = Array.from(container.querySelectorAll<HTMLElement>(focusableItemQuery))
+        .filter((el) => !isHidden(el));
+
+      return {
+        container,
+        firstEl: contents[0],
+        lastEl: contents[contents.length - 1],
+      };
+    }
+  ).filter((c) => c.firstEl);
+
+  if (focusableElements.length === 0) {
+    return () => null;
+  }
+
+  // A typical implementation, adapted for crossing multiple containers
+  // e.g. https://hidde.blog/using-javascript-to-trap-focus-in-an-element/
   return (event: KeyboardEvent) => {
     if (event.key !== 'Tab') { return; }
 
-    const el = ref.current;
-    if (!el) { return; }
+    // Keep track of where we came from
+    const startEl = document?.activeElement as HTMLElement;
+    const feEntry = focusableElements.find((entry) => entry.container.contains(startEl));
+
+    // Focus has escaped the trap
+    if (!feEntry) {
+      focusableElements[0].firstEl.focus();
+      event.preventDefault();
+      return;
+    }
 
-    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);
+    // Move to the next container
+    if (event.shiftKey) {
+      if (startEl === feEntry.firstEl) {
+        const feIdx = focusableElements.indexOf(feEntry);
+        const newIdx = ringAdd(focusableElements, feIdx, -1);
 
-    if (focusableElements.length === 0) { return; }
-    const firstFocusableElement = focusableElements[0];
-    const lastFocusableElement = focusableElements[focusableElements.length - 1];
+        focusableElements[newIdx].lastEl.focus();
+        event.preventDefault();
+      }
+    } else if (startEl === feEntry.lastEl) {
+      const feIdx = focusableElements.indexOf(feEntry);
+      const newIdx = ringAdd(focusableElements, feIdx, 1);
 
-    if (event.shiftKey && document?.activeElement === firstFocusableElement) {
-      lastFocusableElement.focus();
-      event.preventDefault();
-    } else if (document?.activeElement === lastFocusableElement) {
-      firstFocusableElement.focus();
+      focusableElements[newIdx].firstEl.focus();
       event.preventDefault();
     }
   };
@@ -47,7 +89,11 @@ export function createTrapTab(ref: React.MutableRefObject<HTMLElement | null>) {
 export function useTrapTabNavigation(ref: React.MutableRefObject<HTMLElement | null>) {
   React.useEffect(
     () => {
-      const trapTab = createTrapTab(ref);
+      const el = ref.current;
+      if (!el) {
+        return;
+      }
+      const trapTab = createTrapTab(el);
 
       document?.addEventListener('keydown', trapTab, true);