diff --git a/.changeset/giant-radios-sniff.md b/.changeset/giant-radios-sniff.md new file mode 100644 index 00000000000..6d97b2453fb --- /dev/null +++ b/.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 diff --git a/src/ActionList/ActionList.docs.json b/src/ActionList/ActionList.docs.json index 392c6db523a..2049f3dc674 100644 --- a/src/ActionList/ActionList.docs.json +++ b/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 @@ ] } ] -} \ No newline at end of file +} diff --git a/src/ActionList/ActionList.test.tsx b/src/ActionList/ActionList.test.tsx index cd2cf3219b9..f7635fd0b50 100644 --- a/src/ActionList/ActionList.test.tsx +++ b/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( + + Heading + + Group Heading + + , + ) + 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( + + Heading + + Group Heading + + , + ) + 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( + + Heading + + Group Heading + + , + ) + 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( + + Heading + + Group Heading + + , + ) + 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( + + Heading + + Group Heading + Item + + , + ) + 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( + + Heading + + Group Heading + Item + + , + ) + 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( + + Heading + + Group Heading + Item + + , + ) + const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`) + const heading = getByText('Group Heading') + expect(list).toHaveAttribute('aria-label', heading.textContent) + }) }) diff --git a/src/ActionList/Group.tsx b/src/ActionList/Group.tsx index f6332760c1f..97a39107306 100644 --- a/src/ActionList/Group.tsx +++ b/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 -export const GroupContext = React.createContext({}) +type ContextProps = Pick & {groupHeadingId: string | undefined} +export const GroupContext = React.createContext({ + groupHeadingId: undefined, + selectionVariant: undefined, +}) export const Group: React.FC> = ({ title, @@ -44,9 +52,25 @@ export const Group: React.FC> = ({ 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 ( > = ({ }} {...props} > - {title &&
} - + + {title && !slots.groupHeading ? ( + + ) : null} + {!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null} ) } -export type HeaderProps = Pick & { - labelId: string -} +export type GroupHeadingProps = Pick & + Omit & + SxProp & + React.HTMLAttributes & { + 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> = ({variant, title, auxiliaryText, labelId, ...props}) => { - const {variant: listVariant} = React.useContext(ListContext) +export const GroupHeading: React.FC> = ({ + 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> = ({variant, title, } return ( - + <> + {listRole ? ( + + ) : ( + <> + (styles, sx)} {...props}> + {title ?? children} + + {auxiliaryText && {auxiliaryText}} + + )} + ) } diff --git a/src/ActionList/index.ts b/src/ActionList/index.ts index 202a96a1a4b..0bc89ae5986 100644 --- a/src/ActionList/index.ts +++ b/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, }) diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index d90f6f1c4c1..eebaf1c1e46 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/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`] = `
  • - + Overview +
      - + Components +