fix(PageHeader): add role prop and aria-label in top-level element (#…

…4956) * fix(PageHeader): add role prop and aria-label in top-level element * docs(PageHeader): add doc for new role prop * docs(PageHeader) add aria-label to sb examples and tests * Create fifty-rockets-joke.md * docs(PageHeader) add aria-label to sb example * test(PageHeader) add aria-label to test
primer · Sep 25, 2024 · 15cb90f · 15cb90f
1 parent 1691e46
commit 15cb90f
Show file tree

Hide file tree

Showing 11 changed files with 104 additions and 38 deletions.
diff --git a/.changeset/fifty-rockets-joke.md b/.changeset/fifty-rockets-joke.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+fix(PageHeader): add role prop and aria-label in top-level element
diff --git a/packages/react/src/Hidden/Hidden.examples.stories.tsx b/packages/react/src/Hidden/Hidden.examples.stories.tsx
@@ -22,7 +22,7 @@ const setViewportParamToNarrow = {
 }
 export const Webhooks = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Webhooks">
       <PageHeader.ContextArea>
         <PageHeader.ParentLink href="http://github.com">Repository settings</PageHeader.ParentLink>
       </PageHeader.ContextArea>
@@ -51,7 +51,7 @@ WebhooksOnNarrowViewport.parameters = setViewportParamToNarrow
 
 export const PullRequestPage = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Add Hidden utility component">
       <PageHeader.ContextArea>
         <PageHeader.ParentLink href="http://github.com">Pull requests</PageHeader.ParentLink>
       </PageHeader.ContextArea>

diff --git a/packages/react/src/PageHeader/PageHeader.dev.stories.tsx b/packages/react/src/PageHeader/PageHeader.dev.stories.tsx
@@ -18,7 +18,11 @@ export default meta
 
 export const LargeVariantWithMultilineTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader
+      role="banner"
+      aria-label="Title long title some extra loooong looong words here some extra loooong looong words here some extra loooong
+          looong words here some extra loooong looong words here some extra loooong looong words here"
+    >
       <PageHeader.LeadingAction>
         <IconButton aria-label="Edit" icon={PencilIcon} variant="invisible" />
       </PageHeader.LeadingAction>
@@ -46,7 +50,7 @@ export const LargeVariantWithMultilineTitle = () => (
 
 export const ArrayTypeFontSizeOnTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Issue Title">
       <PageHeader.TitleArea>
         <PageHeader.Title
           sx={{
@@ -64,7 +68,7 @@ export const ArrayTypeFontSizeOnTitle = () => (
 
 export const ThemeBaseFontSizeOnTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Issue Title">
       <PageHeader.TitleArea>
         <PageHeader.Title
           sx={{
@@ -80,7 +84,7 @@ export const ThemeBaseFontSizeOnTitle = () => (
 
 export const StringTypeFontSizeOnTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Issue Title">
       <PageHeader.TitleArea>
         <PageHeader.Title
           sx={{

diff --git a/packages/react/src/PageHeader/PageHeader.docs.json b/packages/react/src/PageHeader/PageHeader.docs.json
@@ -10,7 +10,7 @@
       "name": "aria-label",
       "type": "string | undefined",
       "defaultValue": "",
-      "description": "A unique label for the rendered main landmark"
+      "description": "A unique label for the rendered landmark"
     },
     {
       "name": "className",
@@ -24,6 +24,11 @@
       "defaultValue": "false",
       "description": "Whether the content is hidden."
     },
+    {
+      "name": "role",
+      "type": "AriaRole",
+      "description": "The ARIA role to assign to the top-level node of this component."
+    },
     {
       "name": "sx",
       "type": "SystemStyleObject"

diff --git a/packages/react/src/PageHeader/PageHeader.examples.stories.tsx b/packages/react/src/PageHeader/PageHeader.examples.stories.tsx
@@ -52,7 +52,7 @@ const setViewportParamToNarrow = {
 }
 export const Webhooks = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Webhooks">
       <PageHeader.TitleArea>
         <PageHeader.Title as="h2">Webhooks</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -78,7 +78,10 @@ WebhooksOnNarrowViewport.parameters = setViewportParamToNarrow
 
 export const PullRequestPage = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader
+      role="banner"
+      aria-label="PageHeader component initial layout explorations extra long pull request title"
+    >
       <PageHeader.TitleArea>
         <PageHeader.Title as="h1">
           PageHeader component initial layout explorations extra long pull request title
@@ -146,7 +149,7 @@ PullRequestPageOnNarrowViewport.parameters = setViewportParamToNarrow
 
 export const FilesPage = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="PageHeader.tsx">
       <PageHeader.TitleArea sx={{alignItems: 'center'}}>
         <Text sx={{color: 'rgb(101, 109, 118)', fontSize: '14px', fontWeight: 'normal'}}>/</Text>
         <PageHeader.Title as="h1" sx={{fontSize: '14px', height: '21px'}}>

diff --git a/packages/react/src/PageHeader/PageHeader.features.stories.tsx b/packages/react/src/PageHeader/PageHeader.features.stories.tsx
@@ -31,7 +31,7 @@ const meta: Meta = {
 
 export const HasTitleOnly = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -41,7 +41,7 @@ export const HasTitleOnly = () => (
 
 export const HasLargeTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea variant="large">
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -51,7 +51,7 @@ export const HasLargeTitle = () => (
 
 export const WithLeadingAndTrailingVisuals = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.LeadingVisual>
           <GitPullRequestIcon />
@@ -67,7 +67,7 @@ export const WithLeadingAndTrailingVisuals = () => (
 
 export const WithLeadingVisualHiddenOnRegularViewport = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.LeadingVisual hidden={{regular: true}}>
           <GitPullRequestIcon />
@@ -89,7 +89,7 @@ WithLeadingVisualHiddenOnRegularViewport.parameters = {
 
 export const WithActions = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -107,7 +107,7 @@ export const WithActions = () => (
 
 export const WithDescriptionSlot = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Add-pageheader-docs">
       <PageHeader.TitleArea>
         <PageHeader.Title>add-pageheader-docs</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -125,7 +125,7 @@ export const WithDescriptionSlot = () => (
 
 export const WithNavigationSlot = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Pull request title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Pull request title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -151,7 +151,7 @@ export const WithNavigationSlot = () => (
 
 export const WithCustomNavigation = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Pull request title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Pull request title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -173,7 +173,7 @@ export const WithCustomNavigation = () => (
 
 export const WithLeadingAndTrailingActions = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -189,7 +189,7 @@ export const WithLeadingAndTrailingActions = () => (
 
 export const WithParentLinkAndActionsOfContextArea = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -215,7 +215,7 @@ WithParentLinkAndActionsOfContextArea.parameters = {
 
 export const WithContextBarAndActionsOfContextArea = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
@@ -250,7 +250,7 @@ WithContextBarAndActionsOfContextArea.parameters = {
 }
 export const WithActionsThatHaveResponsiveContent = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Webhooks">
       <PageHeader.TitleArea>
         <PageHeader.Title as="h2">Webhooks</PageHeader.Title>
       </PageHeader.TitleArea>

diff --git a/packages/react/src/PageHeader/PageHeader.stories.tsx b/packages/react/src/PageHeader/PageHeader.stories.tsx
@@ -189,7 +189,7 @@ export default meta
 
 export const Playground: StoryFn = args => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader aria-label={args.Title} role="banner">
       <PageHeader.TitleArea
         variant={{
           narrow: args['Title.variant'],
@@ -287,7 +287,7 @@ export const Playground: StoryFn = args => (
 
 export const Default = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader role="banner" aria-label="Title">
       <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>

diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx
@@ -29,7 +29,7 @@ describe('PageHeader', () => {
     Component: PageHeader,
     options: {skipAs: true, skipSx: true},
     toRender: () => (
-      <PageHeader>
+      <PageHeader role="banner" aria-label="Banner">
         <PageHeader.TitleArea></PageHeader.TitleArea>
         <PageHeader.ContextArea></PageHeader.ContextArea>
         <PageHeader.Description></PageHeader.Description>
@@ -138,7 +138,7 @@ describe('PageHeader', () => {
   })
   it('respects the title variant prop', () => {
     const {getByText} = render(
-      <PageHeader>
+      <PageHeader role="banner" aria-label="Title">
         <PageHeader.TitleArea variant="large">
           <PageHeader.Title>Title</PageHeader.Title>
         </PageHeader.TitleArea>
@@ -149,7 +149,7 @@ describe('PageHeader', () => {
   })
   it('renders "aria-label" prop when Navigation is rendered as "nav" landmark', () => {
     const {getByLabelText, getByText} = render(
-      <PageHeader>
+      <PageHeader role="banner" aria-label="Title">
         <PageHeader.TitleArea>
           <PageHeader.Title>Title</PageHeader.Title>
         </PageHeader.TitleArea>
@@ -161,9 +161,9 @@ describe('PageHeader', () => {
     expect(getByLabelText('Custom')).toBeInTheDocument()
     expect(getByText('Navigation')).toHaveAttribute('aria-label', 'Custom')
   })
-  it('does not renders "aria-label" prop when Navigation is rendered as "div"', () => {
+  it('does not render "aria-label" prop when Navigation is rendered as "div"', () => {
     const {getByText} = render(
-      <PageHeader>
+      <PageHeader role="banner" aria-label="Title">
         <PageHeader.TitleArea>
           <PageHeader.Title>Title</PageHeader.Title>
         </PageHeader.TitleArea>
@@ -175,7 +175,7 @@ describe('PageHeader', () => {
   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(
-      <PageHeader>
+      <PageHeader role="banner" aria-label="Title">
         <PageHeader.TitleArea>
           <PageHeader.Title>Title</PageHeader.Title>
         </PageHeader.TitleArea>
@@ -186,4 +186,44 @@ describe('PageHeader', () => {
 
     consoleSpy.mockRestore()
   })
+  it('does not render "role" attribute when not explicitly specified', () => {
+    const {container} = render(
+      <PageHeader>
+        <PageHeader.TitleArea>
+          <PageHeader.Title>Title</PageHeader.Title>
+        </PageHeader.TitleArea>
+      </PageHeader>,
+    )
+    expect(container.firstChild).not.toHaveAttribute('role')
+  })
+  it('renders "role" attribute when explicitly specified', () => {
+    const {container} = render(
+      <PageHeader role="banner">
+        <PageHeader.TitleArea>
+          <PageHeader.Title>Title</PageHeader.Title>
+        </PageHeader.TitleArea>
+      </PageHeader>,
+    )
+    expect(container.firstChild).toHaveAttribute('role', 'banner')
+  })
+  it('does not render "aria-label" attribute when not explicitly specified', () => {
+    const {container} = render(
+      <PageHeader role="banner">
+        <PageHeader.TitleArea>
+          <PageHeader.Title>Title</PageHeader.Title>
+        </PageHeader.TitleArea>
+      </PageHeader>,
+    )
+    expect(container.firstChild).not.toHaveAttribute('aria-label')
+  })
+  it('renders custom "aria-label" attribute when explicitly specified', () => {
+    const {container} = render(
+      <PageHeader aria-label="Custom aria-label" role="banner">
+        <PageHeader.TitleArea>
+          <PageHeader.Title>Title</PageHeader.Title>
+        </PageHeader.TitleArea>
+      </PageHeader>,
+    )
+    expect(container.firstChild).toHaveAttribute('aria-label', 'Custom aria-label')
+  })
 })
diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx
@@ -13,6 +13,7 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti
 import {getBreakpointDeclarations} from '../utils/getBreakpointDeclarations'
 import {warning} from '../utils/warning'
 import {useProvidedRefOrCreate} from '../hooks'
+import type {AriaRole} from '../utils/types'
 
 const GRID_ROW_ORDER = {
   ContextArea: 1,
@@ -63,10 +64,11 @@ export type PageHeaderProps = {
   'aria-label'?: React.AriaAttributes['aria-label']
   as?: React.ElementType | 'header' | 'div'
   className?: string
+  role?: AriaRole
 } & SxProp
 
 const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeaderProps>>(
-  ({children, className, sx = {}, as = 'div'}, forwardedRef) => {
+  ({children, className, sx = {}, as = 'div', 'aria-label': ariaLabel, role}, forwardedRef) => {
     const rootStyles = {
       display: 'grid',
       // We have max 5 columns.
@@ -158,7 +160,14 @@ const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeader
       [children, rootRef],
     )
     return (
-      <Box ref={rootRef} as={as} className={className} sx={merge<BetterSystemStyleObject>(rootStyles, sx)}>
+      <Box
+        ref={rootRef}
+        as={as}
+        className={className}
+        sx={merge<BetterSystemStyleObject>(rootStyles, sx)}
+        aria-label={ariaLabel}
+        role={role}
+      >
         {children}
       </Box>
     )