Skip to content

Commit

Permalink
TreeView: Improve performance when rendering lots of items (#2460)
Browse files Browse the repository at this point in the history 
* Avoid unecessary style recalculation

* Add stress test story

* Only render subtree if it's expanded

* Don't expand current item by default

* Update controlled story

* Create popular-taxis-yawn.md
  • Loading branch information
@colebemis
colebemis committed Oct 20, 2022
1 parent aca96c0 commit 1f25c90
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 258 deletions.
5 changes: 5 additions & 0 deletions .changeset/popular-taxis-yawn.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

TreeView: Improve performance when rendering lots of items
139 changes: 46 additions & 93 deletions src/TreeView/TreeView.stories.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {DiffAddedIcon, DiffModifiedIcon, DiffRemovedIcon, DiffRenamedIcon, FileIcon} from '@primer/octicons-react'
import {Meta, Story} from '@storybook/react'
import React from 'react'
import {ActionList} from '../ActionList'
import {ActionMenu} from '../ActionMenu'
import Box from '../Box'
import {Button} from '../Button'
import StyledOcticon from '../StyledOcticon'
Expand All @@ -20,7 +18,7 @@ export const FileTreeWithDirectoryLinks: Story = () => (
<Box sx={{p: 3, maxWidth: 400}}>
<nav aria-label="Files">
<TreeView aria-label="Files">
<TreeView.LinkItem href="#src">
<TreeView.LinkItem href="#src" defaultExpanded>
<TreeView.LeadingVisual>
<TreeView.DirectoryIcon />
</TreeView.LeadingVisual>
Expand Down Expand Up @@ -241,107 +239,35 @@ const CurrentPathContext = React.createContext<{
setCurrentPath: () => {}
})

export const Controlled: Story = () => {
const [currentPath, setCurrentPath] = React.useState<string[]>(['src', 'Avatar.tsx'])
const [tree, setTree] = React.useState<TreeItem[]>([
{
data: {
name: 'src',
expanded: false
},
children: [
{
data: {
name: 'Avatar.tsx',
expanded: false
},
children: []
},
{
data: {
name: 'Button',
expanded: false
},
children: [
{
data: {
name: 'Button.tsx',
expanded: false
},
children: []
},
{
data: {
name: 'Button.test.tsx',
expanded: false
},
children: []
}
]
}
]
},
{
data: {
name: 'public',
expanded: false
},
children: [
{
data: {
name: 'index.html',
expanded: false
},
children: []
},
{
data: {
name: 'favicon.ico',
expanded: false
},
children: []
}
]
const TREE: TreeItem[] = Array.from({length: 5}).map((_, i) => ({
data: {
name: `Item ${i}`,
expanded: false
},
children: Array.from({length: 5}).map((_, j) => ({
data: {
name: `Item ${i}.${j}`,
expanded: false
},
{
children: Array.from({length: 5}).map((_, k) => ({
data: {
name: 'package.json',
name: `Item ${i}.${j}.${k}`,
expanded: false
},
children: []
}
])
}))
}))
}))

export const Controlled: Story = () => {
const [currentPath, setCurrentPath] = React.useState<string[]>(['src', 'Avatar.tsx'])
const [tree, setTree] = React.useState<TreeItem[]>(TREE)

return (
<Box sx={{p: 3, display: 'grid', gap: 3}}>
<Box sx={{display: 'flex', gap: 2}}>
<Button onClick={() => setTree(collapseAll)}>Collapse all</Button>
<Button onClick={() => setTree(expandAll)}>Expand all</Button>
<ActionMenu>
<ActionMenu.Button>Jump to</ActionMenu.Button>

<ActionMenu.Overlay>
<ActionList>
<ActionList.Item onSelect={() => setCurrentPath(['src'])}>src</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['src', 'Avatar.tsx'])}>src/Avatar.tsx</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['src', 'Button'])}>src/Button</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['src', 'Button', 'Button.tsx'])}>
src/Button/Button.tsx
</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['src', 'Button', 'Button.test.tsx'])}>
src/Button/Button.test.tsx
</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['public'])}>public</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['public', 'index.html'])}>
public/index.html
</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['public', 'favicon.ico'])}>
public/favicon.ico
</ActionList.Item>
<ActionList.Item onSelect={() => setCurrentPath(['package.json'])}>package.json</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Box>
<nav aria-label="Files">
<CurrentPathContext.Provider value={{currentPath, setCurrentPath}}>
Expand Down Expand Up @@ -669,4 +595,31 @@ AsyncError.args = {
responseTime: 2000
}

export const StressTest: Story = () => {
return (
<Box sx={{p: 3, maxWidth: 400}}>
<TreeView aria-label="Files">
{Array.from({length: 1000}).map((_, index) => (
<TreeView.Item key={index}>
<TreeView.LeadingVisual>
<TreeView.DirectoryIcon />
</TreeView.LeadingVisual>
Directory {index}
<TreeView.SubTree>
{Array.from({length: 100}).map((_, index) => (
<TreeView.Item key={index}>
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
File {index}
</TreeView.Item>
))}
</TreeView.SubTree>
</TreeView.Item>
))}
</TreeView>
</Box>
)
}

export default meta
133 changes: 0 additions & 133 deletions src/TreeView/TreeView.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,139 +73,6 @@ describe('Markup', () => {
expect(currentItem).toHaveAttribute('aria-current', 'true')
})

it('expands the path to the current item (level 2) 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 (level 3) 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>
Item 2.2
<TreeView.SubTree>
<TreeView.Item current>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.1 should be the current item
expect(item221).toHaveAttribute('aria-current', 'true')
})

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()
})

it('should be described by leading visuals', () => {
const {getByLabelText} = renderWithTheme(
<TreeView aria-label="Test tree">
Expand Down
Loading

0 comments on commit 1f25c90

Please sign in to comment.