Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IterateFocusableElements options to focusZone #235

Merged
merged 5 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/neat-timers-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/behaviors': minor
---

Add IterateFocusableElements options to focusZone
32 changes: 26 additions & 6 deletions src/__tests__/focus-trap.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@ beforeAll(() => {
getClientRects: {
get: () => () => [42],
},
offsetParent: {
get() {
// eslint-disable-next-line @typescript-eslint/no-this-alias
for (let element = this; element; element = element.parentNode) {
if (element.style?.display?.toLowerCase() === 'none') {
return null
}
}

if (this.style?.position?.toLowerCase() === 'fixed') {
return null
}

if (this.tagName.toLowerCase() in ['html', 'body']) {
return null
}

return this.parentNode
},
},
})
} catch {
// ignore
Expand All @@ -40,7 +60,7 @@ it('Should initially focus the first element when activated', () => {
const controller = focusTrap(trapContainer)
expect(document.activeElement).toEqual(firstButton)

controller.abort()
controller?.abort()
})

it('Should initially focus the initialFocus element when specified', () => {
Expand All @@ -57,7 +77,7 @@ it('Should initially focus the initialFocus element when specified', () => {
const controller = focusTrap(trapContainer, secondButton)
expect(document.activeElement).toEqual(secondButton)

controller.abort()
controller?.abort()
})

it('Should prevent focus from exiting the trap, returns focus to first element', async () => {
Expand Down Expand Up @@ -86,7 +106,7 @@ it('Should prevent focus from exiting the trap, returns focus to first element',
await user.tab()
expect(document.activeElement).toEqual(firstButton)

controller.abort()
controller?.abort()

lastButton.focus()
await user.tab()
Expand Down Expand Up @@ -125,7 +145,7 @@ it('Should cycle focus from last element to first element and vice-versa', async
await user.tab({shift: true})
expect(document.activeElement).toEqual(lastButton)

controller.abort()
controller?.abort()
})

it('Should should release the trap when the signal is aborted', async () => {
Expand Down Expand Up @@ -154,7 +174,7 @@ it('Should should release the trap when the signal is aborted', async () => {
await user.tab()
expect(document.activeElement).toEqual(firstButton)

controller.abort()
controller?.abort()

lastButton.focus()
await user.tab()
Expand Down Expand Up @@ -218,5 +238,5 @@ it('Should handle dynamic content', async () => {
await user.tab({shift: true})
expect(document.activeElement).toEqual(secondButton)

controller.abort()
controller?.abort()
})
80 changes: 80 additions & 0 deletions src/__tests__/focus-zone.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ beforeAll(() => {
getClientRects: {
get: () => () => [42],
},
offsetParent: {
get() {
// eslint-disable-next-line @typescript-eslint/no-this-alias
for (let element = this; element; element = element.parentNode) {
if (element.style?.display?.toLowerCase() === 'none') {
return null
}
}

if (this.style?.position?.toLowerCase() === 'fixed') {
return null
}

if (this.tagName.toLowerCase() in ['html', 'body']) {
return null
}

return this.parentNode
},
},
})
} catch {
// ignore
Expand Down Expand Up @@ -569,3 +589,63 @@ it('Should handle elements being reordered', async () => {

controller.abort()
})

it('Should ignore hidden elements if strict', async () => {
const user = userEvent.setup()
const {container} = render(
<div id="focusZone">
<button>Apple</button>
<button style={{visibility: 'hidden'}}>Banana</button>
<button style={{display: 'none'}}>Watermelon</button>
<div style={{visibility: 'hidden'}}>
<div>
<button>Cherry</button>
</div>
</div>
<div style={{display: 'none'}}>
<div>
<button>Peach</button>
</div>
</div>
<button tabIndex={-1}>Cantaloupe</button>
</div>,
)
const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
const allButtons = focusZoneContainer.querySelectorAll('button')
const firstButton = allButtons[0]
const lastButton = allButtons[allButtons.length - 1]
const controller = focusZone(focusZoneContainer, {strict: true})

firstButton.focus()
expect(document.activeElement).toEqual(firstButton)

await user.keyboard('{arrowdown}')
expect(document.activeElement).toEqual(lastButton)

controller.abort()
})

it('Shoud move to tabbable elements if onlyTabbable', async () => {
const user = userEvent.setup()
const {container} = render(
<div id="focusZone">
<button>Apple</button>
<button tabIndex={-1}>Cherry</button>
<button tabIndex={0}>Cantaloupe</button>
</div>,
)

const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
const allButtons = focusZoneContainer.querySelectorAll('button')
const firstButton = allButtons[0]
const lastButton = allButtons[allButtons.length - 1]
const controller = focusZone(focusZoneContainer, {onlyTabbable: true})

firstButton.focus()
expect(document.activeElement).toEqual(firstButton)

await user.keyboard('{arrowdown}')
expect(document.activeElement).toEqual(lastButton)

controller.abort()
})
67 changes: 67 additions & 0 deletions src/__tests__/iterate-focusable-elements.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,46 @@ import React from 'react'
import {iterateFocusableElements} from '../utils/iterate-focusable-elements.js'
import {render} from '@testing-library/react'

// Since we use strict checks for size and parent, we need to mock these
// properties that Jest does not populate.
beforeAll(() => {
try {
Object.defineProperties(HTMLElement.prototype, {
offsetHeight: {
get: () => 42,
},
offsetWidth: {
get: () => 42,
},
getClientRects: {
get: () => () => [42],
},
offsetParent: {
get() {
// eslint-disable-next-line @typescript-eslint/no-this-alias
for (let element = this; element; element = element.parentNode) {
if (element.style?.display?.toLowerCase() === 'none') {
return null
}
}

if (this.style?.position?.toLowerCase() === 'fixed') {
return null
}

if (this.tagName.toLowerCase() in ['html', 'body']) {
return null
}

return this.parentNode
},
},
})
} catch {
// ignore
}
})

it('Should iterate through focusable elements only', () => {
const {container} = render(
<div>
Expand Down Expand Up @@ -57,3 +97,30 @@ it('Should iterate through focusable elements in reverse', () => {
expect(focusable[3].tagName.toLowerCase()).toEqual('input')
expect(focusable[4].tagName.toLowerCase()).toEqual('textarea')
})

it('Should ignore hidden elements if strict', async () => {
const {container} = render(
<div>
<button>Apple</button>
<button style={{visibility: 'hidden'}}>Banana</button>
<button style={{display: 'none'}}>Watermelon</button>
<div style={{visibility: 'hidden'}}>
<div>
<button>Cherry</button>
</div>
</div>
<div style={{display: 'none'}}>
<div>
<button>Peach</button>
</div>
</div>
<button tabIndex={-1}>Cantaloupe</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something. Doesn't this make it non focusable with keyboard? How does this button have a place in the focusable array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The iterateFocusableElements will return "any element with tabindex explicitly set can be focusable, even if it's set to "-1"" if onlyTabbable option is false

</div>,
)
const focusable = Array.from(iterateFocusableElements(container as HTMLElement, {strict: true}))
expect(focusable.length).toEqual(2)
expect(focusable[0].tagName.toLowerCase()).toEqual('button')
expect(focusable[0].textContent).toEqual('Apple')
expect(focusable[1].tagName.toLowerCase()).toEqual('button')
expect(focusable[1].textContent).toEqual('Cantaloupe')
})
15 changes: 10 additions & 5 deletions src/focus-zone.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {polyfill as eventListenerSignalPolyfill} from './polyfills/event-listener-signal.js'
import {isMacOS} from './utils/user-agent.js'
import {iterateFocusableElements} from './utils/iterate-focusable-elements.js'
import {IterateFocusableElements, iterateFocusableElements} from './utils/iterate-focusable-elements.js'
import {uniqueId} from './utils/unique-id.js'

eventListenerSignalPolyfill()
Expand Down Expand Up @@ -114,7 +114,7 @@ const KEY_TO_DIRECTION = {
/**
* Options that control the behavior of the arrow focus behavior.
*/
export interface FocusZoneSettings {
export type FocusZoneSettings = IterateFocusableElements & {
/**
* Choose the behavior applied in cases where focus is currently at either the first or
* last element of the container.
Expand Down Expand Up @@ -504,8 +504,13 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
}
}

const iterateFocusableElementsOptions: IterateFocusableElements = {
reverse: settings?.reverse,
strict: settings?.strict,
onlyTabbable: settings?.onlyTabbable,
}
// Take all tabbable elements within container under management
beginFocusManagement(...iterateFocusableElements(container))
beginFocusManagement(...iterateFocusableElements(container, iterateFocusableElementsOptions))

// Open the first tabbable element for tabbing
const initialElement =
Expand All @@ -519,14 +524,14 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
for (const mutation of mutations) {
for (const removedNode of mutation.removedNodes) {
if (removedNode instanceof HTMLElement) {
endFocusManagement(...iterateFocusableElements(removedNode))
endFocusManagement(...iterateFocusableElements(removedNode, iterateFocusableElementsOptions))
}
}
}
for (const mutation of mutations) {
for (const addedNode of mutation.addedNodes) {
if (addedNode instanceof HTMLElement) {
beginFocusManagement(...iterateFocusableElements(addedNode))
beginFocusManagement(...iterateFocusableElements(addedNode, iterateFocusableElementsOptions))
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/utils/iterate-focusable-elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ export function isFocusable(elem: HTMLElement, strict = false): boolean {
// Each of the conditions checked below require a reflow, thus are gated by the `strict`
// argument. If any are true, the element is not focusable, even if tabindex is set.
if (strict) {
const style = getComputedStyle(elem)
const sizeInert = elem.offsetWidth === 0 || elem.offsetHeight === 0
const visibilityInert = ['hidden', 'collapse'].includes(getComputedStyle(elem).visibility)
const visibilityInert = ['hidden', 'collapse'].includes(style.visibility)
const displayInert = style.display === 'none' || !elem.offsetParent
const clientRectsInert = elem.getClientRects().length === 0
if (sizeInert || visibilityInert || clientRectsInert) {
if (sizeInert || visibilityInert || clientRectsInert || displayInert) {
return false
}
}
Expand Down
Loading