From c33915293d80fd8c6116601f83b99b477c3dd7d3 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Tue, 22 Oct 2024 00:14:20 -0400 Subject: [PATCH 1/8] Add indication of empty directory --- packages/react/src/TreeView/TreeView.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/react/src/TreeView/TreeView.tsx b/packages/react/src/TreeView/TreeView.tsx index b891aa97a74..e8b6f891d22 100644 --- a/packages/react/src/TreeView/TreeView.tsx +++ b/packages/react/src/TreeView/TreeView.tsx @@ -482,7 +482,7 @@ const Item = React.forwardRef( aria-labelledby={ariaLabel ? undefined : ariaLabelledby || labelId} aria-describedby={`${leadingVisualId} ${trailingVisualId}`} aria-level={level} - aria-expanded={isSubTreeEmpty ? undefined : isExpanded} + aria-expanded={isExpanded} aria-current={isCurrentItem ? 'true' : undefined} aria-selected={isFocused ? 'true' : 'false'} data-has-leading-action={slots.leadingAction ? true : undefined} @@ -625,6 +625,7 @@ const SubTree: React.FC = ({count, state, children}) => { if (ref.current?.childElementCount) { announceUpdate(`${parentName} content loaded`) } else { + console.log(`${parentName} is empty`) announceUpdate(`${parentName} is empty`) } @@ -697,6 +698,7 @@ const SubTree: React.FC = ({count, state, children}) => { ref={ref} > {state === 'loading' ? : children} + {isSubTreeEmpty && state !== 'loading' ? : null} ) } @@ -785,6 +787,14 @@ const LoadingItem = React.forwardRef(({count}, re ) }) +const EmptyItem = React.forwardRef((props, ref) => { + return ( + + No items found + + ) +}) + function useSubTree(children: React.ReactNode) { return React.useMemo(() => { const subTree = React.Children.toArray(children).find( From 91666f2832eba52216c63c35614c8aac6e502bee Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Tue, 22 Oct 2024 00:15:14 -0400 Subject: [PATCH 2/8] clean up --- packages/react/src/TreeView/TreeView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react/src/TreeView/TreeView.tsx b/packages/react/src/TreeView/TreeView.tsx index e8b6f891d22..67eaf16a0cc 100644 --- a/packages/react/src/TreeView/TreeView.tsx +++ b/packages/react/src/TreeView/TreeView.tsx @@ -625,7 +625,6 @@ const SubTree: React.FC = ({count, state, children}) => { if (ref.current?.childElementCount) { announceUpdate(`${parentName} content loaded`) } else { - console.log(`${parentName} is empty`) announceUpdate(`${parentName} is empty`) } From 3312e4180e7d57b2cc2109eeb5b91baa9ec652d6 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 23 Oct 2024 09:56:32 -0400 Subject: [PATCH 3/8] Add changeset --- .changeset/lovely-shirts-hope.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lovely-shirts-hope.md diff --git a/.changeset/lovely-shirts-hope.md b/.changeset/lovely-shirts-hope.md new file mode 100644 index 00000000000..d628f02357e --- /dev/null +++ b/.changeset/lovely-shirts-hope.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +TreeView: Adds indication of no nodes in a tree item, allows for `aria-expanded even if the item is empty. From 47bccf22b40833e96640041eaff3fb9ef8448eb4 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 23 Oct 2024 10:03:29 -0400 Subject: [PATCH 4/8] Update `TreeView` test --- packages/react/src/TreeView/TreeView.test.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/react/src/TreeView/TreeView.test.tsx b/packages/react/src/TreeView/TreeView.test.tsx index 056b6714acb..baf21f769ec 100644 --- a/packages/react/src/TreeView/TreeView.test.tsx +++ b/packages/react/src/TreeView/TreeView.test.tsx @@ -217,10 +217,10 @@ describe('Markup', () => { expect(treeitem).toHaveAttribute('aria-expanded', 'true') treeitem = getByLabelText(/Item 2/) - expect(treeitem).not.toHaveAttribute('aria-expanded') + expect(treeitem).toHaveAttribute('aria-expanded', 'false') await user.click(getByText(/Item 2/)) - expect(treeitem).not.toHaveAttribute('aria-expanded') + expect(treeitem).toHaveAttribute('aria-expanded', 'true') }) it('should render with containIntrinsicSize', () => { @@ -1537,7 +1537,7 @@ describe('Asyncronous loading', () => { expect(parentItem).toHaveAttribute('aria-expanded', 'true') }) - it('should remove `aria-expanded` if no content is loaded in', async () => { + it('should update `aria-expanded` if no content is loaded in', async () => { function Example() { const [state, setState] = React.useState('loading') const timeoutId = React.useRef | null>(null) @@ -1584,6 +1584,7 @@ describe('Asyncronous loading', () => { jest.runAllTimers() }) - expect(treeitem).not.toHaveAttribute('aria-expanded') + expect(treeitem).toHaveAttribute('aria-expanded', 'true') + expect(getByLabelText('No items found')).toBeInTheDocument() }) }) From be5748e5ffd1443cc7ea0657dfcd058680561070 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 4 Nov 2024 09:05:38 -0500 Subject: [PATCH 5/8] Fix story --- packages/react/src/TreeView/TreeView.features.stories.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/TreeView/TreeView.features.stories.tsx b/packages/react/src/TreeView/TreeView.features.stories.tsx index b4d9a8181b4..b608bc51fb2 100644 --- a/packages/react/src/TreeView/TreeView.features.stories.tsx +++ b/packages/react/src/TreeView/TreeView.features.stories.tsx @@ -580,7 +580,7 @@ AsyncError.args = { } export const EmptyDirectories: StoryFn = () => { - const [state, setState] = React.useState('loading') + const [state, setState] = React.useState('initial') const timeoutId = React.useRef | null>(null) React.useEffect(() => { @@ -597,6 +597,7 @@ export const EmptyDirectories: StoryFn = () => { { + setState('loading') if (expanded) { timeoutId.current = setTimeout(() => { setState('done') From 8b9e0f53a4dfc38fa00cbccc6089524e2b84de9b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 4 Nov 2024 09:56:16 -0500 Subject: [PATCH 6/8] Redo conditional --- packages/react/src/TreeView/TreeView.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react/src/TreeView/TreeView.tsx b/packages/react/src/TreeView/TreeView.tsx index 67eaf16a0cc..3db8ce587a9 100644 --- a/packages/react/src/TreeView/TreeView.tsx +++ b/packages/react/src/TreeView/TreeView.tsx @@ -361,7 +361,7 @@ export type TreeViewItemProps = { containIntrinsicSize?: string current?: boolean defaultExpanded?: boolean - expanded?: boolean + expanded?: boolean | null onExpandedChange?: (expanded: boolean) => void onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void className?: string @@ -401,7 +401,7 @@ const Item = React.forwardRef( // If defaultExpanded is not provided, we default to false unless the item // is the current item, in which case we default to true. defaultValue: () => expandedStateCache.current?.get(itemId) ?? defaultExpanded ?? isCurrentItem, - value: expanded, + value: expanded === null ? false : expanded, onChange: onExpandedChange, }) const {level} = React.useContext(ItemContext) @@ -482,7 +482,7 @@ const Item = React.forwardRef( aria-labelledby={ariaLabel ? undefined : ariaLabelledby || labelId} aria-describedby={`${leadingVisualId} ${trailingVisualId}`} aria-level={level} - aria-expanded={isExpanded} + aria-expanded={(isSubTreeEmpty && !isExpanded) || expanded === null ? undefined : isExpanded} aria-current={isCurrentItem ? 'true' : undefined} aria-selected={isFocused ? 'true' : 'false'} data-has-leading-action={slots.leadingAction ? true : undefined} @@ -788,7 +788,7 @@ const LoadingItem = React.forwardRef(({count}, re const EmptyItem = React.forwardRef((props, ref) => { return ( - + No items found ) From 1f5f0e6ba2d49422c71d4d151d7f2920dc2d394c Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 4 Nov 2024 10:05:27 -0500 Subject: [PATCH 7/8] Fix test --- packages/react/src/TreeView/TreeView.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/TreeView/TreeView.test.tsx b/packages/react/src/TreeView/TreeView.test.tsx index baf21f769ec..d42fa6247d9 100644 --- a/packages/react/src/TreeView/TreeView.test.tsx +++ b/packages/react/src/TreeView/TreeView.test.tsx @@ -217,7 +217,7 @@ describe('Markup', () => { expect(treeitem).toHaveAttribute('aria-expanded', 'true') treeitem = getByLabelText(/Item 2/) - expect(treeitem).toHaveAttribute('aria-expanded', 'false') + expect(treeitem).not.toHaveAttribute('aria-expanded') await user.click(getByText(/Item 2/)) expect(treeitem).toHaveAttribute('aria-expanded', 'true') From 5ace102acc9c79e2ac7c01db8323077325eaa7d6 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 4 Nov 2024 10:39:53 -0500 Subject: [PATCH 8/8] Add test, fix condition --- packages/react/src/TreeView/TreeView.test.tsx | 39 +++++++++++++++++++ packages/react/src/TreeView/TreeView.tsx | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/react/src/TreeView/TreeView.test.tsx b/packages/react/src/TreeView/TreeView.test.tsx index d42fa6247d9..78bfceae3ba 100644 --- a/packages/react/src/TreeView/TreeView.test.tsx +++ b/packages/react/src/TreeView/TreeView.test.tsx @@ -1587,4 +1587,43 @@ describe('Asyncronous loading', () => { expect(treeitem).toHaveAttribute('aria-expanded', 'true') expect(getByLabelText('No items found')).toBeInTheDocument() }) + + it('should have `aria-expanded` when directory is empty', async () => { + const {getByRole} = renderWithTheme( + + + + + + Parent + + child + + child current + + + empty child + + + + + , + ) + + const parentItem = getByRole('treeitem', {name: 'Parent'}) + + // Parent item should be expanded + expect(parentItem).toHaveAttribute('aria-expanded', 'true') + + // Current child should not have `aria-expanded` + expect(getByRole('treeitem', {name: 'child current'})).not.toHaveAttribute('aria-expanded') + + // Empty child should not have `aria-expanded` when closed + expect(getByRole('treeitem', {name: 'empty child'})).not.toHaveAttribute('aria-expanded') + + fireEvent.click(getByRole('treeitem', {name: 'empty child'})) + + // Empty child should have `aria-expanded` when opened + expect(getByRole('treeitem', {name: 'empty child'})).toHaveAttribute('aria-expanded') + }) }) diff --git a/packages/react/src/TreeView/TreeView.tsx b/packages/react/src/TreeView/TreeView.tsx index 3db8ce587a9..71f41042722 100644 --- a/packages/react/src/TreeView/TreeView.tsx +++ b/packages/react/src/TreeView/TreeView.tsx @@ -482,7 +482,7 @@ const Item = React.forwardRef( aria-labelledby={ariaLabel ? undefined : ariaLabelledby || labelId} aria-describedby={`${leadingVisualId} ${trailingVisualId}`} aria-level={level} - aria-expanded={(isSubTreeEmpty && !isExpanded) || expanded === null ? undefined : isExpanded} + aria-expanded={(isSubTreeEmpty && (!isExpanded || !hasSubTree)) || expanded === null ? undefined : isExpanded} aria-current={isCurrentItem ? 'true' : undefined} aria-selected={isFocused ? 'true' : 'false'} data-has-leading-action={slots.leadingAction ? true : undefined}