feat(TreeView): add label prop for Leading and Trailing Visual (#2434)

* feat(TreeView): add label prop for Leading and Trailing Visual * chore: changeset * chore: update tests * Update .changeset/pretty-bats-cough.md Co-authored-by: Cole Bemis <colebemis@github.com> * docs: update prop table * docs(TreeView): add note for decoarative visuals * docs(TreeView): add note for decoarative visuals * chore: remove duplicate import Co-authored-by: Cole Bemis <colebemis@github.com>
primer · Oct 17, 2022 · 57c3b4d · 57c3b4d
1 parent 8f92b0f
commit 57c3b4d
Show file tree

Hide file tree

Showing 5 changed files with 152 additions and 19 deletions.
diff --git a/.changeset/pretty-bats-cough.md b/.changeset/pretty-bats-cough.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Add support for labels in TreeView.LeadingVisual and TreeView.TrailingVisual
diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx
@@ -320,6 +320,11 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree
     type={`| React.ReactNode
 | (props: {isExpanded: boolean}) => React.ReactNode`}
   />
+  <PropsTableRow
+    name="label"
+    type="string"
+    description="Provide an accessible label for the leading visual. This is not necessary for decorative visuals"
+  />
   {/* <PropsTableSxRow /> */}
 </PropsTable>
 
@@ -331,6 +336,11 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree
     type={`| React.ReactNode
 | (props: {isExpanded: boolean}) => React.ReactNode`}
   />
+  <PropsTableRow
+    name="label"
+    type="string"
+    description="Provide an accessible label for the trailing visual. This is not necessary for decorative visuals"
+  />
   {/* <PropsTableSxRow /> */}
 </PropsTable>
 

diff --git a/src/TreeView/TreeView.stories.tsx b/src/TreeView/TreeView.stories.tsx
@@ -112,8 +112,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => {
                   <FileIcon />
                 </TreeView.LeadingVisual>
                 Avatar.tsx
-                <TreeView.TrailingVisual>
-                  <StyledOcticon icon={DiffAddedIcon} color="success.fg" aria-label="added" />
+                <TreeView.TrailingVisual label="added">
+                  <StyledOcticon icon={DiffAddedIcon} color="success.fg" />
                 </TreeView.TrailingVisual>
               </TreeView.LinkItem>
               <TreeView.Item defaultExpanded>
@@ -127,17 +127,17 @@ export const FileTreeWithoutDirectoryLinks: Story = () => {
                       <FileIcon />
                     </TreeView.LeadingVisual>
                     Button.tsx
-                    <TreeView.TrailingVisual>
-                      <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" aria-label="modified" />
+                    <TreeView.TrailingVisual label="modified">
+                      <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" />
                     </TreeView.TrailingVisual>
                   </TreeView.LinkItem>
                   <TreeView.LinkItem href="#button-test-tsx">
                     <TreeView.LeadingVisual>
                       <FileIcon />
                     </TreeView.LeadingVisual>
                     Button.test.tsx
-                    <TreeView.TrailingVisual>
-                      <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" aria-label="modified" />
+                    <TreeView.TrailingVisual label="modified">
+                      <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" />
                     </TreeView.TrailingVisual>
                   </TreeView.LinkItem>
                 </TreeView.SubTree>
@@ -147,8 +147,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => {
                   <FileIcon />
                 </TreeView.LeadingVisual>
                 ReallyLongFileNameThatShouldBeTruncated.tsx
-                <TreeView.TrailingVisual>
-                  <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" aria-label="modified" />
+                <TreeView.TrailingVisual label="modified">
+                  <StyledOcticon icon={DiffModifiedIcon} color="attention.fg" />
                 </TreeView.TrailingVisual>
               </TreeView.Item>
             </TreeView.SubTree>
@@ -164,17 +164,17 @@ export const FileTreeWithoutDirectoryLinks: Story = () => {
                   <FileIcon />
                 </TreeView.LeadingVisual>
                 index.html
-                <TreeView.TrailingVisual>
-                  <StyledOcticon icon={DiffRenamedIcon} aria-label="renamed" />
+                <TreeView.TrailingVisual label="renamed">
+                  <StyledOcticon icon={DiffRenamedIcon} />
                 </TreeView.TrailingVisual>
               </TreeView.LinkItem>
               <TreeView.LinkItem href="#favicon-ico">
                 <TreeView.LeadingVisual>
                   <FileIcon />
                 </TreeView.LeadingVisual>
                 favicon.ico
-                <TreeView.TrailingVisual>
-                  <StyledOcticon icon={DiffRemovedIcon} color="danger.fg" aria-label="removed" />
+                <TreeView.TrailingVisual label="removed">
+                  <StyledOcticon icon={DiffRemovedIcon} color="danger.fg" />
                 </TreeView.TrailingVisual>
               </TreeView.LinkItem>
             </TreeView.SubTree>

diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx
@@ -235,6 +235,89 @@ describe('Markup', () => {
     // 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">
+        <TreeView.Item>
+          <TreeView.LeadingVisual label="leading">
+            <svg aria-hidden={true} />
+          </TreeView.LeadingVisual>
+          Item 1
+        </TreeView.Item>
+        <TreeView.Item>
+          <TreeView.LeadingVisual>
+            <svg aria-hidden={true} />
+          </TreeView.LeadingVisual>
+          Item 2
+        </TreeView.Item>
+      </TreeView>
+    )
+    const item = getByLabelText(/Item 1/)
+    expect(item).toHaveAccessibleDescription('leading')
+
+    const noDescription = getByLabelText(/Item 2/)
+    expect(noDescription).not.toHaveAccessibleDescription()
+  })
+
+  it('should be described by trailing visuals', () => {
+    const {getByLabelText} = renderWithTheme(
+      <TreeView aria-label="Test tree">
+        <TreeView.Item>
+          Item 1
+          <TreeView.TrailingVisual label="trailing">
+            <svg aria-hidden={true} />
+          </TreeView.TrailingVisual>
+        </TreeView.Item>
+        <TreeView.Item>
+          Item 2
+          <TreeView.TrailingVisual>
+            <svg aria-hidden={true} />
+          </TreeView.TrailingVisual>
+        </TreeView.Item>
+      </TreeView>
+    )
+    const item = getByLabelText(/Item 1/)
+    expect(item).toHaveAccessibleDescription('trailing')
+
+    const noDescription = getByLabelText(/Item 2/)
+    expect(noDescription).not.toHaveAccessibleDescription()
+  })
+
+  it('should be described by leading and trailing visuals', () => {
+    const {getByLabelText} = renderWithTheme(
+      <TreeView aria-label="Test tree">
+        <TreeView.Item>
+          <TreeView.LeadingVisual label="leading">
+            <svg aria-hidden={true} />
+          </TreeView.LeadingVisual>
+          Item 1
+          <TreeView.TrailingVisual label="trailing">
+            <svg aria-hidden={true} />
+          </TreeView.TrailingVisual>
+        </TreeView.Item>
+        <TreeView.Item>
+          <TreeView.LeadingVisual>
+            <svg aria-hidden={true} />
+          </TreeView.LeadingVisual>
+          Item 2
+          <TreeView.TrailingVisual>
+            <svg aria-hidden={true} />
+          </TreeView.TrailingVisual>
+        </TreeView.Item>
+      </TreeView>
+    )
+    const item = getByLabelText(/Item 1/)
+    expect(item).toHaveAccessibleDescription('leading trailing')
+
+    const noDescription = getByLabelText(/Item 2/)
+    // Note: it seems the computed description here is a string with a single
+    // space due to the implementation of `aria-describedby`. We currently set
+    // both trailing and visual and when the nodes are not found in
+    // `aria-describedby="uuid-leading uuid-trailing"` then it computes to a
+    // space
+    expect(noDescription).toHaveAccessibleDescription(' ')
+  })
 })
 
 describe('Keyboard interactions', () => {
@@ -812,7 +895,13 @@ describe('Keyboard interactions', () => {
     })
 
     it('navigates to href if provided', () => {
-      const windowSpy = jest.spyOn(window, 'open')
+      const windowSpy = jest
+        .spyOn(window, 'open')
+        .mockImplementation(
+          (_url?: string | URL | undefined, _target?: string | undefined, _features?: string | undefined) => {
+            return null
+          }
+        )
       const onSelect = jest.fn()
       const {getByRole} = renderWithTheme(
         <TreeView aria-label="Test tree">

diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx
@@ -39,11 +39,15 @@ const ItemContext = React.createContext<{
   level: number
   isExpanded: boolean
   expandParents: () => void
+  leadingVisualId: string
+  trailingVisualId: string
 }>({
   itemId: '',
   level: 1,
   isExpanded: false,
-  expandParents: () => {}
+  expandParents: () => {},
+  leadingVisualId: '',
+  trailingVisualId: ''
 })
 
 // ----------------------------------------------------------------------------
@@ -128,6 +132,8 @@ const Item: React.FC<TreeViewItemProps> = ({
   const {setActiveDescendant} = React.useContext(RootContext)
   const itemId = useSSRSafeId()
   const labelId = useSSRSafeId()
+  const leadingVisualId = useSSRSafeId()
+  const trailingVisualId = useSSRSafeId()
   const itemRef = React.useRef<HTMLLIElement>(null)
   const [isExpanded, setIsExpanded] = useControllableState({
     name: itemId,
@@ -204,12 +210,22 @@ const Item: React.FC<TreeViewItemProps> = ({
   }, [toggle, onSelect, setIsExpanded])
 
   return (
-    <ItemContext.Provider value={{itemId, level: level + 1, isExpanded, expandParents: expandParentsAndSelf}}>
+    <ItemContext.Provider
+      value={{
+        itemId,
+        level: level + 1,
+        isExpanded,
+        expandParents: expandParentsAndSelf,
+        leadingVisualId,
+        trailingVisualId
+      }}
+    >
       <li
         id={itemId}
         ref={itemRef}
         role="treeitem"
         aria-labelledby={labelId}
+        aria-describedby={`${leadingVisualId} ${trailingVisualId}`}
         aria-level={level}
         aria-expanded={hasSubTree ? isExpanded : undefined}
         aria-current={isCurrentItem ? 'true' : undefined}
@@ -512,26 +528,39 @@ function useSubTree(children: React.ReactNode) {
 
 export type TreeViewVisualProps = {
   children: React.ReactNode | ((props: {isExpanded: boolean}) => React.ReactNode)
+  // Provide an accessible name for the visual. This should provide information
+  // about what the visual indicates or represents
+  label?: string
 }
 
 const LeadingVisual: React.FC<TreeViewVisualProps> = props => {
-  const {isExpanded} = React.useContext(ItemContext)
+  const {isExpanded, leadingVisualId} = React.useContext(ItemContext)
   const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children
   return (
     <Slot name="LeadingVisual">
-      <Box sx={{display: 'flex', color: 'fg.muted'}}>{children}</Box>
+      <VisuallyHidden aria-hidden={true} id={leadingVisualId}>
+        {props.label}
+      </VisuallyHidden>
+      <Box aria-hidden={true} sx={{display: 'flex', color: 'fg.muted'}}>
+        {children}
+      </Box>
     </Slot>
   )
 }
 
 LeadingVisual.displayName = 'TreeView.LeadingVisual'
 
 const TrailingVisual: React.FC<TreeViewVisualProps> = props => {
-  const {isExpanded} = React.useContext(ItemContext)
+  const {isExpanded, trailingVisualId} = React.useContext(ItemContext)
   const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children
   return (
     <Slot name="TrailingVisual">
-      <Box sx={{display: 'flex', color: 'fg.muted'}}>{children}</Box>
+      <VisuallyHidden aria-hidden={true} id={trailingVisualId}>
+        {props.label}
+      </VisuallyHidden>
+      <Box aria-hidden={true} sx={{display: 'flex', color: 'fg.muted'}}>
+        {children}
+      </Box>
     </Slot>
   )
 }