diff --git a/.changeset/chilly-rabbits-invent.md b/.changeset/chilly-rabbits-invent.md new file mode 100644 index 00000000000..b7f27103eb8 --- /dev/null +++ b/.changeset/chilly-rabbits-invent.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +TreeView: Reliably move focus from loading item to first asynchronously loaded item diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index 8f14689e586..a8b230efa3c 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -1230,6 +1230,11 @@ describe('Asyncronous loading', () => { }) it('moves focus from loading item to first child', async () => { + // We get a focus zone warning in this test that doesn't + // happen in the browser. We're not sure why, so we're + // suppressing it for now. + jest.spyOn(console, 'warn').mockImplementation() + function TestTree() { const [state, setState] = React.useState('loading') @@ -1259,21 +1264,21 @@ describe('Asyncronous loading', () => { act(() => { // Focus first item parentItem.focus() - - // Press ↓ to move focus to loading item - fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) }) + // Press ↓ to move focus to loading item + fireEvent.keyDown(document.activeElement || document.body, {key: 'ArrowDown'}) + // Loading item should be focused expect(loadingItem).toHaveFocus() - // Wait for async loading to complete - const firstChild = await findByRole('treeitem', {name: 'Child 1'}) - act(() => { jest.runAllTimers() }) + // Wait for async loading to complete + const firstChild = await findByRole('treeitem', {name: 'Child 1'}) + // First child should be focused expect(firstChild).toHaveFocus() }) diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index dae141a63fa..b178db7c133 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -10,7 +10,6 @@ import styled, {keyframes} from 'styled-components' import {get} from '../constants' import {ConfirmationDialog} from '../Dialog/ConfirmationDialog' import {useControllableState} from '../hooks/useControllableState' -import useSafeTimeout from '../hooks/useSafeTimeout' import {useId} from '../hooks/useId' import Spinner from '../Spinner' import sx, {SxProp} from '../sx' @@ -510,17 +509,10 @@ export type TreeViewSubTreeProps = { const SubTree: React.FC = ({count, state, children}) => { const {announceUpdate} = React.useContext(RootContext) const {itemId, isExpanded, isSubTreeEmpty, setIsSubTreeEmpty} = React.useContext(ItemContext) - const [isLoadingItemVisible, setIsLoadingItemVisible] = React.useState(false) - const {safeSetTimeout} = useSafeTimeout() const loadingItemRef = React.useRef(null) const ref = React.useRef(null) - const [isPending, setPending] = React.useState(state === 'loading') - - React.useEffect(() => { - if (state === 'loading') { - setPending(true) - } - }, [state]) + const [loadingFocused, setLoadingFocused] = React.useState(false) + const previousState = usePreviousValue(state) React.useEffect(() => { // If `state` is undefined, we're working in a synchronous context and need @@ -536,61 +528,66 @@ const SubTree: React.FC = ({count, state, children}) => { } }, [state, isSubTreeEmpty, setIsSubTreeEmpty, children]) - // If a consumer sets state="done" without having a previous state (like `loading`), - // then it would announce on the first render. Using isPending is to only - // announce being "loaded" when the state has changed from `loading` --> `done`. + // Handle transition from loading to done state React.useEffect(() => { - if (isPending && state === 'done') { - const parentItem = document.getElementById(itemId) + if (previousState === 'loading' && state === 'done') { + const parentElement = document.getElementById(itemId) + if (!parentElement) return + + // Announce update to screen readers + const parentName = getAccessibleName(parentElement) - if (!parentItem) return + if (ref.current?.childElementCount) { + announceUpdate(`${parentName} content loaded`) + } else { + announceUpdate(`${parentName} is empty`) + } - const {current: node} = ref - const parentName = getAccessibleName(parentItem) + // Move focus to the first child if the loading indicator + // was focused when the async items finished loading + if (loadingFocused) { + const firstChild = getFirstChildElement(parentElement) - safeSetTimeout(() => { - if (node && node.childElementCount > 0) { - announceUpdate(`${parentName} content loaded`) + if (firstChild) { + firstChild.focus() } else { - announceUpdate(`${parentName} is empty`) + parentElement.focus() } - }) - setPending(false) + setLoadingFocused(false) + } } - }, [state, itemId, announceUpdate, safeSetTimeout, isPending]) + }, [loadingFocused, previousState, state, itemId, announceUpdate, ref]) - // Manage loading indicator state + // Track focus on the loading indicator React.useEffect(() => { - // If we're in the loading state, but not showing the loading indicator yet, - // show the loading indicator - if (state === 'loading' && !isLoadingItemVisible) { - setIsLoadingItemVisible(true) + function handleFocus() { + setLoadingFocused(true) } - // If we're not in the loading state, but we're still showing a loading indicator, - // hide the loading indicator and move focus if necessary - if (state !== 'loading' && isLoadingItemVisible) { - const isLoadingItemFocused = document.activeElement === loadingItemRef.current + function handleBlur(event: FocusEvent) { + // Skip blur events that are caused by the element being removed from the DOM. + // This can happen when the loading indicator is focused when async items are + // done loading and the loading indicator is removed from the DOM. + // If `loadingFocused` is `true` when `state` is `"done"` then the loading indicator + // was focused when the async items finished loading and we need to move focus to the + // first child. + if (!event.relatedTarget) return - setIsLoadingItemVisible(false) + setLoadingFocused(false) + } - if (isLoadingItemFocused) { - safeSetTimeout(() => { - const parentElement = document.getElementById(itemId) - if (!parentElement) return + const loadingElement = loadingItemRef.current + if (!loadingElement) return - const firstChild = getFirstChildElement(parentElement) + loadingElement.addEventListener('focus', handleFocus) + loadingElement.addEventListener('blur', handleBlur) - if (firstChild) { - firstChild.focus() - } else { - parentElement.focus() - } - }) - } + return () => { + loadingElement.removeEventListener('focus', handleFocus) + loadingElement.removeEventListener('blur', handleBlur) } - }, [state, safeSetTimeout, isLoadingItemVisible, itemId]) + }, [loadingItemRef, state]) if (!isExpanded) { return null @@ -607,13 +604,23 @@ const SubTree: React.FC = ({count, state, children}) => { // @ts-ignore Box doesn't have type support for `ref` used in combination with `as` ref={ref} > - {isLoadingItemVisible ? : children} + {state === 'loading' ? : children} ) } SubTree.displayName = 'TreeView.SubTree' +function usePreviousValue(value: T): T { + const ref = React.useRef(value) + + React.useEffect(() => { + ref.current = value + }, [value]) + + return ref.current +} + const shimmer = keyframes` from { mask-position: 200%; } to { mask-position: 0%; }