diff --git a/.changeset/eleven-countries-hang.md b/.changeset/eleven-countries-hang.md new file mode 100644 index 00000000000..fc25ba2c85a --- /dev/null +++ b/.changeset/eleven-countries-hang.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Add support for dynamic aria-expanded support on TreeView.Item diff --git a/src/TreeView/TreeView.stories.tsx b/src/TreeView/TreeView.stories.tsx index e186675e1b0..ff4092d5af3 100644 --- a/src/TreeView/TreeView.stories.tsx +++ b/src/TreeView/TreeView.stories.tsx @@ -442,6 +442,9 @@ export const AsyncWithCount: Story = args => { + + + src @@ -622,4 +625,48 @@ export const StressTest: Story = () => { ) } +export const EmptyDirectory: Story = () => { + const [state, setState] = React.useState('loading') + const timeoutId = React.useRef | null>(null) + + React.useEffect(() => { + return () => { + if (timeoutId.current) { + clearTimeout(timeoutId.current) + timeoutId.current = null + } + } + }, []) + + return ( + + + { + if (expanded) { + timeoutId.current = setTimeout(() => { + setState('done') + timeoutId.current = null + }, 2000) + } + }} + > + + + + src + + + + + + + .github + + + + + ) +} + export default meta diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index c8b4c2d143b..4072d8955a2 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -1,8 +1,12 @@ import {fireEvent, render, waitFor} from '@testing-library/react' +import userEvent from '@testing-library/user-event' import React from 'react' +import {act} from 'react-dom/test-utils' import {ThemeProvider} from '../ThemeProvider' import {SubTreeState, TreeView} from './TreeView' +jest.useFakeTimers() + // TODO: Move this function into a shared location function renderWithTheme( ui: Parameters[0], @@ -155,6 +159,40 @@ describe('Markup', () => { // space expect(noDescription).toHaveAccessibleDescription(' ') }) + + it('should include `aria-expanded` when a SubTree contains content', async () => { + const user = userEvent.setup({ + advanceTimers: jest.advanceTimersByTime + }) + const {getByLabelText, getByText} = renderWithTheme( + + + Item 1 + + Item 1.a + Item 1.b + Item 1.c + + + + Item 2 + + + + ) + + let treeitem = getByLabelText(/Item 1/) + expect(treeitem).toHaveAttribute('aria-expanded', 'false') + + await user.click(getByText(/Item 1/)) + expect(treeitem).toHaveAttribute('aria-expanded', 'true') + + treeitem = getByLabelText(/Item 2/) + expect(treeitem).not.toHaveAttribute('aria-expanded') + + await user.click(getByText(/Item 2/)) + expect(treeitem).not.toHaveAttribute('aria-expanded') + }) }) describe('Keyboard interactions', () => { @@ -940,6 +978,10 @@ describe('Asyncronous loading', () => { // Click done button to mimic the completion of async loading fireEvent.click(doneButton) + act(() => { + jest.runAllTimers() + }) + // Live region should be updated expect(liveRegion).toHaveTextContent('Parent content loaded') }) @@ -983,13 +1025,15 @@ describe('Asyncronous loading', () => { // Wait for async loading to complete const firstChild = await findByRole('treeitem', {name: 'Child 1'}) - setTimeout(() => { - // First child should be focused - expect(firstChild).toHaveFocus() + act(() => { + jest.runAllTimers() }) + + // First child should be focused + expect(firstChild).toHaveFocus() }) - it.only('moves focus to parent item after closing error dialog', async () => { + it('moves focus to parent item after closing error dialog', async () => { function TestTree() { const [error, setError] = React.useState('Test error') @@ -1064,4 +1108,53 @@ describe('Asyncronous loading', () => { // Parent item should still be expanded expect(parentItem).toHaveAttribute('aria-expanded', 'true') }) + + it('should remove `aria-expanded` if no content is loaded in', async () => { + function Example() { + const [state, setState] = React.useState('loading') + const timeoutId = React.useRef | null>(null) + + React.useEffect(() => { + return () => { + if (timeoutId.current) { + clearTimeout(timeoutId.current) + timeoutId.current = null + } + } + }, []) + + return ( + + { + if (expanded) { + timeoutId.current = setTimeout(() => { + setState('done') + }, 1000) + } + }} + > + Item 1 + + + + ) + } + const {getByLabelText, getByText} = renderWithTheme() + const user = userEvent.setup({ + advanceTimers: jest.advanceTimersByTime + }) + + const treeitem = getByLabelText(/Item 1/) + expect(treeitem).toHaveAttribute('aria-expanded', 'false') + + await user.click(getByText(/Item 1/)) + expect(treeitem).toHaveAttribute('aria-expanded', 'true') + + act(() => { + jest.runAllTimers() + }) + + expect(treeitem).not.toHaveAttribute('aria-expanded') + }) }) diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index 3ea37f1bb4a..25c4ff34aa1 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -35,6 +35,8 @@ const RootContext = React.createContext<{ const ItemContext = React.createContext<{ itemId: string level: number + isSubTreeEmpty: boolean + setIsSubTreeEmpty: React.Dispatch> isExpanded: boolean setIsExpanded: React.Dispatch> leadingVisualId: string @@ -42,6 +44,8 @@ const ItemContext = React.createContext<{ }>({ itemId: '', level: 1, + isSubTreeEmpty: false, + setIsSubTreeEmpty: () => {}, isExpanded: false, setIsExpanded: () => {}, leadingVisualId: '', @@ -135,6 +139,7 @@ const Item = React.forwardRef( }) const {level} = React.useContext(ItemContext) const {hasSubTree, subTree, childrenWithoutSubTree} = useSubTree(children) + const [isSubTreeEmpty, setIsSubTreeEmpty] = React.useState(!hasSubTree) // Expand or collapse the subtree const toggle = React.useCallback( @@ -189,6 +194,8 @@ const Item = React.forwardRef( value={{ itemId, level: level + 1, + isSubTreeEmpty, + setIsSubTreeEmpty, isExpanded, setIsExpanded, leadingVisualId, @@ -205,7 +212,7 @@ const Item = React.forwardRef( aria-labelledby={labelId} aria-describedby={`${leadingVisualId} ${trailingVisualId}`} aria-level={level} - aria-expanded={hasSubTree ? isExpanded : undefined} + aria-expanded={isSubTreeEmpty ? undefined : isExpanded} aria-current={isCurrentItem ? 'true' : undefined} onKeyDown={handleKeyDown} sx={{ @@ -328,7 +335,7 @@ const Item = React.forwardRef( - {isExpanded ? subTree : null} + {subTree} ) @@ -409,11 +416,26 @@ export type TreeViewSubTreeProps = { const SubTree: React.FC = ({count, state, children}) => { const {announceUpdate} = React.useContext(RootContext) - const {itemId, isExpanded} = React.useContext(ItemContext) + const {itemId, isExpanded, isSubTreeEmpty, setIsSubTreeEmpty} = React.useContext(ItemContext) const [isLoadingItemVisible, setIsLoadingItemVisible] = React.useState(false) const {safeSetTimeout, safeClearTimeout} = useSafeTimeout() const timeoutId = React.useRef(0) const loadingItemRef = React.useRef(null) + const ref = React.useRef(null) + + React.useEffect(() => { + // If `state` is undefined, we're working in a synchronous context and need + // to detect if the sub-tree has content. If `state === 'done` then we're + // working in an asynchronous context and need to see if there is content + // that has been loaded in. + if (state === undefined || state === 'done') { + if (!isSubTreeEmpty && !children) { + setIsSubTreeEmpty(true) + } else if (isSubTreeEmpty && children) { + setIsSubTreeEmpty(false) + } + } + }, [state, isSubTreeEmpty, setIsSubTreeEmpty, children]) // Announce when content has loaded React.useEffect(() => { @@ -422,10 +444,18 @@ const SubTree: React.FC = ({count, state, children}) => { if (!parentItem) return + const {current: node} = ref const parentName = getAccessibleName(parentItem) - announceUpdate(`${parentName} content loaded`) + + safeSetTimeout(() => { + if (node && node.childElementCount > 0) { + announceUpdate(`${parentName} content loaded`) + } else { + announceUpdate(`${parentName} is empty`) + } + }) } - }, [state, itemId, announceUpdate]) + }, [state, itemId, announceUpdate, safeSetTimeout]) // Show loading indicator after a short delay React.useEffect(() => { @@ -462,6 +492,10 @@ const SubTree: React.FC = ({count, state, children}) => { } }, [state, safeSetTimeout, safeClearTimeout, isLoadingItemVisible, itemId]) + if (!isExpanded) { + return null + } + return ( = ({count, state, children}) => { padding: 0, margin: 0 }} + // @ts-ignore Box doesn't have type support for `ref` used in combination with `as` + ref={ref} > {isLoadingItemVisible ? : children}