Merge pull request #473 from primer/tylerjdev/fix-duplicate-sentinels

Focus-trap: Prevent duplicate sentinels

Author: TylerJDev

.changeset/large-poets-run.md

@@ -0,0 +1,5 @@
+---
+'@primer/behaviors': patch
+---
+
+Prevent duplicate sentinels from being added if some already exist in the container of the focus trap.

src/__tests__/focus-trap.test.tsx

@@ -328,3 +328,42 @@ it('should remove the mutation observer when the focus trap is released', async
   expect(document.activeElement).not.toEqual(newLastButton)
   expect(trapContainer.lastElementChild).toEqual(newLastButton)
 })
+
+it('Should only have one set of sentinels', async () => {
+  const {container} = render(
+    <div>
+      <div id="trapContainer">
+        <button tabIndex={0}>Apple</button>
+      </div>
+      <button id="durian" tabIndex={0}>
+        Durian
+      </button>
+    </div>,
+  )
+
+  const trapContainer = container.querySelector<HTMLElement>('#trapContainer')!
+  const durianButton = container.querySelector<HTMLElement>('#durian')!
+  const firstButton = trapContainer.querySelector('button')!
+
+  focusTrap(trapContainer)
+
+  durianButton.focus()
+  expect(document.activeElement).toEqual(firstButton)
+
+  trapContainer.insertAdjacentHTML(
+    'afterbegin',
+    '<div id="newTrapContainer"><button id="newFirst" tabindex="0">New first button</button></div>',
+  )
+
+  const newTrapContainer = trapContainer.querySelector<HTMLElement>('#newTrapContainer')!
+  const newFirstButton = newTrapContainer.querySelector<HTMLElement>('#newFirst')!
+
+  const controller = focusTrap(newTrapContainer)
+  expect(document.activeElement).toEqual(newFirstButton)
+  expect(document.querySelectorAll('.sentinel').length).toEqual(4)
+
+  controller?.abort()
+
+  trapContainer.removeChild(newTrapContainer)
+  expect(document.querySelectorAll('.sentinel').length).toEqual(2)
+})

src/focus-trap.ts

@@ -98,8 +98,17 @@ export function focusTrap(
     const firstFocusableChild = getFocusableChild(container)
     firstFocusableChild?.focus()
   }
-  container.prepend(sentinelStart)
-  container.append(sentinelEnd)
+
+  // If the container already has sentinels as direct children, don't add more.
+  // The mutation observer will take care of moving existing sentinels to the correct position.
+  const existingSentinels = Array.from(container.children).filter(
+    e => e.classList.contains('sentinel') && e.tagName === 'SPAN',
+  )
+
+  if (!existingSentinels.length) {
+    container.prepend(sentinelStart)
+    container.append(sentinelEnd)
+  }
 
   const observer = observeFocusTrap(container, [sentinelStart, sentinelEnd])