Skip to content

Avoid headings in native details demo and docs (#1160) - #1187

Open
chanellebingham wants to merge 2 commits into
mozilla:mainfrom
chanellebingham:fix-1160-details-no-headings
Open

chanellebingham wants to merge 2 commits into
mozilla:mainfrom
chanellebingham:fix-1160-details-no-headings

Conversation

@chanellebingham

@chanellebingham chanellebingham commented Oct 21, 2026 •

Copy link
Copy Markdown

Description

Fixes #1160.

<summary> has no ARIA role of its own and some screen readers expose it as a button, so a heading placed inside it drops out of heading navigation (background in the issue). The native <details> demo put an <h3> inside each <summary>, and its readme told people they could combine the two.

What changed:

  • components/00-basic-elements/details/details.html: plain-text summaries. The <h4> inside the second details body is gone too, so the demo matches the rule "native <details> only for content without headings".
  • components/00-basic-elements/details/readme.md: the "combine with headings" sentence is replaced with the rule, the reason, and a pointer to the Details component for content with headings.
  • components/details-component/readme.md: one sentence saying to use it instead of the native element when the content has headings.
  • CHANGELOG.md: entries under HEAD > Accessibility and HEAD > Component changes > Details.
  • assets/sass/protocol/base/elements/_details.scss (from review): native <summary> is bold by default, native <details> has margin: 0 0 1.25em, and details[open] > summary has margin-bottom: 0.5em. A closed details doesn't collapse the summary's margin into its own, so that margin only applies while it's open.
  • The demo summaries use mzp-u-heading-xs (from review).

No JS changes. The first commit changed no Sass either. The summary weight and spacing in the second commit came from review (open question 2).

  • I have documented this change in the design system.
  • I have recorded this change in CHANGELOG.md.

Issue

#1160

Testing

The docs were built from main (715e175) and from this branch, and headings were counted in headless Chrome on components/preview/details.html and components/render/details.html:

  • document.querySelectorAll('summary :is(h1,h2,h3,h4,h5,h6)').length: 2 before, 0 after.
  • document.querySelectorAll('details :is(h1,h2,h3,h4,h5,h6)').length: 3 before, 0 after.
  • Both before and after have 2 <summary> elements ("Summary One", "Summary Two"), so the zero is not a page that failed to load.

Run on this branch with Node 24:

  • npm run build-package (what CI runs first: npm ci, eslint, stylelint, webpack): exit 0.
  • npm run lint and npm run test-build: exit 0. Then npx jasmine-browser-runner runSpecs --browser=headlessChrome --config=tests/jasmine-browser.json: 47 specs, 0 failures.
  • npm run build-docs: 524 of 524 items exported, no errors.
  • Second commit (review changes), computed styles on the built demo in headless Chromium: a plain summary has weight 700 and a bottom margin of 0px when closed and 8px when open. Each details has a 20px bottom margin. With mzp-u-heading-xs, the open summary's margin is 14px. With the CSS from main, the same check gives 400 and 0px. npm run lint, npm run build-package and npm run build-docs (524 of 524) pass.

Not run locally: the Firefox half of npm test (no Firefox on the machine used) and the other Node versions in the CI matrix. The change touches no JS.

Open questions

  1. This also removes the <h4>Sub Heading</h4> from the demo body, not only the <h3>s in the summaries, reading the rule in the issue ("use <details> only for content where headings are not used") as covering body headings too. If you'd rather keep a body heading in the demo, it's a one-line revert.
  2. Resolved in review: summary is bold by default, the demo uses mzp-u-heading-xs, and native details and summary have default margins (second commit).
  3. The screen-reader behaviour comes from the issue (JAWS leaves the headings out of heading navigation; NVDA does not). It has not been retested with a screen reader for this PR.

Made with my AI assistant, a setup I've built on Anthropic's Claude over months to read everything and prove its fixes with tests. It wrote and tested the change; I reviewed it before opening.

Plain-text summaries in the <details> demo; readme now says to use native
<details> only for content without headings and the Details component when
headings are present. CHANGELOG entry under Accessibility.
@stevejalim
stevejalim requested review from knowler and stephaniehobson and removed request for stephaniehobson October 21, 2026 11:24
@stephaniehobson stephaniehobson self-assigned this Oct 27, 2026
@stephaniehobson

Copy link
Copy Markdown
Contributor

Hi @chanellebingham, thank you for the contribution. Open question #2 needs to be addressed.

Please:

  • make the summary element bold by default
  • add the class mzp-u-heading-xs to the summary in the examples
  • declare default margins on the summary (0.5em) and details (1.25em) elements
    • unfortunately the margins don't collapse when it's closed so the summary margin should only display when the details are open.

Following review: native <summary> is bold by default, native <details>
has a 1.25em bottom margin, and the summary of an open <details> has a
0.5em bottom margin. A closed details doesn't collapse the summary's
margin into its own, so that margin only applies while it's open. The
demo summaries use mzp-u-heading-xs. CHANGELOG entry under Details.
@chanellebingham

Copy link
Copy Markdown
Author

Thanks @stephaniehobson! Done in a2cdabc:

  • summary is bold by default.
  • The demo summaries have mzp-u-heading-xs.
  • details has margin: 0 0 1.25em, and details[open] > summary has margin-bottom: 0.5em, so the summary's margin only shows while the details are open.

Checked on the built demo in headless Chromium:

  • A plain summary is bold (700), with 0px below it when closed and 8px when open, and each details has 20px below it.
  • With mzp-u-heading-xs, the summary's 0.5em comes out at 14px, since em follows the summary's own font size.
  • With the CSS from main, the same check gives a weight of 400 and 0px everywhere.

npm run lint, npm run build-package and npm run build-docs pass. I've updated the CHANGELOG (a new "Details" entry) and the PR description to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid <details>/<summary> for content with headings

2 participants