ActionList: Add ActionList.GroupHeading component and update the existing internal Header component for better semantics

.changeset/giant-radios-sniff.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+ActionList: Add ActionList.GroupHeading component to label the group lists and update labelling semantics for accessibility

src/ActionList/ActionList.docs.json

@@ -183,6 +183,35 @@
         }
       ]
     },
+    {
+      "name": "ActionList.GroupHeading",
+      "props": [
+        {
+          "name": "children",
+          "type": "React.ReactNode",
+          "defaultValue": "",
+          "required": true,
+          "description": "Use to give a heading to the groups"
+        },
+        {
+          "name": "variant",
+          "type": "'filled' | 'subtle'",
+          "defaultValue": "'subtle'",
+          "description": "`filled` style has a background color and top and bottom borders. Subtle style has no background or borders."
+        },
+        {
+          "name": "as",
+          "type": "h1 | h2 | h3 | h4 | h5 | h6",
+          "defaultValue": "h3",
+          "required": false,
+          "description": "The level of the heading and it is only required (enforce by runtime warning) for lists. (i.e. not required for ActionMenu or listbox roles)"
+        },
+        {
+          "name": "sx",
+          "type": "SystemStyleObject"
+        }
+      ]
+    },
     {
       "name": "ActionList.Group",
       "props": [
@@ -230,4 +259,4 @@
       ]
     }
   ]
-}
+}

src/ActionList/ActionList.test.tsx

@@ -215,4 +215,100 @@ describe('ActionList', () => {
     expect(spy).toHaveBeenCalled()
     spy.mockRestore()
   })
+  it('should render the ActionList.GroupHeading component as a heading with the given heading level', async () => {
+    const container = HTMLRender(
+      <ActionList>
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group>
+          <ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const heading = container.getByRole('heading', {level: 2})
+    expect(heading).toBeInTheDocument()
+    expect(heading).toHaveTextContent('Group Heading')
+  })
+  it('should throw a warning if ActionList.Group is used without as prop when no role is specified (for list role)', async () => {
+    const spy = jest.spyOn(console, 'warn').mockImplementationOnce(() => {})
+
+    HTMLRender(
+      <ActionList>
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group>
+          <ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    expect(spy).toHaveBeenCalledTimes(1)
+    spy.mockRestore()
+  })
+  it('should render the ActionList.GroupHeading component as a span (not a heading tag) when role is specified as listbox', async () => {
+    const container = HTMLRender(
+      <ActionList role="listbox">
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group>
+          <ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const label = container.getByText('Group Heading')
+    expect(label).toBeInTheDocument()
+    expect(label.tagName).toEqual('SPAN')
+  })
+  it('should render the ActionList.GroupHeading component as a span with role="presentation" and aria-hidden="true" when role is specified as listbox', async () => {
+    const container = HTMLRender(
+      <ActionList role="listbox">
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group>
+          <ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const label = container.getByText('Group Heading')
+    const wrapper = label.parentElement
+    expect(wrapper).toHaveAttribute('role', 'presentation')
+    expect(wrapper).toHaveAttribute('aria-hidden', 'true')
+  })
+  it('should label the list with the group heading id', async () => {
+    const {container, getByText} = HTMLRender(
+      <ActionList>
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group data-test-id="ActionList.Group">
+          <ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
+          <ActionList.Item>Item</ActionList.Item>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
+    const heading = getByText('Group Heading')
+    expect(list).toHaveAttribute('aria-labelledby', heading.id)
+  })
+  it('should NOT label the list with the group heading id when role is specified', async () => {
+    const {container, getByText} = HTMLRender(
+      <ActionList role="listbox">
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group data-test-id="ActionList.Group">
+          <ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
+          <ActionList.Item>Item</ActionList.Item>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
+    const heading = getByText('Group Heading')
+    expect(list).not.toHaveAttribute('aria-labelledby', heading.id)
+  })
+  it('should label the list using aria-label when role is specified', async () => {
+    const {container, getByText} = HTMLRender(
+      <ActionList role="listbox">
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Group data-test-id="ActionList.Group">
+          <ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
+          <ActionList.Item>Item</ActionList.Item>
+        </ActionList.Group>
+      </ActionList>,
+    )
+    const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
+    const heading = getByText('Group Heading')
+    expect(list).toHaveAttribute('aria-label', heading.textContent)
+  })
 })

src/ActionList/Group.tsx

@@ -1,9 +1,14 @@
 import React from 'react'
 import {useId} from '../hooks/useId'
 import Box from '../Box'
-import {SxProp} from '../sx'
+import {SxProp, BetterSystemStyleObject, merge} from '../sx'
 import {ListContext, ActionListProps} from './List'
 import {AriaRole} from '../utils/types'
+import {default as Heading} from '../Heading'
+import type {ActionListHeadingProps} from './Heading'
+import {useSlots} from '../hooks/useSlots'
+import {defaultSxProp} from '../utils/defaultSxProp'
+import {warning} from '../utils/warning'
 
 export type ActionListGroupProps = {
   /**
@@ -32,8 +37,11 @@ export type ActionListGroupProps = {
     selectionVariant?: ActionListProps['selectionVariant'] | false
   }
 
-type ContextProps = Pick<ActionListGroupProps, 'selectionVariant'>
-export const GroupContext = React.createContext<ContextProps>({})
+type ContextProps = Pick<ActionListGroupProps, 'selectionVariant'> & {groupHeadingId: string | undefined}
+export const GroupContext = React.createContext<ContextProps>({
+  groupHeadingId: undefined,
+  selectionVariant: undefined,
+})
 
 export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
   title,
@@ -44,9 +52,25 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
   sx = {},
   ...props
 }) => {
-  const labelId = useId()
+  const id = useId()
   const {role: listRole} = React.useContext(ListContext)
 
+  const [slots, childrenWithoutSlots] = useSlots(props.children, {
+    groupHeading: GroupHeading,
+  })
+
+  let groupHeadingId = undefined
+
+  // ActionList.GroupHeading
+  if (slots.groupHeading) {
+    // If there is an id prop passed in the ActionList.GroupHeading, use it otherwise use the generated id.
+    groupHeadingId = slots.groupHeading.props.id ?? id
+  }
+  // Supports the deprecated `title` prop
+  if (title) {
+    groupHeadingId = id
+  }
+
   return (
     <Box
       as="li"
@@ -58,32 +82,59 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
       }}
       {...props}
     >
-      {title && <Header title={title} variant={variant} auxiliaryText={auxiliaryText} labelId={labelId} />}
-      <GroupContext.Provider value={{selectionVariant}}>
+      <GroupContext.Provider value={{selectionVariant, groupHeadingId}}>
+        {title && !slots.groupHeading ? (
+          <GroupHeading title={title} variant={variant} auxiliaryText={auxiliaryText} />
+        ) : null}
+        {!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null}
         <Box
           as="ul"
           sx={{paddingInlineStart: 0}}
-          aria-labelledby={title ? labelId : undefined}
+          // if listRole is set (listbox or menu), we don't label the list with the groupHeadingId
+          // because the heading is hidden from the accessibility tree and only used for presentation role.
+          // We will instead use aria-label to label the list. See a line below.
+          aria-labelledby={listRole ? undefined : groupHeadingId}
+          aria-label={listRole ? title ?? (slots.groupHeading?.props.children as string) : undefined}
           role={role || (listRole && 'group')}
         >
-          {props.children}
+          {slots.groupHeading ? childrenWithoutSlots : props.children}
         </Box>
       </GroupContext.Provider>
     </Box>
   )
 }
 
-export type HeaderProps = Pick<ActionListGroupProps, 'variant' | 'title' | 'auxiliaryText'> & {
-  labelId: string
-}
+export type GroupHeadingProps = Pick<ActionListGroupProps, 'variant' | 'title' | 'auxiliaryText'> &
+  Omit<ActionListHeadingProps, 'as'> &
+  SxProp &
+  React.HTMLAttributes<HTMLElement> & {
+    as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+  }
 
 /**
- * Displays the name and description of a `Group`.
+ * Heading of  a `Group`.
  *
- * For visual presentation only. It's hidden from screen readers.
+ * As default, the role of ActionList is "list" and therefore group heading is rendered as a proper heading tag.
+ * If the role is "listbox" or "menu" (ActionMenu), the group heading is rendered as a div with presentation role and it is
+ * hidden from the accessibility tree due to the limitation of listbox children. https://w3c.github.io/aria/#listbox
+ * groups under menu or listbox are labelled by `aria-label`
  */
-const Header: React.FC<React.PropsWithChildren<HeaderProps>> = ({variant, title, auxiliaryText, labelId, ...props}) => {
-  const {variant: listVariant} = React.useContext(ListContext)
+export const GroupHeading: React.FC<React.PropsWithChildren<GroupHeadingProps>> = ({
+  as,
+  variant,
+  title,
+  auxiliaryText,
+  children,
+  sx = defaultSxProp,
+  ...props
+}) => {
+  const {variant: listVariant, role: listRole} = React.useContext(ListContext)
+  const {groupHeadingId} = React.useContext(GroupContext)
+  // for list role, the headings are proper heading tags, for menu and listbox, they are just representational and divs
+  warning(
+    listRole === undefined && children !== undefined && as === undefined,
+    `You are setting a heading for a list, that requires a heading level. Please use 'as' prop to set a proper heading level.`,
+  )
 
   const styles = {
     paddingY: '6px',
@@ -102,9 +153,20 @@ const Header: React.FC<React.PropsWithChildren<HeaderProps>> = ({variant, title,
   }
 
   return (
-    <Box sx={styles} role="presentation" aria-hidden="true" {...props}>
-      <span id={labelId}>{title}</span>
-      {auxiliaryText && <span>{auxiliaryText}</span>}
-    </Box>
+    <>
+      {listRole ? (
+        <Box sx={styles} role="presentation" aria-hidden="true" {...props}>
+          <span id={groupHeadingId}>{title ?? children}</span>
+          {auxiliaryText && <span>{auxiliaryText}</span>}
+        </Box>
+      ) : (
+        <>
+          <Heading as={as || 'h3'} id={groupHeadingId} sx={merge<BetterSystemStyleObject>(styles, sx)} {...props}>
+            {title ?? children}
+          </Heading>
+          {auxiliaryText && <span>{auxiliaryText}</span>}
+        </>
+      )}
+    </>
   )
 }

src/ActionList/index.ts

@@ -1,5 +1,5 @@
 import {List} from './List'
-import {Group} from './Group'
+import {Group, GroupHeading} from './Group'
 import {Item} from './Item'
 import {LinkItem} from './LinkItem'
 import {Divider} from './Divider'
@@ -43,4 +43,7 @@ export const ActionList = Object.assign(List, {
 
   /** Heading for an `ActionList`. */
   Heading,
+
+  /** Heading for `ActionList.Group` */
+  GroupHeading,
 })

src/NavList/__snapshots__/NavList.test.tsx.snap

@@ -399,16 +399,6 @@ exports[`NavList renders with groups 1`] = `
   margin-top: 8px;
 }
 
-.c3 {
-  padding-top: 6px;
-  padding-bottom: 6px;
-  padding-left: 16px;
-  padding-right: 16px;
-  font-size: 12px;
-  font-weight: 600;
-  color: #656d76;
-}
-
 .c4 {
   padding-inline-start: 0;
 }
@@ -436,6 +426,19 @@ exports[`NavList renders with groups 1`] = `
   font-weight: 400;
 }
 
+.c3 {
+  font-weight: 600;
+  font-size: 32px;
+  margin: 0;
+  padding-top: 6px;
+  padding-bottom: 6px;
+  padding-left: 16px;
+  padding-right: 16px;
+  font-size: 12px;
+  font-weight: 600;
+  color: #656d76;
+}
+
 .c0 {
   margin: 0;
   padding-inline-start: 0;
@@ -723,17 +726,12 @@ exports[`NavList renders with groups 1`] = `
       <li
         class="c2"
       >
-        <div
-          aria-hidden="true"
+        <h3
           class="c3"
-          role="presentation"
+          id="react-aria-2"
         >
-          <span
-            id="react-aria-2"
-          >
-            Overview
-          </span>
-        </div>
+          Overview
+        </h3>
         <ul
           aria-labelledby="react-aria-2"
           class="c4"
@@ -772,17 +770,12 @@ exports[`NavList renders with groups 1`] = `
       <li
         class="c2"
       >
-        <div
-          aria-hidden="true"
+        <h3
           class="c3"
-          role="presentation"
+          id="react-aria-4"
         >
-          <span
-            id="react-aria-4"
-          >
-            Components
-          </span>
-        </div>
+          Components
+        </h3>
         <ul
           aria-labelledby="react-aria-4"
           class="c4"