diff --git a/.changeset/silent-rivers-sell.md b/.changeset/silent-rivers-sell.md new file mode 100644 index 00000000000..addb854fb6f --- /dev/null +++ b/.changeset/silent-rivers-sell.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +PageHeader: A11y eng review remediations diff --git a/docs/content/drafts/PageHeader.mdx b/docs/content/drafts/PageHeader.mdx index b16684f384f..6eaef75d158 100644 --- a/docs/content/drafts/PageHeader.mdx +++ b/docs/content/drafts/PageHeader.mdx @@ -121,6 +121,8 @@ import {PageHeader} from '@primer/react/drafts' ### With navigation slot +#### Utilizing a Navigation component + ```jsx live drafts @@ -145,6 +147,28 @@ import {PageHeader} from '@primer/react/drafts' ``` +#### Utilizing a custom navigation + +```jsx live drafts + + + Page Title + + + +
  • + + Item 1 + +
    • +
  • + Item 2 +
    • +     +     +     +``` + ### With ContextArea #### With parent link and actions (only visible on mobile) diff --git a/generated/components.json b/generated/components.json index f05734a9b75..f0e59d2526a 100644 --- a/generated/components.json +++ b/generated/components.json @@ -3018,7 +3018,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ParentLink", "props": [ { "name": "href", @@ -3039,7 +3039,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ContextBar", "props": [ { "name": "hidden", @@ -3054,7 +3054,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ContextAreaActions", "props": [ { "name": "hidden", @@ -3090,7 +3090,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.LeadingAction", "props": [ { "name": "hidden", @@ -3105,7 +3105,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.LeadingVisual", "props": [ { "name": "hidden", @@ -3120,7 +3120,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.Title", "props": [ { "name": "hidden", @@ -3140,7 +3140,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.TrailingVisual", "props": [ { "name": "hidden", @@ -3155,7 +3155,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.TrailingAction", "props": [ { "name": "hidden", @@ -3170,7 +3170,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.Actions", "props": [ { "name": "hidden", @@ -3202,6 +3202,22 @@ { "name": "PageHeader.Navigation", "props": [ + { + "name": "aria-label", + "type": "string", + "description": "The aria-label attribute for the navigaton component when it is rendered as a \"nav\" element." + }, + { + "name": "aria-labelledby", + "type": "string", + "description": "The aria-labelledby attribute for the navigaton component when it is rendered as a \"nav\" element." + }, + { + "name": "as", + "type": "div | nav", + "defaultValue": "\"div\"", + "description": "The HTML element used to render the navigation." + }, { "name": "hidden", "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", diff --git a/src/PageHeader/PageHeader.docs.json b/src/PageHeader/PageHeader.docs.json index 582ab767934..cfc051f30f2 100644 --- a/src/PageHeader/PageHeader.docs.json +++ b/src/PageHeader/PageHeader.docs.json @@ -44,7 +44,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ParentLink", "props": [ { "name": "href", @@ -65,7 +65,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ContextBar", "props": [ { "name": "hidden", @@ -80,7 +80,7 @@ ] }, { - "name": "PageHeader.ContextArea Children", + "name": "PageHeader.ContextAreaActions", "props": [ { "name": "hidden", @@ -116,7 +116,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.LeadingAction", "props": [ { "name": "hidden", @@ -131,7 +131,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.LeadingVisual", "props": [ { "name": "hidden", @@ -146,7 +146,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.Title", "props": [ { "name": "hidden", @@ -166,7 +166,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.TrailingVisual", "props": [ { "name": "hidden", @@ -181,7 +181,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.TrailingAction", "props": [ { "name": "hidden", @@ -196,7 +196,7 @@ ] }, { - "name": "PageHeader.TitleArea Children", + "name": "PageHeader.Actions", "props": [ { "name": "hidden", @@ -228,6 +228,22 @@ { "name": "PageHeader.Navigation", "props": [ + { + "name": "aria-label", + "type": "string", + "description": "The aria-label attribute for the navigaton component when it is rendered as a \"nav\" element." + }, + { + "name": "aria-labelledby", + "type": "string", + "description": "The aria-labelledby attribute for the navigaton component when it is rendered as a \"nav\" element." + }, + { + "name": "as", + "type": "div | nav", + "defaultValue": "\"div\"", + "description": "The HTML element used to render the navigation." + }, { "name": "hidden", "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", @@ -241,4 +257,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/src/PageHeader/PageHeader.test.tsx b/src/PageHeader/PageHeader.test.tsx index d2d00b5a908..f96fb9fd940 100644 --- a/src/PageHeader/PageHeader.test.tsx +++ b/src/PageHeader/PageHeader.test.tsx @@ -178,6 +178,36 @@ describe('PageHeader', () => { expect(getByText('Trailing Action')).toHaveStyle('height: 3rem') // add actions here }) + it('renders "aria-label" prop when Navigation is rendered as "nav" landmark', () => { + const {getByLabelText, getByText} = render( + + + Navigation + + , + ) + expect(getByLabelText('Custom')).toBeInTheDocument() + expect(getByText('Navigation')).toHaveAttribute('aria-label', 'Custom') + }) + it('does not renders "aria-label" prop when Navigation is rendered as "div"', () => { + const {getByText} = render( + + Navigation + , + ) + expect(getByText('Navigation')).not.toHaveAttribute('aria-label') + }) + it('logs a warning when the Navigation component is rendered as "nav" but no "aria-label" or "aria-labelledby" prop is provided', () => { + const consoleSpy = jest.spyOn(global.console, 'warn').mockImplementation() + render( + + Navigation + , + ) + expect(consoleSpy).toHaveBeenCalled() + + consoleSpy.mockRestore() + }) }) checkStoriesForAxeViolations('examples', '../PageHeader/') diff --git a/src/PageHeader/PageHeader.tsx b/src/PageHeader/PageHeader.tsx index 5bf2f304ccb..c876dd1a646 100644 --- a/src/PageHeader/PageHeader.tsx +++ b/src/PageHeader/PageHeader.tsx @@ -101,17 +101,7 @@ type LinkProps = Pick< export type ParentLinkProps = React.PropsWithChildren const ParentLink = React.forwardRef( - ( - { - children, - sx = {}, - href, - 'aria-label': ariaLabel = `Back to ${children}`, - as = 'a', - hidden = hiddenOnRegularAndWide, - }, - ref, - ) => { + ({children, sx = {}, href, 'aria-label': ariaLabel, as = 'a', hidden = hiddenOnRegularAndWide}, ref) => { return ( <> > = ({chil ) } +export type NavigationProps = { + as?: 'nav' | 'div' + 'aria-label'?: React.AriaAttributes['aria-label'] + 'aria-labelledby'?: React.AriaAttributes['aria-labelledby'] +} & ChildrenPropTypes + // PageHeader.Navigation: The local navigation area of the header. Visible on all viewports -const Navigation: React.FC> = ({children, sx = {}, hidden = false}) => { +const Navigation: React.FC> = ({ + children, + sx = {}, + hidden = false, + as, + 'aria-label': ariaLabel, + 'aria-labelledby': ariaLabelledBy, +}) => { + // TODO: use warning utility function when it is merged https://github.com/primer/react/pull/2901/ + if (__DEV__) { + if (as === 'nav' && !ariaLabel && !ariaLabelledBy) { + // eslint-disable-next-line no-console + console.warn( + 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', + ) + } + } return ( ( { display: 'flex', diff --git a/src/PageHeader/features.stories.tsx b/src/PageHeader/features.stories.tsx index 260945d71c4..f64c4db5c1e 100644 --- a/src/PageHeader/features.stories.tsx +++ b/src/PageHeader/features.stories.tsx @@ -147,6 +147,28 @@ export const WithNavigationSlot = () => ( ) +export const WithCustomNavigation = () => ( + + + + Pull request title + + + +
  • + + Item 1 + +
    • +
  • + Item 2 +
    • +     +     +     +     +) + export const WithLeadingAndTrailingActions = () => (