Merge branch 'main' into link-polish-books-back-to-openstax.pl

src/app/content/highlights/components/CardWrapper.tsx

@@ -56,6 +56,9 @@ const Wrapper = ({highlights, className, container, highlighter}: WrapperProps)
 
   useKeyCombination(highlightKeyCombination, moveFocus, noopKeyCombinationHandler([container, element]));
 
+  // Allow to move back to highlight from EditCard using Escape key
+  useKeyCombination({key: 'Escape'}, moveFocus);
+
   // Clear shouldFocusCard when focus is lost from the CardWrapper.
   // If we don't do this then card related for the focused highlight will be focused automatically.
   useFocusLost(element, shouldFocusCard, React.useCallback(() => setShouldFocusCard(false), [setShouldFocusCard]));

src/app/content/highlights/components/EditCard.tsx

@@ -8,7 +8,7 @@ import styled, { css } from 'styled-components/macro';
 import { useAnalyticsEvent } from '../../../../helpers/analytics';
 import * as selectAuth from '../../../auth/selectors';
 import Button, { ButtonGroup } from '../../../components/Button';
-import { useFocusElement, useOnEsc } from '../../../reactUtils';
+import { useFocusElement, useOnEsc, useTrapTabNavigation } from '../../../reactUtils';
 import theme from '../../../theme';
 import { assertDefined, assertWindow, mergeRefs } from '../../../utils';
 import { highlightStyles } from '../../constants';
@@ -174,8 +174,12 @@ function ActiveEditCard({
   const onColorChange = useOnColorChange(props);
   const saveAnnotation = useSaveAnnotation(props, element, pendingAnnotation);
 
+  const ref = React.useRef<HTMLElement>(null);
+
+  useTrapTabNavigation(ref, editingAnnotation);
+
   return (
-    <HiddenOnMobile>
+    <HiddenOnMobile ref={ref}>
       <ColorPicker
         color={props.data?.color}
         onChange={onColorChange}

src/app/reactUtils.spec.tsx

@@ -272,7 +272,6 @@ describe('useTrapTabNavigation', () => {
   it('attaches listeners', () => {
     const tr = renderer.create(<Component />);
 
-    expect(addEventListener).toHaveBeenCalled();
     renderer.act(
       () => {
         dispatchKeyDownEvent({key: 'Tab'});
@@ -341,7 +340,12 @@ describe('createTrapTab', () => {
     expect(b.focus).toHaveBeenCalled();
     expect(preventDefault).toHaveBeenCalled();
    });
+   it('short circuits when no focusable elements', () => {
+    const emptyEl = assertDocument().createElement('div');
 
+    trapTab = utils.createTrapTab(emptyEl);
+    trapTab({key: 'Tab', shiftKey: true, preventDefault} as unknown as KeyboardEvent);
+   });
 });
 
 describe('onKeyHandler', () => {

src/app/reactUtils.ts

@@ -86,21 +86,26 @@ export function createTrapTab(...elements: HTMLElement[]) {
   };
 }
 
-export function useTrapTabNavigation(ref: React.MutableRefObject<HTMLElement | null>) {
-  React.useEffect(
-    () => {
-      const el = ref.current;
-      if (!el) {
-        return;
-      }
-      const trapTab = createTrapTab(el);
-
-      document?.addEventListener('keydown', trapTab, true);
-
-      return () => document?.removeEventListener('keydown', trapTab, true);
-    },
-    [ref]
-  );
+  // Supply otherDep in cases where the focusable elements might change due to
+  // additional dependencies (combine them into one variable): see EditCard
+  export function useTrapTabNavigation(
+    ref: React.MutableRefObject<HTMLElement | null>,
+    otherDep?: unknown
+  ) {
+    React.useEffect(
+      () => {
+        const el = ref.current;
+        if (!el?.addEventListener) {
+          return;
+        }
+        const trapTab = createTrapTab(el);
+
+        el.addEventListener('keydown', trapTab, true);
+
+        return () => el.removeEventListener('keydown', trapTab, true);
+      },
+      [ref, otherDep]
+    );
 }
 
 export const onFocusInOrOutHandler = (