Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
91203b8
Add TreeView docs
colebemis Sep 2, 2022
d425776
Update TreeView props
colebemis Sep 2, 2022
6523985
Scaffold treeview markup
colebemis Sep 2, 2022
30bbae4
Add TreeView to drafts
colebemis Sep 2, 2022
15dc5b2
Add comment
colebemis Sep 2, 2022
46fd1d5
Track item levels
colebemis Sep 2, 2022
a980f76
Add TreeView stories
colebemis Sep 8, 2022
9961985
Update TreeView docs
colebemis Sep 8, 2022
945c58c
Update TreeView markup
colebemis Sep 8, 2022
5b03f71
Create curly-birds-argue.md
colebemis Sep 8, 2022
4072099
Merge branch 'main' into treeview-markup
colebemis Sep 8, 2022
109f05e
Fix examples
colebemis Sep 9, 2022
49303b0
Update src/TreeView/TreeView.tsx
colebemis Sep 9, 2022
cbebf14
Update docs/content/TreeView.mdx
colebemis Sep 9, 2022
b3a3096
Add some tests
colebemis Sep 9, 2022
2b666bd
Implement arrow key navigation
colebemis Sep 12, 2022
df5444b
Fix arrow left
colebemis Sep 12, 2022
ba2166a
Set up aria-activedescendant
colebemis Sep 13, 2022
3437e11
Display focus ring on active descendant
colebemis Sep 13, 2022
d6002e3
Use arrow key to change active descendant
colebemis Sep 14, 2022
769715b
Handle expand/collapse with arrow keys
colebemis Sep 14, 2022
c4bc882
Merge branch 'main' into treeview-arrow-keys
colebemis Sep 14, 2022
bbec34b
Fix lint error
colebemis Sep 14, 2022
e22679d
Add up and down arrow key test
colebemis Sep 14, 2022
dbece82
Prevent default when moving focus with arrow keys
colebemis Sep 14, 2022
2f58284
Open links in same tab by default
colebemis Sep 14, 2022
2ad9f73
Add test for right arrow key expand
colebemis Sep 15, 2022
0065cfa
Add defaultExpanded prop
colebemis Sep 15, 2022
d41f319
Test left arrow key collapse
colebemis Sep 15, 2022
515a9fc
Reorganize tests
colebemis Sep 15, 2022
f99388b
Create neat-squids-cheat.md
colebemis Sep 15, 2022
b165dcf
Add more tests
colebemis Sep 15, 2022
96ac6d8
Implement home and end key behavior
colebemis Sep 15, 2022
311c27f
Merge branch 'treeview-arrow-keys' of github.com:primer/react into tr…
colebemis Sep 15, 2022
1325ca4
Update active descendant on click
colebemis Sep 15, 2022
c0c1745
Remove focus state
colebemis Sep 15, 2022
8303416
Remove focus state from context
colebemis Sep 15, 2022
9106046
Test enter and space keys
colebemis Sep 15, 2022
847053d
Update focus style
colebemis Sep 15, 2022
e949bc6
Merge branch 'main' into treeview-arrow-keys
colebemis Sep 16, 2022
79d0ccc
Remove Space key behavior
colebemis Sep 16, 2022
e1dcac4
Merge branch 'treeview-arrow-keys' of github.com:primer/react into tr…
colebemis Sep 16, 2022
d1f2945
Wrap test cases with themeprovider
colebemis Sep 16, 2022
13473b2
Add test for link item
colebemis Sep 16, 2022
b9b4415
Update urls
colebemis Sep 19, 2022
9f9e93c
Add current prop
colebemis Sep 19, 2022
f10aabd
Fix current styles
colebemis Sep 19, 2022
a4e3e76
Expand current items be default
colebemis Sep 19, 2022
6456565
Initialize active descendant to current item
colebemis Sep 19, 2022
ca3a56d
Add tests
colebemis Sep 19, 2022
ab4a0ce
Update stories
colebemis Sep 19, 2022
96fd464
Update docs examples
colebemis Sep 19, 2022
b62f693
Create tidy-beans-punch.md
colebemis Sep 19, 2022
b3e6da4
Update src/TreeView/TreeView.tsx
colebemis Sep 19, 2022
cacc441
Merge branch 'main' into treeview-current
colebemis Sep 20, 2022
fb11143
Update src/TreeView/TreeView.stories.tsx
colebemis Sep 20, 2022
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/tidy-beans-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Add a `current` prop to `TreeView.Item` and `TreeView.LinkItem`
12 changes: 10 additions & 2 deletions docs/content/TreeView.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ description: A hierarchical list of items where nested items can be expanded and
<TreeView.Item>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#">Button.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#" current>
Button.tsx
</TreeView.LinkItem>
<TreeView.LinkItem href="#">Button.test.tsx</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
Expand All @@ -46,7 +48,7 @@ description: A hierarchical list of items where nested items can be expanded and
src
<TreeView.SubTree>
<TreeView.LinkItem href="#">Avatar.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#">
<TreeView.LinkItem href="#" current>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#">Button.tsx</TreeView.LinkItem>
Expand Down Expand Up @@ -80,6 +82,12 @@ description: A hierarchical list of items where nested items can be expanded and

<PropsTable>
<PropsTableRow name="children" type="React.ReactNode" required />
<PropsTable
name="current"
type="boolean"
defaultValue="false"
description="Whether the item is the current item. No more than one item should be current at once."
/>
<PropsTableRow name="defaultExpanded" type="boolean" description="Whether the item is expanded by default." />
<PropsTableRow
name="onSelect"
Expand Down
70 changes: 37 additions & 33 deletions src/TreeView/TreeView.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const FileTreeWithDirectoryLinks: Story = () => (
src
<TreeView.SubTree>
<TreeView.LinkItem href="#avatar-tsx">Avatar.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#button">
<TreeView.LinkItem href="#button" current>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#button-tsx">Button.tsx</TreeView.LinkItem>
Expand All @@ -45,37 +45,41 @@ export const FileTreeWithDirectoryLinks: Story = () => (
</Box>
)

export const FileTreeWithoutDirectoryLinks: Story = () => (
<Box p={3}>
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.Item>
src
<TreeView.SubTree>
<TreeView.LinkItem href="#avatar-tsx">Avatar.tsx</TreeView.LinkItem>
<TreeView.Item>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#button-tsx">Button.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#button-test-tsx">Button.test.tsx</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item
// eslint-disable-next-line no-console
onToggle={isExpanded => console.log(`${isExpanded ? 'Expanded' : 'Collapsed'} "public" folder `)}
>
public
<TreeView.SubTree>
<TreeView.LinkItem href="#index-html">index.html</TreeView.LinkItem>
<TreeView.LinkItem href="#favicon-ico">favicon.ico</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.LinkItem href="#package-json">package.json</TreeView.LinkItem>
</TreeView>
</nav>
</Box>
)
export const FileTreeWithoutDirectoryLinks: Story = () => {
return (
<Box p={3}>
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.Item>
src
<TreeView.SubTree>
<TreeView.LinkItem href="#avatar-tsx">Avatar.tsx</TreeView.LinkItem>
<TreeView.Item>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#button-tsx" current>
Button.tsx
</TreeView.LinkItem>
<TreeView.LinkItem href="#button-test-tsx">Button.test.tsx</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item
// eslint-disable-next-line no-console
onToggle={isExpanded => console.log(`${isExpanded ? 'Expanded' : 'Collapsed'} "public" folder `)}
>
public
<TreeView.SubTree>
<TreeView.LinkItem href="#index-html">index.html</TreeView.LinkItem>
<TreeView.LinkItem href="#favicon-ico">favicon.ico</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.LinkItem href="#package-json">package.json</TreeView.LinkItem>
</TreeView>
</nav>
</Box>
)
}

export default meta
121 changes: 120 additions & 1 deletion src/TreeView/TreeView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,22 @@ describe('Markup', () => {
expect(subtree).toBeNull()
})

it('initializes aria-activedescendant to the first item by default', () => {
it('initializes aria-activedescendant to the current item by default', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Oh this is cool!

const {queryByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item>Item 1</TreeView.Item>
<TreeView.Item current>Item 2</TreeView.Item>
<TreeView.Item>Item 3</TreeView.Item>
</TreeView>
)

const root = queryByRole('tree')
const currentItem = queryByRole('treeitem', {name: 'Item 2'})

expect(root).toHaveAttribute('aria-activedescendant', currentItem?.id)
})

it('initializes aria-activedescendant to the first item if there is no current item', () => {
const {queryByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item>Item 1</TreeView.Item>
Expand All @@ -73,6 +88,110 @@ describe('Markup', () => {

expect(root).toHaveAttribute('aria-activedescendant', firstItem?.id)
})

it('uses aria-current', () => {
const {getByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item>Item 1</TreeView.Item>
<TreeView.Item current>Item 2</TreeView.Item>
<TreeView.Item>Item 3</TreeView.Item>
</TreeView>
)

const currentItem = getByRole('treeitem', {name: 'Item 2'})

expect(currentItem).toHaveAttribute('aria-current', 'true')
})

it('expands the path to the current item by default', () => {
const {getByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item>
Item 1
<TreeView.SubTree>
<TreeView.Item>Item 1.1</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item>
Item 2
<TreeView.SubTree>
<TreeView.Item>Item 2.1</TreeView.Item>
<TreeView.Item current>
Item 2.2
<TreeView.SubTree>
<TreeView.Item>Item 2.2.1</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item>Item 3</TreeView.Item>
</TreeView>
)

const item1 = getByRole('treeitem', {name: 'Item 1'})
const item2 = getByRole('treeitem', {name: 'Item 2'})
const item22 = getByRole('treeitem', {name: 'Item 2.2'})
const item221 = getByRole('treeitem', {name: 'Item 2.2.1'})

// Item 1 should not be expanded because it is not the parent of the current item
expect(item1).toHaveAttribute('aria-expanded', 'false')

// Item 2 should be expanded because it is the parent of the current item
expect(item2).toHaveAttribute('aria-expanded', 'true')

// Item 2.2 should be expanded because it is the current item
expect(item22).toHaveAttribute('aria-expanded', 'true')

// Item 2.2 should have an aria-current value of true
expect(item22).toHaveAttribute('aria-current', 'true')

// Item 2.2.1 should be visible because it is a child of the current item
expect(item221).toBeVisible()
})

it('expands the path to the current item when the current item is changed', () => {
function TestTree() {
const [current, setCurrent] = React.useState('item1')
return (
<div>
<button onClick={() => setCurrent('item2')}>Jump to Item 2</button>
<TreeView aria-label="Test tree">
<TreeView.Item current={current === 'item1'}>Item 1</TreeView.Item>
<TreeView.Item current={current === 'item2'}>
Item 2
<TreeView.SubTree>
<TreeView.Item current={current === 'item2.1'}>Item 2.1</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item current={current === 'item3'}>Item 3</TreeView.Item>
</TreeView>
</div>
)
}

const {getByRole, getByText} = renderWithTheme(<TestTree />)

const item1 = getByRole('treeitem', {name: 'Item 1'})
const item2 = getByRole('treeitem', {name: 'Item 2'})

// Item 1 should have an aria-current value of true
expect(item1).toHaveAttribute('aria-current', 'true')

// Item 2 should not be expanded because it is not the current item or the parent of the current item
expect(item2).toHaveAttribute('aria-expanded', 'false')

// Click the button to change the current item to Item 2
fireEvent.click(getByText('Jump to Item 2'))

// Item 1 should not have an aria-current value
expect(item1).not.toHaveAttribute('aria-current')

// Item 2 should be expanded because it is the current item
expect(item2).toHaveAttribute('aria-expanded', 'true')

// Item 2.1 should be visible because it is a child of the current item
expect(getByRole('treeitem', {name: 'Item 2.1'})).toBeVisible()
})
})

describe('Keyboard interactions', () => {
Expand Down
55 changes: 47 additions & 8 deletions src/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,29 @@ export type TreeViewProps = {
const UlBox = styled.ul<SxProp>(sx)

const Root: React.FC<TreeViewProps> = ({'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, children}) => {
const rootRef = React.useRef<HTMLUListElement>(null)
const [activeDescendant, setActiveDescendant] = React.useState('')

React.useEffect(() => {
// Initialize the active descendant to the first item in the tree
if (!activeDescendant) {
const firstItem = document.querySelector('[role="treeitem"]')
if (firstItem) setActiveDescendant(firstItem.id)
if (rootRef.current && !activeDescendant) {
const currentItem = rootRef.current.querySelector('[role="treeitem"][aria-current="true"]')
const firstItem = rootRef.current.querySelector('[role="treeitem"]')

// If current item exists, use it as the initial value for active descendant
if (currentItem) {
setActiveDescendant(currentItem.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it weird that aria-activedescendant updates on the parent, but aria-current does not move unless you manage it with state?

It seemed strange to me at first, but then I thought about it and was ok with it.

(non-blocking)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think of aria-current more as a "default value" for aria-activedescandant rather than state that should be kept in sync

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I update the code comments to make that more clear 👍

}
// Otherwise, initialize the active descendant to the first item in the tree
else if (firstItem) {
setActiveDescendant(firstItem.id)
}
}
}, [activeDescendant])
}, [rootRef, activeDescendant])

return (
<RootContext.Provider value={{activeDescendant, setActiveDescendant}}>
<UlBox
ref={rootRef}
tabIndex={0}
role="tree"
aria-label={ariaLabel}
Expand Down Expand Up @@ -226,12 +236,19 @@ function getLastElement(element: HTMLElement): HTMLElement | undefined {

export type TreeViewItemProps = {
children: React.ReactNode
current?: boolean
defaultExpanded?: boolean
onSelect?: (event: React.MouseEvent<HTMLElement> | KeyboardEvent) => void
onToggle?: (isExpanded: boolean) => void
}

const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, onToggle, children}) => {
const Item: React.FC<TreeViewItemProps> = ({
current: isCurrent = false,
defaultExpanded = false,
onSelect,
onToggle,
children
}) => {
const {setActiveDescendant} = React.useContext(RootContext)
const itemId = useSSRSafeId()
const labelId = useSSRSafeId()
Expand All @@ -250,6 +267,13 @@ const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, o
[isExpanded, onToggle]
)

// Expand item if it is the current item or contains the current item
React.useLayoutEffect(() => {
if (isCurrent || itemRef.current?.querySelector('[aria-current=true]')) {
setIsExpanded(true)
}
}, [itemRef, isCurrent, subTree])

React.useEffect(() => {
const element = itemRef.current

Expand All @@ -271,11 +295,11 @@ const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, o
break

case 'ArrowRight':
if (!isExpanded) setIsExpanded(true)
if (!isExpanded) toggle()
break

case 'ArrowLeft':
if (isExpanded) setIsExpanded(false)
if (isExpanded) toggle()
break
}
}
Expand All @@ -293,6 +317,7 @@ const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, o
aria-labelledby={labelId}
aria-level={level}
aria-expanded={hasSubTree ? isExpanded : undefined}
aria-current={isCurrent ? 'true' : undefined}
>
<Box
onClick={event => {
Expand All @@ -304,6 +329,7 @@ const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, o
}
}}
sx={{
Copy link
Contributor

Choose a reason for hiding this comment

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

The value of sx could get reallly big as we start to get more into the styling. Maybe we could consider moving the style code into it's own file or at least outside of the component function's scope.

(Non-blocking for this PR)

position: 'relative',
display: 'grid',
gridTemplateColumns: `calc(${level - 1} * 8px) 16px 1fr`,
gridTemplateAreas: `"spacer toggle content"`,
Expand All @@ -319,6 +345,19 @@ const Item: React.FC<TreeViewItemProps> = ({defaultExpanded = false, onSelect, o
},
[`[role=tree][aria-activedescendant="${itemId}"]:focus-visible &`]: {
boxShadow: (theme: Theme) => `0 0 0 2px ${theme.colors.accent.emphasis}`
},
'[role=treeitem][aria-current=true] > &': {
bg: 'actionListItem.default.selectedBg',
Copy link
Member

Choose a reason for hiding this comment

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

This primitive looks tidied to the ActionList, I haven't seen many primitives like this so far in Primer. Is it temporary or any other reasoning behind using a component scoped primitive name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TreeView builds on ActionList styles so we're reusing some ActionList tokens/primitives here

'&::after': {
position: 'absolute',
top: 'calc(50% - 12px)',
left: -2,
width: '4px',
height: '24px',
content: '""',
bg: 'accent.fg',
borderRadius: 2
}
}
}}
>
Expand Down