From 04fa6a8eecc0ebb9e108324e2194826073d9323b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Tue, 6 Aug 2024 17:17:13 -0400 Subject: [PATCH 1/9] Add observer to focus-trap behavior --- src/focus-trap.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index c3de92e..1af2e99 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -30,6 +30,25 @@ function followSignal(signal: AbortSignal): AbortController { return controller } +function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { + const observer = new MutationObserver(mutations => { + for (const mutation of mutations) { + if (mutation.type === 'childList' && mutation.addedNodes.length) { + // If the first and last children of container aren't sentinels, move them to the start and end + const firstChild = container.firstElementChild + const lastChild = container.lastElementChild + + if (!firstChild?.classList.contains('sentinel')) container.insertAdjacentElement('afterbegin', sentinels[0]) + if (!lastChild?.classList.contains('sentinel')) container.insertAdjacentElement('beforeend', sentinels[1]) + } + } + }) + + observer.observe(container, {childList: true}) + + return observer +} + /** * Traps focus within the given container * @param container The container in which to trap focus @@ -67,6 +86,8 @@ export function focusTrap( container.prepend(sentinelStart) container.append(sentinelEnd) + const observer = observeFocusTrap(container, [sentinelStart, sentinelEnd]) + let lastFocusedChild: HTMLElement | undefined = undefined // Ensure focus remains in the trap zone by checking that a given recently-focused // element is inside the trap zone. If it isn't, redirect focus to a suitable @@ -117,6 +138,7 @@ export function focusTrap( if (suspendedTrapIndex >= 0) { suspendedTrapStack.splice(suspendedTrapIndex, 1) } + observer.disconnect() tryReactivate() }) From 1729a68fc4d51d5b9983391c7c54e98c07217d42 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 7 Aug 2024 09:34:00 -0400 Subject: [PATCH 2/9] Add test --- src/__tests__/focus-trap.test.tsx | 88 +++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/__tests__/focus-trap.test.tsx b/src/__tests__/focus-trap.test.tsx index 68ed72e..0ad0039 100644 --- a/src/__tests__/focus-trap.test.tsx +++ b/src/__tests__/focus-trap.test.tsx @@ -240,3 +240,91 @@ it('Should handle dynamic content', async () => { controller?.abort() }) + +it('should keep the sentinel elements at the start/end of the inner container', async () => { + const user = userEvent.setup() + const {container} = render( +
+
+ + + +
+ +
, + ) + + const trapContainer = container.querySelector('#trapContainer')! + const [firstButton, secondButton] = trapContainer.querySelectorAll('button') + const controller = focusTrap(trapContainer) + + secondButton.focus() + await user.tab() + await user.tab() + expect(document.activeElement).toEqual(firstButton) + + trapContainer.insertAdjacentHTML('afterbegin', '') + const newFirstButton = trapContainer.querySelector('#first') + + const sentinelStart = trapContainer.querySelector('.sentinel') + + await user.tab({shift: true}) + expect(trapContainer.firstElementChild).toEqual(sentinelStart) + expect(document.activeElement).toEqual(newFirstButton) + + trapContainer.insertAdjacentHTML('beforeend', '') + const newLastButton = trapContainer.querySelector('#last') + + const sentinelEnd = trapContainer.querySelector('.sentinel') + + await user.tab({shift: true}) + expect(trapContainer.lastElementChild).toEqual(sentinelEnd) + expect(document.activeElement).toEqual(newLastButton) + + controller?.abort() +}) + +it('should remove the mutation observer when the focus trap is released', async () => { + const user = userEvent.setup() + const {container} = render( +
+
+ + + +
+ +
, + ) + + const trapContainer = container.querySelector('#trapContainer')! + const [firstButton, secondButton] = trapContainer.querySelectorAll('button') + const controller = focusTrap(trapContainer) + + secondButton.focus() + await user.tab() + await user.tab() + expect(document.activeElement).toEqual(firstButton) + + trapContainer.insertAdjacentHTML('afterbegin', '') + const newFirstButton = trapContainer.querySelector('#first') + + const sentinelStart = trapContainer.querySelector('.sentinel') + + await user.tab({shift: true}) + expect(trapContainer.firstElementChild).toEqual(sentinelStart) + expect(document.activeElement).toEqual(newFirstButton) + + controller?.abort() + + trapContainer.insertAdjacentHTML('beforeend', '') + const newLastButton = trapContainer.querySelector('#last') + + await user.tab({shift: true}) + expect(document.activeElement).not.toEqual(newLastButton) + expect(trapContainer.lastElementChild).toEqual(newLastButton) +}) From 73302574bc90d3b7b46225039ef3cb81634202b5 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 7 Aug 2024 09:34:46 -0400 Subject: [PATCH 3/9] Edits to `observeFocusTrap` --- src/focus-trap.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index 1af2e99..e1589f3 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -38,8 +38,10 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { const firstChild = container.firstElementChild const lastChild = container.lastElementChild - if (!firstChild?.classList.contains('sentinel')) container.insertAdjacentElement('afterbegin', sentinels[0]) - if (!lastChild?.classList.contains('sentinel')) container.insertAdjacentElement('beforeend', sentinels[1]) + const [sentinelStart, sentinelEnd] = sentinels + + if (!firstChild?.classList.contains('sentinel')) container.insertAdjacentElement('afterbegin', sentinelStart) + if (!lastChild?.classList.contains('sentinel')) container.insertAdjacentElement('beforeend', sentinelEnd) } } }) From 4b729b084c161e2f81356fe706bf1b8243cf021e Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 7 Aug 2024 09:44:38 -0400 Subject: [PATCH 4/9] Add changeset --- .changeset/curly-lions-cheat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/curly-lions-cheat.md diff --git a/.changeset/curly-lions-cheat.md b/.changeset/curly-lions-cheat.md new file mode 100644 index 0000000..8d11fce --- /dev/null +++ b/.changeset/curly-lions-cheat.md @@ -0,0 +1,5 @@ +--- +'@primer/behaviors': patch +--- + +Adds mutation observer to `focus-trap` to ensure sentinel elements are always in the correct position From 6a888bf61f2c139b037f6f5f7792d625506056c4 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 14 Aug 2024 14:20:31 +0000 Subject: [PATCH 5/9] Add comment --- src/focus-trap.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index e1589f3..f66f72c 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -40,6 +40,7 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { const [sentinelStart, sentinelEnd] = sentinels + // Adds back sentinel to correct position in the DOM if (!firstChild?.classList.contains('sentinel')) container.insertAdjacentElement('afterbegin', sentinelStart) if (!lastChild?.classList.contains('sentinel')) container.insertAdjacentElement('beforeend', sentinelEnd) } From b33926fda5cf3d49c7d109f674d753a3b6098738 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 14 Aug 2024 15:29:42 +0000 Subject: [PATCH 6/9] Add logs to test performance --- src/focus-trap.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index f66f72c..edadab7 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -31,9 +31,13 @@ function followSignal(signal: AbortSignal): AbortController { } function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { + console.log('Observe Focus Trap Init') const observer = new MutationObserver(mutations => { + console.log('New mutation') for (const mutation of mutations) { + console.log('Mutations:', mutations) if (mutation.type === 'childList' && mutation.addedNodes.length) { + console.log('ChildList Mutation') // If the first and last children of container aren't sentinels, move them to the start and end const firstChild = container.firstElementChild const lastChild = container.lastElementChild @@ -41,8 +45,14 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { const [sentinelStart, sentinelEnd] = sentinels // Adds back sentinel to correct position in the DOM - if (!firstChild?.classList.contains('sentinel')) container.insertAdjacentElement('afterbegin', sentinelStart) - if (!lastChild?.classList.contains('sentinel')) container.insertAdjacentElement('beforeend', sentinelEnd) + if (!firstChild?.classList.contains('sentinel')) { + container.insertAdjacentElement('afterbegin', sentinelStart) + console.log('Adjust sentinel to start') + } + if (!lastChild?.classList.contains('sentinel')) { + container.insertAdjacentElement('beforeend', sentinelEnd) + console.log('Adjust sentinel to end') + } } } }) From 810400dfa86193dc1a7b597d154e19bb04a2650c Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 21 Aug 2024 16:22:12 +0000 Subject: [PATCH 7/9] Add additional check for mutation observer --- src/focus-trap.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index edadab7..c9e2e62 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -37,6 +37,15 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { for (const mutation of mutations) { console.log('Mutations:', mutations) if (mutation.type === 'childList' && mutation.addedNodes.length) { + const sentinelChildren = Array.from( + mutation.addedNodes, + e => e instanceof HTMLElement && e.classList.contains('sentinel') && e.tagName === 'SPAN', + ) + + // If any of the added nodes are sentinels, don't do anything + if (sentinelChildren.length) { + return + } console.log('ChildList Mutation') // If the first and last children of container aren't sentinels, move them to the start and end const firstChild = container.firstElementChild From 9c89a2797e818923c47f4ef6098db30a503180d2 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 21 Aug 2024 18:07:43 +0000 Subject: [PATCH 8/9] Remove logs --- src/focus-trap.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index c9e2e62..1e4a4b9 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -31,11 +31,8 @@ function followSignal(signal: AbortSignal): AbortController { } function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { - console.log('Observe Focus Trap Init') const observer = new MutationObserver(mutations => { - console.log('New mutation') for (const mutation of mutations) { - console.log('Mutations:', mutations) if (mutation.type === 'childList' && mutation.addedNodes.length) { const sentinelChildren = Array.from( mutation.addedNodes, @@ -46,7 +43,6 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { if (sentinelChildren.length) { return } - console.log('ChildList Mutation') // If the first and last children of container aren't sentinels, move them to the start and end const firstChild = container.firstElementChild const lastChild = container.lastElementChild @@ -56,11 +52,9 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { // Adds back sentinel to correct position in the DOM if (!firstChild?.classList.contains('sentinel')) { container.insertAdjacentElement('afterbegin', sentinelStart) - console.log('Adjust sentinel to start') } if (!lastChild?.classList.contains('sentinel')) { container.insertAdjacentElement('beforeend', sentinelEnd) - console.log('Adjust sentinel to end') } } } From 07e1664495e18e9957550ed3ddbdb7fa788802b2 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 21 Aug 2024 18:35:37 +0000 Subject: [PATCH 9/9] Change to filter --- src/focus-trap.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/focus-trap.ts b/src/focus-trap.ts index 1e4a4b9..395c35b 100644 --- a/src/focus-trap.ts +++ b/src/focus-trap.ts @@ -34,8 +34,7 @@ function observeFocusTrap(container: HTMLElement, sentinels: HTMLElement[]) { const observer = new MutationObserver(mutations => { for (const mutation of mutations) { if (mutation.type === 'childList' && mutation.addedNodes.length) { - const sentinelChildren = Array.from( - mutation.addedNodes, + const sentinelChildren = Array.from(mutation.addedNodes).filter( e => e instanceof HTMLElement && e.classList.contains('sentinel') && e.tagName === 'SPAN', )