Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TreeView: Add support for leading and trailing visuals #2383

Merged
merged 15 commits into from
Sep 30, 2022
Merged

TreeView: Add support for leading and trailing visuals #2383

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
03c21cc
Create LeadingVisual and TrailingVisual components
colebemis Sep 28, 2022
ef86924
Create TreeView.DirectoryIcon
colebemis Sep 28, 2022
48fb1af
Update stories
colebemis Sep 28, 2022
0f30a19
Truncate text labels
colebemis Sep 28, 2022
9b8c2f2
Add comments
colebemis Sep 28, 2022
76e180e
Create twelve-beers-raise.md
colebemis Sep 28, 2022
39201ca
Merge remote-tracking branch 'origin/main' into treeview-visuals
colebemis Sep 28, 2022
8c300c3
Update docs
colebemis Sep 28, 2022
039c29a
Merge branch 'main' into treeview-visuals
colebemis Sep 29, 2022
f48c098
Update docs and stories
colebemis Sep 30, 2022
9057ae4
Merge branch 'treeview-visuals' of github.com:primer/react into treev…
colebemis Sep 30, 2022
1447bf5
Update docs/content/TreeView.mdx
colebemis Sep 30, 2022
3cfb814
Update docs/content/TreeView.mdx
colebemis Sep 30, 2022
6f4e25b
Merge branch 'main' into treeview-visuals
colebemis Sep 30, 2022
4787ccb
Update docs/content/TreeView.mdx
colebemis Sep 30, 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/twelve-beers-raise.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

TreeView: Add support for leading and trailing visuals
155 changes: 93 additions & 62 deletions docs/content/TreeView.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,63 +10,98 @@ description: A hierarchical list of items where nested items can be expanded and
### File tree navigation without directory links
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get an example (and maybe a story?) showing how to use the isExpanded render prop to set different icons for expanded and collapsed states.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea 👍 Let me know if the docs I added make sense


```jsx live drafts
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.Item>
src
<TreeView.SubTree>
<TreeView.LinkItem href="#">Avatar.tsx</TreeView.LinkItem>
<TreeView.Item>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#" current>
Button.tsx
</TreeView.LinkItem>
<TreeView.LinkItem href="#">Button.test.tsx</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item>
public
<TreeView.SubTree>
<TreeView.LinkItem href="#">index.html</TreeView.LinkItem>
<TreeView.LinkItem href="#">favicon.ico</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.LinkItem href="#">package.json</TreeView.LinkItem>
</TreeView>
</nav>
<Box sx={{maxWidth: 320}}>
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.Item>
<TreeView.LeadingVisual>
<TreeView.DirectoryIcon />
</TreeView.LeadingVisual>
src
<TreeView.SubTree>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
Avatar.tsx
<TreeView.TrailingVisual>
<StyledOcticon icon={DiffAddedIcon} color="success.fg" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Since these trailing visuals are used to visually communicate diff status, I'm pretty sure they should have an aria-label.

cc @ericwbailey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. How should the aria-label for the trailing visual impact the aria-label of the treeitem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the aria-label would be Avatar.tsx added. That seems correct but I'd love confirmation from @ericwbailey.

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 updated the docs and stories to include aria-label on these trailing visuals 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to get to this after merging. I try to avoid using aria-label on non-interactive elements, given their mixed support. In the API guidance I am recommending content that uses both aria-hidden="true" and a sr-only class applied to it.

In addition to the mixed support concern, we're also going to be using the text string to construct aria-labeledby announcements for the relevant TreeView node.

</TreeView.TrailingVisual>
</TreeView.LinkItem>
<TreeView.LinkItem href="#" current>
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
Button.tsx
<TreeView.TrailingVisual>
<StyledOcticon icon={DiffModifiedIcon} color="attention.fg" />
</TreeView.TrailingVisual>
</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
package.json
<TreeView.TrailingVisual>
<StyledOcticon icon={DiffModifiedIcon} color="attention.fg" />
</TreeView.TrailingVisual>
</TreeView.LinkItem>
</TreeView>
</nav>
</Box>
```

### File tree navigation with directory links

```jsx live drafts
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.LinkItem href="#">
src
<TreeView.SubTree>
<TreeView.LinkItem href="#">Avatar.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#" current>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#">Button.tsx</TreeView.LinkItem>
<TreeView.LinkItem href="#">Button.test.tsx</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.LinkItem>
<TreeView.LinkItem href="#">
public
<TreeView.SubTree>
<TreeView.LinkItem href="#">index.html</TreeView.LinkItem>
<TreeView.LinkItem href="#">favicon.ico</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.LinkItem>
<TreeView.LinkItem href="#">package.json</TreeView.LinkItem>
</TreeView>
</nav>
<Box sx={{maxWidth: 320}}>
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<TreeView.DirectoryIcon />
</TreeView.LeadingVisual>
src
<TreeView.SubTree>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
Avatar.tsx
</TreeView.LinkItem>
<TreeView.LinkItem href="#" current>
<TreeView.LeadingVisual>
<TreeView.DirectoryIcon />
</TreeView.LeadingVisual>
Button
<TreeView.SubTree>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
Button.tsx
</TreeView.LinkItem>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
Button.test.tsx
</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.LinkItem>
</TreeView.SubTree>
</TreeView.LinkItem>
<TreeView.LinkItem href="#">
<TreeView.LeadingVisual>
<FileIcon />
</TreeView.LeadingVisual>
package.json
</TreeView.LinkItem>
</TreeView>
</nav>
</Box>
```

### With controlled expanded state
Expand All @@ -76,7 +111,7 @@ function ControlledTreeView() {
const [expanded, setExpanded] = React.useState(false)

return (
<Box sx={{display: 'grid', gap: 2}}>
<Box sx={{display: 'grid', gap: 2, maxWidth: 320}}>
<Button onClick={() => setExpanded(!expanded)}>{expanded ? 'Collapse' : 'Expand'}</Button>
<nav aria-label="File navigation">
<TreeView aria-label="File navigation">
Expand Down Expand Up @@ -190,17 +225,15 @@ render(<ControlledTreeView />)
{/* <PropsTableSxRow /> */}
</PropsTable>

<!-- TODO: Add leading and trailing visuals -->

<!-- ### TreeView.LeadingVisual
### TreeView.LeadingVisual

<PropsTable>
<PropsTableRow
name="children"
type={`| React.ReactNode
| (props: {isExpanded: boolean}) => React.ReactNode`}
/>
<PropsTableSxRow />
{/* <PropsTableSxRow /> */}
</PropsTable>

### TreeView.TrailingVisual
Expand All @@ -211,14 +244,12 @@ render(<ControlledTreeView />)
type={`| React.ReactNode
| (props: {isExpanded: boolean}) => React.ReactNode`}
/>
<PropsTableSxRow />
{/* <PropsTableSxRow /> */}
</PropsTable>

### TreeView.FolderIcon
### TreeView.DirectoryIcon

<PropsTable>
<PropsTableSxRow />
</PropsTable> -->
<PropsTable>{/* <PropsTableSxRow /> */}</PropsTable>

<!-- TODO: Add components to support async behavior (e.g. LoadingItem) -->

Expand Down
Loading