Skip to content

Commit 19d4d3d

Browse files
authored
Merge pull request #263 from primer/tylerjdev/no-focus-if-hidden
Prevent focusing hidden/disabled elements
2 parents f0fbf65 + a54cafc commit 19d4d3d

File tree

3 files changed

+123
-0
lines changed

3 files changed

+123
-0
lines changed

.changeset/eighty-steaks-camp.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/behaviors': minor
3+
---
4+
5+
Adjusts mutation observer to now track `hidden` and `disabled` attributes being applied or removed.

src/__tests__/focus-zone.test.tsx

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,3 +649,105 @@ it('Shoud move to tabbable elements if onlyTabbable', async () => {
649649

650650
controller.abort()
651651
})
652+
653+
it('Should ignore hidden elements after focus zone is enabled', async () => {
654+
const user = userEvent.setup()
655+
const {container, rerender} = render(
656+
<div id="focusZone">
657+
<button tabIndex={0}>Apple</button>
658+
<button tabIndex={0}>Banana</button>
659+
<button tabIndex={0}>Cantaloupe</button>
660+
</div>,
661+
)
662+
663+
const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
664+
const [firstButton, , thirdButton] = focusZoneContainer.querySelectorAll('button')
665+
const controller = focusZone(focusZoneContainer)
666+
667+
firstButton.focus()
668+
expect(document.activeElement).toEqual(firstButton)
669+
670+
rerender(
671+
<div id="focusZone">
672+
<button tabIndex={0}>Apple</button>
673+
<button tabIndex={0} hidden>
674+
Banana
675+
</button>
676+
<button tabIndex={0}>Cantaloupe</button>
677+
</div>,
678+
)
679+
680+
await user.keyboard('{arrowdown}')
681+
expect(document.activeElement).toEqual(thirdButton)
682+
683+
controller.abort()
684+
})
685+
686+
it('Should respect unhidden elements after focus zone is enabled', async () => {
687+
const user = userEvent.setup()
688+
const {container, rerender} = render(
689+
<div id="focusZone">
690+
<button tabIndex={0}>Apple</button>
691+
<button tabIndex={0} hidden>
692+
Banana
693+
</button>
694+
<button tabIndex={0}>Cantaloupe</button>
695+
</div>,
696+
)
697+
698+
const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
699+
const [firstButton, secondButton, thirdButton] = focusZoneContainer.querySelectorAll('button')
700+
const controller = focusZone(focusZoneContainer)
701+
702+
firstButton.focus()
703+
expect(document.activeElement).toEqual(firstButton)
704+
705+
await user.keyboard('{arrowdown}')
706+
expect(document.activeElement).toEqual(thirdButton)
707+
708+
rerender(
709+
<div id="focusZone">
710+
<button tabIndex={0}>Apple</button>
711+
<button tabIndex={0}>Banana</button>
712+
<button tabIndex={0}>Cantaloupe</button>
713+
</div>,
714+
)
715+
716+
await user.keyboard('{arrowup}')
717+
expect(document.activeElement).toEqual(secondButton)
718+
719+
controller.abort()
720+
})
721+
722+
it('Should ignore disabled elements after focus zone is enabled', async () => {
723+
const user = userEvent.setup()
724+
const {container, rerender} = render(
725+
<div id="focusZone">
726+
<button tabIndex={0}>Apple</button>
727+
<button tabIndex={0}>Banana</button>
728+
<button tabIndex={0}>Cantaloupe</button>
729+
</div>,
730+
)
731+
732+
const focusZoneContainer = container.querySelector<HTMLElement>('#focusZone')!
733+
const [firstButton, , thirdButton] = focusZoneContainer.querySelectorAll('button')
734+
const controller = focusZone(focusZoneContainer)
735+
736+
firstButton.focus()
737+
expect(document.activeElement).toEqual(firstButton)
738+
739+
rerender(
740+
<div id="focusZone">
741+
<button tabIndex={0}>Apple</button>
742+
<button tabIndex={0} disabled>
743+
Banana
744+
</button>
745+
<button tabIndex={0}>Cantaloupe</button>
746+
</div>,
747+
)
748+
749+
await user.keyboard('{arrowdown}')
750+
expect(document.activeElement).toEqual(thirdButton)
751+
752+
controller.abort()
753+
})

src/focus-zone.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,19 +527,35 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
527527
endFocusManagement(...iterateFocusableElements(removedNode, iterateFocusableElementsOptions))
528528
}
529529
}
530+
// If an element is hidden or disabled, remove it from the list of focusable elements
531+
if (mutation.type === 'attributes' && mutation.oldValue === null) {
532+
if (mutation.target instanceof HTMLElement) {
533+
endFocusManagement(mutation.target)
534+
}
535+
}
530536
}
531537
for (const mutation of mutations) {
532538
for (const addedNode of mutation.addedNodes) {
533539
if (addedNode instanceof HTMLElement) {
534540
beginFocusManagement(...iterateFocusableElements(addedNode, iterateFocusableElementsOptions))
535541
}
536542
}
543+
544+
// Similarly, if an element is unhidden or "enabled", add it to the list of focusable elements
545+
// If `mutation.oldValue` is not null, then we may assume that the element was previously hidden or disabled
546+
if (mutation.type === 'attributes' && mutation.oldValue !== null) {
547+
if (mutation.target instanceof HTMLElement) {
548+
beginFocusManagement(mutation.target)
549+
}
550+
}
537551
}
538552
})
539553

540554
observer.observe(container, {
541555
subtree: true,
542556
childList: true,
557+
attributeFilter: ['hidden', 'disabled'],
558+
attributeOldValue: true,
543559
})
544560

545561
const controller = new AbortController()

0 commit comments

Comments
 (0)