Skip to content

Commit 6d396f8

Browse files
committed
ui/CollapsibleCard: Default Header to a div wrapper
The right heading level depends on the surrounding document outline, so let consumers opt into heading semantics via render={ <h2 /> } instead of defaulting to h3. Addresses review feedback.
1 parent d8f85a0 commit 6d396f8

6 files changed

Lines changed: 51 additions & 70 deletions

File tree

packages/ui/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
- `Select`: `Select.Trigger` now renders a default `"Select"` placeholder when no value is selected, where it previously rendered empty ([#78076](https://github.com/WordPress/gutenberg/pull/78076)).
1010
- `Select`: `Select.Item` no longer renders its `value` as fallback item content. Pass item content explicitly as `children`. Migrate `<Select.Item value="Foo" />` to `<Select.Item value="Foo">Foo</Select.Item>` ([#77861](https://github.com/WordPress/gutenberg/pull/77861)).
1111
- `Tooltip`: **`Popup` positioner API** ([#78089](https://github.com/WordPress/gutenberg/pull/78089)). Add a `Tooltip.Positioner` subcomponent and an optional `positioner` prop on `Tooltip.Popup` (when omitted, the default `Tooltip.Positioner` is used). Remove `side`, `align`, and `sideOffset` from `Tooltip.Popup`; pass `positioner={ <Tooltip.Positioner side="…" align="…" sideOffset={ … } /> }` instead. The new subcomponent exposes the full positioner surface (`alignOffset`, `anchor`, `collisionAvoidance`, `collisionBoundary`, `collisionPadding`, `sticky`, etc.) and mirrors the existing `portal` slot pattern.
12-
- `CollapsibleCard.Header`: Now renders an `<h3>` element by default (was `<div>`) so each card contributes to the document outline. Pass `render={ <h2 /> }` (or another heading level) to change the level, or `render={ <div /> }` to opt out. Forwarded props (`className`, `style`, `aria-*`, `data-*`) and `ref` now land on the outer heading wrapper instead of the inner click-target div; `ref` widens from `HTMLDivElement` to `HTMLHeadingElement` ([#77962](https://github.com/WordPress/gutenberg/pull/77962)).
12+
- `CollapsibleCard.Header`: Now renders an outer `<div>` wrapper around the trigger. Forwarded props (`className`, `aria-*`, `data-*`) and `ref` land on this outer wrapper instead of the inner click-target div ([#77962](https://github.com/WordPress/gutenberg/pull/77962)).
1313

1414
### New Features
1515

@@ -29,7 +29,7 @@
2929
- `Drawer`: Fade the popup elevation shadow alongside the slide ([#77800](https://github.com/WordPress/gutenberg/pull/77800)).
3030
- `Drawer`: Allow mouse-drag swipe-dismiss in the popup-edge padding gutter ([#77800](https://github.com/WordPress/gutenberg/pull/77800)).
3131
- `IconButton`: Add a `positioner` prop, accepting a `<Tooltip.Positioner />` element, to customize how the tooltip is positioned relative to the button ([#78089](https://github.com/WordPress/gutenberg/pull/78089)).
32-
- `CollapsibleCard`: Each card now contributes a heading to the document outline by default, following the W3C APG accordion pattern (heading wraps button) ([#77962](https://github.com/WordPress/gutenberg/pull/77962)).
32+
- `CollapsibleCard.Header`: Pass `render={ <h2 /> }` (or any of `<h1>``<h6>`) to wrap the trigger in a heading and contribute to the document outline, following the W3C APG accordion pattern (heading wraps button) ([#77962](https://github.com/WordPress/gutenberg/pull/77962)).
3333

3434
### Internal
3535

packages/ui/src/collapsible-card/header.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,19 @@ import type { HeaderProps } from './types';
1616
* toggle trigger — clicking anywhere on it expands or collapses the
1717
* card's content.
1818
*
19-
* Renders an `<h3>` element by default so each card contributes to the
20-
* document outline. Pass `render` to change the heading level (e.g.
21-
* `render={ <h2 /> }`) or to opt out of heading semantics entirely
22-
* (`render={ <div /> }`) when no outline contribution is desired.
19+
* Defaults to a `<div>` wrapper around the trigger. Since the right heading
20+
* level depends on the surrounding document outline, the consumer is
21+
* expected to opt in to heading semantics. Pass `render` to wrap the
22+
* trigger in a heading (e.g. `render={ <h2 /> }`), following the W3C APG
23+
* accordion pattern (heading wraps button).
2324
*
2425
* Avoid placing interactive elements (buttons, links, inputs) inside the
2526
* header, since the entire area is clickable and their events will bubble
2627
* to trigger the collapse toggle.
2728
*/
28-
export const Header = forwardRef< HTMLHeadingElement, HeaderProps >(
29+
export const Header = forwardRef< HTMLDivElement, HeaderProps >(
2930
function CollapsibleCardHeader(
30-
{ children, className, style, render, ...restProps },
31+
{ children, className, render, ...restProps },
3132
ref
3233
) {
3334
const [ descriptionId, setDescriptionId ] = useState< string >();
@@ -38,16 +39,15 @@ export const Header = forwardRef< HTMLHeadingElement, HeaderProps >(
3839
);
3940

4041
return useRender( {
41-
defaultTagName: 'h3',
42+
defaultTagName: 'div',
4243
render,
4344
ref,
44-
props: mergeProps< 'h3' >( restProps, {
45+
props: mergeProps< 'div' >( restProps, {
4546
className: clsx(
4647
defenseStyles.heading,
4748
styles[ 'heading-wrapper' ],
4849
className
4950
),
50-
style,
5151
children: (
5252
<HeaderDescriptionIdContext.Provider value={ contextValue }>
5353
<Collapsible.Trigger

packages/ui/src/collapsible-card/stories/index.story.tsx

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,11 @@ export const Stacked: Story = {
166166
};
167167

168168
/**
169-
* `CollapsibleCard.Header` renders an `<h3>` by default so the card
170-
* contributes to the document outline. Use the `render` prop to change the
171-
* heading level (e.g. `render={ <h2 /> }`) or to opt out of heading
172-
* semantics entirely (`render={ <div /> }`).
169+
* `CollapsibleCard.Header` renders a `<div>` wrapper by default. Pass an
170+
* `<h1>`–`<h6>` React element to the `render` prop to wrap the trigger in
171+
* a heading and contribute to the document outline. The right level
172+
* depends on the surrounding outline, so the consumer is expected to opt
173+
* in.
173174
*/
174175
export const WithHeadingElement: Story = {
175176
parameters: { controls: { disable: true } },
@@ -193,24 +194,25 @@ export const WithHeadingElement: Story = {
193194
</CollapsibleCard.Content>
194195
</CollapsibleCard.Root>
195196
<CollapsibleCard.Root>
196-
<CollapsibleCard.Header>
197-
<Card.Title>Heading level 3 (default)</Card.Title>
197+
<CollapsibleCard.Header render={ <h3 /> }>
198+
<Card.Title>Heading level 3</Card.Title>
198199
</CollapsibleCard.Header>
199200
<CollapsibleCard.Content>
200201
<Text>
201-
Without a render prop, the header defaults to an h3
202-
element.
202+
Pass any of h1–h6 to choose the level that fits the
203+
surrounding document outline.
203204
</Text>
204205
</CollapsibleCard.Content>
205206
</CollapsibleCard.Root>
206207
<CollapsibleCard.Root>
207-
<CollapsibleCard.Header render={ <div /> }>
208-
<Card.Title>No heading semantics</Card.Title>
208+
<CollapsibleCard.Header>
209+
<Card.Title>No heading (default)</Card.Title>
209210
</CollapsibleCard.Header>
210211
<CollapsibleCard.Content>
211212
<Text>
212-
Passing a div React element to the render prop opts out
213-
of heading semantics entirely.
213+
Without a render prop, the header wraps the trigger in a
214+
plain div and does not contribute to the document
215+
outline.
214216
</Text>
215217
</CollapsibleCard.Content>
216218
</CollapsibleCard.Root>

packages/ui/src/collapsible-card/style.module.css

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
@layer wp-ui-components {
44
/*
5-
* Style overrides for the outer heading wrapper rendered by
6-
* `CollapsibleCard.Header` (defaults to `<h3>`). Combined with
7-
* `defenseStyles.heading` so the unlayered defense class beats wp-admin's
8-
* bare `h2`/`h3`/etc. selectors. Keeps the wrapper visually transparent so
9-
* UA and host stylesheets don't bleed into the inner trigger.
5+
* Style overrides for the outer wrapper rendered by `CollapsibleCard.Header`
6+
* (defaults to `<div>`, but consumers can opt into a heading level via
7+
* `render={ <h2 /> }` etc.). Combined with `defenseStyles.heading` so that,
8+
* when the wrapper renders as an h1–h6, the unlayered defense class beats
9+
* wp-admin's bare `h2`/`h3`/etc. selectors. Keeps the wrapper visually
10+
* transparent so UA and host stylesheets don't bleed into the inner
11+
* trigger.
1012
*/
1113
.heading-wrapper {
1214
--_gcd-heading-color: inherit;

packages/ui/src/collapsible-card/test/index.test.tsx

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe( 'CollapsibleCard', () => {
88
describe( 'basic behaviour', () => {
99
it( 'forwards ref', () => {
1010
const rootRef = createRef< HTMLDivElement >();
11-
const headerRef = createRef< HTMLHeadingElement >();
11+
const headerRef = createRef< HTMLDivElement >();
1212
const contentRef = createRef< HTMLDivElement >();
1313

1414
render(
@@ -23,7 +23,7 @@ describe( 'CollapsibleCard', () => {
2323
);
2424

2525
expect( rootRef.current ).toBeInstanceOf( HTMLDivElement );
26-
expect( headerRef.current ).toBeInstanceOf( HTMLHeadingElement );
26+
expect( headerRef.current ).toBeInstanceOf( HTMLDivElement );
2727
expect( contentRef.current ).toBeInstanceOf( HTMLDivElement );
2828
} );
2929

@@ -179,8 +179,8 @@ describe( 'CollapsibleCard', () => {
179179
} );
180180
} );
181181

182-
describe( 'heading wrapper', () => {
183-
it( 'renders an `<h3>` heading wrapping the trigger by default', () => {
182+
describe( 'header wrapper', () => {
183+
it( 'does not contribute a heading to the document outline by default', () => {
184184
render(
185185
<CollapsibleCard.Root>
186186
<CollapsibleCard.Header>
@@ -189,17 +189,15 @@ describe( 'CollapsibleCard', () => {
189189
</CollapsibleCard.Root>
190190
);
191191

192-
const heading = screen.getByRole( 'heading', {
193-
level: 3,
194-
name: 'Title',
195-
} );
196-
expect( heading ).toBeVisible();
197192
expect(
198-
within( heading ).getByRole( 'button', { name: 'Title' } )
193+
screen.queryByRole( 'heading', { name: 'Title' } )
194+
).not.toBeInTheDocument();
195+
expect(
196+
screen.getByRole( 'button', { name: 'Title' } )
199197
).toBeVisible();
200198
} );
201199

202-
it( 'renders the heading at a different level via `render`', () => {
200+
it( 'wraps the trigger in a heading via `render`', () => {
203201
render(
204202
<CollapsibleCard.Root>
205203
<CollapsibleCard.Header render={ <h2 /> }>
@@ -218,41 +216,25 @@ describe( 'CollapsibleCard', () => {
218216
).toBeVisible();
219217
} );
220218

221-
it( 'opts out of heading semantics with `render={ <div /> }`', () => {
222-
render(
223-
<CollapsibleCard.Root>
224-
<CollapsibleCard.Header render={ <div /> }>
225-
<Card.Title>Title</Card.Title>
226-
</CollapsibleCard.Header>
227-
</CollapsibleCard.Root>
228-
);
229-
230-
expect(
231-
screen.queryByRole( 'heading', { name: 'Title' } )
232-
).not.toBeInTheDocument();
233-
expect(
234-
screen.getByRole( 'button', { name: 'Title' } )
235-
).toBeVisible();
236-
} );
237-
238-
it( 'forwards `className` to the outer heading wrapper', () => {
219+
it( 'forwards `className` and other props to the outer wrapper', () => {
239220
render(
240221
<CollapsibleCard.Root>
241222
<CollapsibleCard.Header
242-
className="custom-heading"
223+
className="custom-header"
243224
data-testid="header"
244225
>
245226
<Card.Title>Title</Card.Title>
246227
</CollapsibleCard.Header>
247228
</CollapsibleCard.Root>
248229
);
249230

250-
const heading = screen.getByRole( 'heading', {
251-
level: 3,
252-
name: 'Title',
253-
} );
254-
expect( heading ).toHaveClass( 'custom-heading' );
255-
expect( heading ).toHaveAttribute( 'data-testid', 'header' );
231+
const wrapper = screen.getByTestId( 'header' );
232+
expect( wrapper ).toHaveClass( 'custom-header' );
233+
// The forwarded attributes land on the outer wrapper, not the
234+
// inner button trigger.
235+
expect(
236+
within( wrapper ).getByRole( 'button', { name: 'Title' } )
237+
).not.toHaveAttribute( 'data-testid' );
256238
} );
257239
} );
258240

@@ -282,11 +264,6 @@ describe( 'CollapsibleCard', () => {
282264
'aria-describedby',
283265
descriptionElement.id
284266
);
285-
286-
// aria-describedby must be on the inner button (which is what's
287-
// being described), not on the outer heading wrapper.
288-
const heading = screen.getByRole( 'heading', { name: 'Settings' } );
289-
expect( heading ).not.toHaveAttribute( 'aria-describedby' );
290267
} );
291268

292269
it( 'marks the description content as aria-hidden', () => {

packages/ui/src/collapsible-card/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface RootProps extends ComponentProps< 'div' > {
3030
disabled?: boolean;
3131
}
3232

33-
export interface HeaderProps extends ComponentProps< 'h3' > {
33+
export interface HeaderProps extends ComponentProps< 'div' > {
3434
/**
3535
* The content to be rendered inside the header.
3636
*/

0 commit comments

Comments
 (0)