Change useTrapFocus API

packages/store-ui/src/molecules/Modal/Modal.test.tsx

@@ -102,15 +102,15 @@ describe('Modal WAI-ARIA Specifications', () => {
       shiftKey: true,
     })
 
-    fireEvent.focus(getByTestId('sentinelStart'))
+    fireEvent.focus(getByTestId('beforeElement'))
     expect(getByTestId('first-button')).toHaveFocus()
 
     // Simulate loop back: from last to first element
     fireEvent.keyDown(document.activeElement!, {
       key: 'Tab',
     })
 
-    fireEvent.focus(getByTestId('sentinelEnd'))
+    fireEvent.focus(getByTestId('afterElement'))
     expect(getByTestId('first-input')).toHaveFocus()
   })
 
@@ -139,7 +139,7 @@ describe('Modal WAI-ARIA Specifications', () => {
       shiftKey: true,
     })
 
-    fireEvent.focus(getAllByTestId('sentinelStart')[1])
+    fireEvent.focus(getAllByTestId('beforeElement')[1])
     const [firstButton, secondButton] = getAllByTestId('first-button')
 
     expect(secondButton).toHaveFocus()

packages/store-ui/src/molecules/Modal/Modal.tsx

@@ -33,7 +33,7 @@ export interface ModalProps extends ModalContentProps {
   isOpen: boolean
 }
 
-/**
+/*
  * This component is based on @reach/dialog.
  * https://github.com/reach/reach-ui/blob/main/packages/dialog/src/index.tsx
  * https://reach.tech/dialog

packages/store-ui/src/molecules/Modal/ModalContent.tsx

@@ -1,7 +1,3 @@
-/* eslint-disable jsx-a11y/no-noninteractive-tabindex */
-/*
- * Disable the eslint rule to use sentinel html elements that aren't interactive.
- */
 import type {
   DetailedHTMLProps,
   HTMLAttributes,
@@ -17,67 +13,52 @@ interface ModalContentPureProps
     DetailedHTMLProps<HTMLAttributes<HTMLDivElement>, HTMLDivElement>,
     'ref'
   > {
-  sentinelStartRef: RefObject<HTMLDivElement>
   trapFocusRef: RefObject<HTMLDivElement>
-  sentinelEndRef: RefObject<HTMLDivElement>
   testId?: string
 }
 
 const ModalContentPure = ({
   trapFocusRef,
-  sentinelStartRef,
-  sentinelEndRef,
   testId = 'store-modal-content',
   children,
   ...props
 }: ModalContentPureProps) => {
   return (
-    <>
-      <div tabIndex={0} data-testid="sentinelStart" ref={sentinelStartRef} />
-      {/*
-       * This next line is disabled due to the onClick prop.
-       * Even though a div isn't clickable, the onClick is required to prevent event bubbles
-       * from reaching the Overlay, which calls onDismiss inside the onClick handler when it's clicked.
-       */}
-      {/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */}
-      <div
-        data-store-modal-content
-        {...props}
-        data-testid={testId}
-        ref={trapFocusRef}
-        aria-modal="true"
-        role="dialog"
-        tabIndex={-1}
-      >
-        {children}
-      </div>
-      <div tabIndex={0} data-testid="sentinelEnd" ref={sentinelEndRef} />
-    </>
+    /*
+     * This next line is disabled due to the onClick prop.
+     * Even though a div isn't clickable, the onClick is required to prevent event bubbles
+     * from reaching the Overlay, which calls onDismiss inside the onClick handler when it's clicked.
+     */
+    <div
+      data-store-modal-content
+      {...props}
+      data-testid={testId}
+      ref={trapFocusRef}
+      aria-modal="true"
+      role="dialog"
+      tabIndex={-1}
+    >
+      {children}
+    </div>
   )
 }
 
 export type ModalContentProps = Omit<
   ModalContentPureProps,
-  'sentinelStartRef' | 'sentinelEndRef' | 'trapFocusRef' | 'onClick'
+  'trapFocusRef' | 'onClick'
 >
 
 const ModalContent = ({ children, ...props }: ModalContentProps) => {
   const trapFocusRef = useRef<HTMLDivElement>(null)
-  const sentinelStartRef = useRef<HTMLDivElement>(null)
-  const sentinelEndRef = useRef<HTMLDivElement>(null)
 
   useTrapFocus({
-    sentinelStartRef,
-    sentinelEndRef,
     trapFocusRef,
   })
 
   return (
     <ModalContentPure
       {...props}
       trapFocusRef={trapFocusRef}
-      sentinelStartRef={sentinelStartRef}
-      sentinelEndRef={sentinelEndRef}
       onClick={(event: MouseEvent) => {
         event.stopPropagation()
       }}

packages/store-ui/src/molecules/Modal/useTrapFocus.ts

@@ -4,29 +4,56 @@ import type { FocusableElement } from 'tabbable'
 import { tabbable } from 'tabbable'
 
 interface TrapFocusParams {
-  sentinelStartRef: RefObject<HTMLElement>
   trapFocusRef: RefObject<HTMLElement>
-  sentinelEndRef: RefObject<HTMLElement>
 }
 
-/**
+/*
  * Element that will maintain the focus inside trapFocusRef, focus the first element,
  * and focus back on the element that was in focus when useTrapFocus was triggered.
  *
- * Inspired by Material UI TrapFocus https://github.com/mui-org/material-ui/blob/master/packages/mui-core/src/Unstable_TrapFocus/Unstable_TrapFocus.js
+ * Inspired by Reakit useTrapFocus https://github.com/reakit/reakit/blob/a211d94da9f3b683182568a56479b91afb1b85ae/packages/reakit/src/Dialog/__utils/useFocusTrap.ts
  */
-const useTrapFocus = ({
-  trapFocusRef,
-  sentinelStartRef,
-  sentinelEndRef,
-}: TrapFocusParams) => {
+const useTrapFocus = ({ trapFocusRef }: TrapFocusParams) => {
+  // #L31
   const tabbableNodesRef = useRef<FocusableElement[]>()
-  const lastKeyDownRef = useRef<KeyboardEvent>()
 
+  const beforeElementRef = useRef<HTMLDivElement | null>(null)
+  const afterElementRef = useRef<HTMLDivElement | null>(null)
   const nodeToRestoreRef = useRef<HTMLElement | null>(
     document.hasFocus() ? (document.activeElement as HTMLElement) : null
   )
 
+  // Add before and after elements, and set refs.
+  useEffect(() => {
+    if (!trapFocusRef.current) {
+      return
+    }
+
+    if (!beforeElementRef.current) {
+      const beforeElement = document.createElement('div')
+
+      beforeElement.tabIndex = 0
+      beforeElement.setAttribute('aria-hidden', 'true')
+      beforeElement.setAttribute('data-testid', 'beforeElement')
+      beforeElementRef.current = beforeElement
+    }
+
+    if (!afterElementRef.current) {
+      afterElementRef.current = beforeElementRef.current.cloneNode() as HTMLDivElement
+      afterElementRef.current.setAttribute('data-testid', 'afterElement')
+    }
+
+    trapFocusRef.current.insertAdjacentElement(
+      'beforebegin',
+      beforeElementRef.current
+    )
+
+    trapFocusRef.current.insertAdjacentElement(
+      'afterend',
+      afterElementRef.current
+    )
+  }, [beforeElementRef, afterElementRef, trapFocusRef])
+
   // Focus back on the element that was focused when useTrapFocus is triggered.
   useEffect(() => {
     const nodeToRestore = nodeToRestoreRef.current
@@ -59,53 +86,54 @@ const useTrapFocus = ({
 
   // Handle loop focus. Set keydown and focusin event listeners
   useEffect(() => {
+    if (
+      !trapFocusRef.current ||
+      !beforeElementRef.current ||
+      !afterElementRef.current
+    ) {
+      return
+    }
+
+    const beforeElement = beforeElementRef.current
+    const afterElement = afterElementRef.current
+    const trapFocus = trapFocusRef.current
+
     const handleLoopFocus = (nativeEvent: FocusEvent) => {
-      if (
-        lastKeyDownRef.current?.key !== 'Tab' ||
-        !trapFocusRef.current ||
-        !document.hasFocus()
-      ) {
+      if (!document.hasFocus()) {
         return
       }
 
-      if (trapFocusRef.current.contains(nativeEvent.target as Node)) {
-        return
-      }
+      tabbableNodesRef.current = tabbable(trapFocusRef.current!)
 
-      tabbableNodesRef.current = tabbable(trapFocusRef.current)
+      if (!tabbableNodesRef.current.length) {
+        trapFocus.focus()
+      }
 
       /*
-       * Handle loop focus from sentinelStartRef. This node can only be focused if the user press shift tab.
+       * Handle loop focus from beforeElementRef. This node can only be focused if the user press shift tab.
        * It will focus the last element of the trapFocusRef.
        */
-      if (
-        nativeEvent.target === sentinelStartRef.current &&
-        lastKeyDownRef.current.shiftKey
-      ) {
+      if (nativeEvent.target === beforeElement) {
         tabbableNodesRef.current[tabbableNodesRef.current.length - 1]?.focus()
       }
 
       /*
-       * Handle loop focus from sentinelEndRef. This node can only be focused if the user press tab.
+       * Handle loop focus from afterElementRef. This node can only be focused if the user press tab.
        * It will focus the first element of the trapFocusRef.
        */
-      if (nativeEvent.target === sentinelEndRef.current) {
+      if (nativeEvent.target === afterElement) {
         tabbableNodesRef.current[0]?.focus()
       }
     }
 
-    const handleKeyDown = (nativeEvent: KeyboardEvent) => {
-      lastKeyDownRef.current = nativeEvent
-    }
-
-    document.addEventListener('focusin', handleLoopFocus)
-    document.addEventListener('keydown', handleKeyDown, true)
+    beforeElement?.addEventListener('focusin', handleLoopFocus)
+    afterElement?.addEventListener('focusin', handleLoopFocus)
 
     return () => {
-      document.removeEventListener('focusin', handleLoopFocus)
-      document.removeEventListener('keydown', handleKeyDown, true)
+      beforeElement?.removeEventListener('focusin', handleLoopFocus)
+      afterElement?.removeEventListener('focusin', handleLoopFocus)
     }
-  }, [tabbableNodesRef, sentinelEndRef, sentinelStartRef, trapFocusRef])
+  }, [tabbableNodesRef, afterElementRef, beforeElementRef, trapFocusRef])
 }
 
 export default useTrapFocus