From fb4f8deaad7bc3522b36abd9d427e588bffb51f3 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 14 Oct 2022 15:59:13 -0500 Subject: [PATCH 1/8] feat(TreeView): add label prop for Leading and Trailing Visual --- src/TreeView/TreeView.stories.tsx | 24 ++++---- src/TreeView/TreeView.test.tsx | 92 ++++++++++++++++++++++++++++++- src/TreeView/TreeView.tsx | 35 ++++++++++-- 3 files changed, 132 insertions(+), 19 deletions(-) diff --git a/src/TreeView/TreeView.stories.tsx b/src/TreeView/TreeView.stories.tsx index e6b8296c9d2..ad4c1637227 100644 --- a/src/TreeView/TreeView.stories.tsx +++ b/src/TreeView/TreeView.stories.tsx @@ -112,8 +112,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { Avatar.tsx - - + + @@ -127,8 +127,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { Button.tsx - - + + @@ -136,8 +136,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { Button.test.tsx - - + + @@ -147,8 +147,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { ReallyLongFileNameThatShouldBeTruncated.tsx - - + + @@ -164,8 +164,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { index.html - - + + @@ -173,8 +173,8 @@ export const FileTreeWithoutDirectoryLinks: Story = () => { favicon.ico - - + + diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index bfa5cafc2ff..de9dc13a2f6 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -1,3 +1,4 @@ +import {computeAccessibleDescription} from 'dom-accessibility-api' import {fireEvent, render} from '@testing-library/react' import React from 'react' import {ThemeProvider} from '../ThemeProvider' @@ -235,6 +236,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( + + + + + + Item 1 + + + + + + Item 2 + + + ) + 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( + + + Item 1 + + + + + + Item 2 + + + + + + ) + 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( + + + + + + Item 1 + + + + + + + + + Item 2 + + + + + + ) + 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 +896,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( diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index 788339c5db2..ec6e3fd059a 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -17,6 +17,7 @@ import {Theme} from '../ThemeProvider' import createSlots from '../utils/create-slots' import {useActiveDescendant} from './useActiveDescendant' import {useTypeahead} from './useTypeahead' +import VisuallyHidden from '../_VisuallyHidden' // ---------------------------------------------------------------------------- // Context @@ -32,10 +33,14 @@ const ItemContext = React.createContext<{ level: number isExpanded: boolean expandParents: () => void + leadingVisualId: string + trailingVisualId: string }>({ level: 1, isExpanded: false, - expandParents: () => {} + expandParents: () => {}, + leadingVisualId: '', + trailingVisualId: '' }) // ---------------------------------------------------------------------------- @@ -108,6 +113,8 @@ const Item: React.FC = ({ const {setActiveDescendant} = React.useContext(RootContext) const itemId = useSSRSafeId() const labelId = useSSRSafeId() + const leadingVisualId = useSSRSafeId() + const trailingVisualId = useSSRSafeId() const itemRef = React.useRef(null) const [isExpanded, setIsExpanded] = useControllableState({ name: itemId, @@ -184,12 +191,15 @@ const Item: React.FC = ({ }, [toggle, onSelect, setIsExpanded]) return ( - +
  • 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 = props => { - const {isExpanded} = React.useContext(ItemContext) + const {isExpanded, leadingVisualId} = React.useContext(ItemContext) const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children return ( - {children} + + {props.label} + + + {children} + ) } const TrailingVisual: React.FC = props => { - const {isExpanded} = React.useContext(ItemContext) + const {isExpanded, trailingVisualId} = React.useContext(ItemContext) const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children return ( - {children} + + {props.label} + + + {children} + ) } From c0bc85a39461fc55e2e5111f183ee66bf1f1bf33 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 14 Oct 2022 15:59:36 -0500 Subject: [PATCH 2/8] chore: changeset --- .changeset/pretty-bats-cough.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pretty-bats-cough.md diff --git a/.changeset/pretty-bats-cough.md b/.changeset/pretty-bats-cough.md new file mode 100644 index 00000000000..08ae315e3ad --- /dev/null +++ b/.changeset/pretty-bats-cough.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add support for labels in TreeView.LeadingVisual and TreeView.TrailingVisual From 0de783fcd9ec2ea30630abba38a2ae8aba645157 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 14 Oct 2022 16:05:42 -0500 Subject: [PATCH 3/8] chore: update tests --- src/TreeView/TreeView.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index de9dc13a2f6..320c78cdea7 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -1,4 +1,3 @@ -import {computeAccessibleDescription} from 'dom-accessibility-api' import {fireEvent, render} from '@testing-library/react' import React from 'react' import {ThemeProvider} from '../ThemeProvider' From a858647cff45fbf1ce8d054b04ea1eb6faa0985e Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 14 Oct 2022 16:22:06 -0500 Subject: [PATCH 4/8] Update .changeset/pretty-bats-cough.md Co-authored-by: Cole Bemis --- .changeset/pretty-bats-cough.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pretty-bats-cough.md b/.changeset/pretty-bats-cough.md index 08ae315e3ad..78d047714aa 100644 --- a/.changeset/pretty-bats-cough.md +++ b/.changeset/pretty-bats-cough.md @@ -1,5 +1,5 @@ --- -'@primer/react': minor +'@primer/react': patch --- Add support for labels in TreeView.LeadingVisual and TreeView.TrailingVisual From 446615d77079dbfa11869b31092e6f12cb5feb98 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 14 Oct 2022 16:25:17 -0500 Subject: [PATCH 5/8] docs: update prop table --- docs/content/TreeView.mdx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx index 0c0250368c5..415fde1fa18 100644 --- a/docs/content/TreeView.mdx +++ b/docs/content/TreeView.mdx @@ -289,6 +289,11 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree type={`| React.ReactNode | (props: {isExpanded: boolean}) => React.ReactNode`} /> + {/* */} @@ -300,6 +305,11 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree type={`| React.ReactNode | (props: {isExpanded: boolean}) => React.ReactNode`} /> + {/* */} From e06395089b965c09ace88754f705f76d8ef2fd70 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 17 Oct 2022 13:02:03 -0500 Subject: [PATCH 6/8] docs(TreeView): add note for decoarative visuals --- docs/content/TreeView.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx index 6087ae879c0..e024a93f27f 100644 --- a/docs/content/TreeView.mdx +++ b/docs/content/TreeView.mdx @@ -323,7 +323,7 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree {/* */} @@ -339,7 +339,7 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree {/* */} From 0c26d2278d1d91b3315d6e99ffb5102b9bdfb29e Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 17 Oct 2022 13:05:19 -0500 Subject: [PATCH 7/8] docs(TreeView): add note for decoarative visuals --- docs/content/TreeView.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx index e024a93f27f..baaa63588ac 100644 --- a/docs/content/TreeView.mdx +++ b/docs/content/TreeView.mdx @@ -323,7 +323,7 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree {/* */} @@ -339,7 +339,7 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree {/* */} From 26d7dbe3cc1813b99f03a97e3956b4cfa6f9431f Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 17 Oct 2022 13:05:44 -0500 Subject: [PATCH 8/8] chore: remove duplicate import --- src/TreeView/TreeView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index 1a8a6526b11..b76a49c4d5f 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -20,7 +20,6 @@ import VisuallyHidden from '../_VisuallyHidden' import {getAccessibleName} from './shared' import {getFirstChildElement, useActiveDescendant} from './useActiveDescendant' import {useTypeahead} from './useTypeahead' -import VisuallyHidden from '../_VisuallyHidden' // ---------------------------------------------------------------------------- // Context