Skip to content

Commit

Permalink
Merge pull request #462 from primer/clear_all_descendants
Browse files Browse the repository at this point in the history
[FocusZone] Clear all active descendants
  • Loading branch information
camertron authored Oct 21, 2024
2 parents 680bb31 + ee1e19d commit 394b0fe
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/long-lizards-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/behaviors': minor
---

Ensure focusZone() clears all potential active descendants
2 changes: 1 addition & 1 deletion rollup.config.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import typescript from '@rollup/plugin-typescript'
import packageJson from './package.json' assert {type: 'json'}
import packageJson from './package.json' with {type: 'json'}

const dependencyTypes = ['peerDependencies', 'dependencies', 'devDependencies']
const dependencies = new Set(
Expand Down
45 changes: 43 additions & 2 deletions src/__tests__/focus-zone.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import {FocusKeys, FocusZoneSettings, focusZone} from '../focus-zone.js'
import {
FocusKeys,
FocusZoneSettings,
activeDescendantActivatedDirectly,
focusZone,
isActiveDescendantAttribute,
} from '../focus-zone.js'
import {fireEvent, render} from '@testing-library/react'
import React from 'react'
import userEvent from '@testing-library/user-event'
Expand Down Expand Up @@ -431,6 +437,41 @@ it('Should focus-in to the first element if the last-focused element is removed'
controller.abort()
})

it('Should clear all active descendants when focus moves outside the zone', async () => {
const {container} = render(
<div>
<button tabIndex={0} id="outside">
Bad Apple
</button>
<input type="text" id="input" tabIndex={0} />
<div id="focusZone">
<button tabIndex={0}>Apple</button>
<button tabIndex={0}>Banana</button>
<button tabIndex={0}>Cantaloupe</button>
</div>
</div>,
)

const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
const control = container.querySelector<HTMLInputElement>('#input')!
const [firstButton, secondButton] = focusZoneContainer.querySelectorAll('button')
const outsideButton = container.querySelector<HTMLElement>('#outside')!
const controller = focusZone(focusZoneContainer, {activeDescendantControl: control})

control.focus()
const allActiveDescendants = focusZoneContainer.querySelectorAll(`[${isActiveDescendantAttribute}]`)
expect(allActiveDescendants.length).toEqual(1)
expect(allActiveDescendants[0]).toEqual(firstButton)

secondButton.setAttribute(isActiveDescendantAttribute, activeDescendantActivatedDirectly)
expect(focusZoneContainer.querySelectorAll(`[${isActiveDescendantAttribute}]`).length).toEqual(2)

outsideButton.focus()
expect(focusZoneContainer.querySelectorAll(`[${isActiveDescendantAttribute}]`).length).toEqual(0)

controller.abort()
})

it('Should call onActiveDescendantChanged properly', async () => {
const user = userEvent.setup()
const {container} = render(
Expand Down Expand Up @@ -625,7 +666,7 @@ it('Should ignore hidden elements if strict', async () => {
controller.abort()
})

it('Shoud move to tabbable elements if onlyTabbable', async () => {
it('Should move to tabbable elements if onlyTabbable', async () => {
const user = userEvent.setup()
const {container} = render(
<div id="focusZone">
Expand Down
53 changes: 35 additions & 18 deletions src/focus-zone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,11 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
activeDescendantControl?.removeAttribute('aria-activedescendant')
container.removeAttribute(hasActiveDescendantAttribute)
previouslyActiveElement?.removeAttribute(isActiveDescendantAttribute)

for (const item of container.querySelectorAll(`[${isActiveDescendantAttribute}]`)) {
item?.removeAttribute(isActiveDescendantAttribute)
}

activeDescendantCallback?.(undefined, previouslyActiveElement, false)
}

Expand Down Expand Up @@ -580,13 +585,17 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
)

if (activeDescendantControl) {
container.addEventListener('focusin', event => {
if (event.target instanceof HTMLElement && focusableElements.includes(event.target)) {
// Move focus to the activeDescendantControl if one of the descendants is focused
activeDescendantControl.focus({preventScroll})
updateFocusedElement(event.target)
}
})
container.addEventListener(
'focusin',
event => {
if (event.target instanceof HTMLElement && focusableElements.includes(event.target)) {
// Move focus to the activeDescendantControl if one of the descendants is focused
activeDescendantControl.focus({preventScroll})
updateFocusedElement(event.target)
}
},
{signal},
)
container.addEventListener(
'mousemove',
({target}) => {
Expand All @@ -604,17 +613,25 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
)

// Listeners specifically on the controlling element
activeDescendantControl.addEventListener('focusin', () => {
// Focus moved into the active descendant input. Activate current or first descendant.
if (!currentFocusedElement) {
updateFocusedElement(getFirstFocusableElement())
} else {
setActiveDescendant(undefined, currentFocusedElement)
}
})
activeDescendantControl.addEventListener('focusout', () => {
clearActiveDescendant()
})
activeDescendantControl.addEventListener(
'focusin',
() => {
// Focus moved into the active descendant input. Activate current or first descendant.
if (!currentFocusedElement) {
updateFocusedElement(getFirstFocusableElement())
} else {
setActiveDescendant(undefined, currentFocusedElement)
}
},
{signal},
)
activeDescendantControl.addEventListener(
'focusout',
() => {
clearActiveDescendant()
},
{signal},
)
} else {
// This is called whenever focus enters an element in the container
container.addEventListener(
Expand Down

0 comments on commit 394b0fe

Please sign in to comment.