Move `<summary>` to top of `<details>` for validity #3423

andersk · 2025-09-11T21:32:51Z

The <summary> element must be the first child of <details>.

changeset-bot · 2025-09-11T21:32:54Z

🦋 Changeset detected

Latest commit: a127aac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@astrojs/starlight	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify · 2025-09-11T21:32:56Z

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	`a127aac`
🔍 Latest deploy log	https://app.netlify.com/projects/astro-starlight/deploys/68cbfc7caabcba00081428f8
😎 Deploy Preview	https://deploy-preview-3423--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (🟢 up 10 from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

delucis

Thanks for the PR @andersk! Just for context, do you have some examples of where this was causing issues?

andersk · 2025-09-13T22:49:06Z

I’m just trying to get my Starlight output to pass an HTML validator. (Example.) Of course I’m aware that not all validation errors are problems in the real world, so I’m filtering out a mix of validator bugs, legitimate nonstandard extensions, and browser bug workarounds to find the real issues. This case clearly does violate the standard, so even if it doesn’t cause problems in practice with today’s common browsers, we might as well fix it to make it easier for real issues to stand out.

delucis

Thanks for the extra context @andersk!

I was hesitant just because the sidebar restoration logic here is a little subtle, but I tested it again and I think this change is safe.

Ideally, we do want the restore point as early as possible in the DOM, so I’ve suggested an alternative refactor. What do you think? Both work in practice, just thinking what’s the most minimal change in terms of execution order (the <SidebarRestorePoint> component renders a custom element which controls the collapsed state of the parent <details>)

delucis · 2025-09-15T10:04:34Z

packages/starlight/components/SidebarSublist.astro

 					>
-						<SidebarRestorePoint />
 						<summary>
 							<div class="group-label">


I wonder if another option here would be to move the restore point inside the <summary> so it’s still as early as possible in the tree:

Suggested change

<div class="group-label">

<SidebarRestorePoint />

<div class="group-label">

delucis · 2025-09-15T10:04:47Z

packages/starlight/components/SidebarSublist.astro

 							</div>
 							<Icon name="right-caret" class="caret" size="1.25rem" />
 						</summary>
+						<SidebarRestorePoint />


And then remove it here:

Suggested change

<SidebarRestorePoint />

andersk · 2025-09-15T10:30:44Z

Yeah that seems fine to me: this.closest('details') in SidebarPersister will still be able to find <details>. Updated.

Signed-off-by: Anders Kaseorg <[email protected]>

delucis

Great — thanks again @andersk 🙌

delucis · 2025-09-15T12:18:02Z

Oops, I should have tested this before suggesting it 😅

Might be best to go with your original suggestion after all 👍 (Or introduce styles, but that seems overkill?)

delucis

Thanks again @andersk — I restored your initial suggestion given the layout issues with mine.

* main: (26 commits) i18n(ja): resources/themes (withastro#3442) add openstatus showcase (withastro#3436) [ci] release (withastro#3434) Add `head` config validation for `meta` tags with `content` (withastro#3380) Fix Astro Island margins in Markdown (withastro#3340) Ensure tab links span the full tab height (withastro#3427) Move `<summary>` to top of `<details>` for validity (withastro#3423) Run all tests on CI workflow changes (withastro#3433) [ci] format i18n(fr): small rewording and fixes in top-level pages (withastro#3432) i18n(fr): small rewording and fixes in `guides` (withastro#3431) i18n(fr): fix typo and links in `reference` files (withastro#3429) i18n(fr): small rewording in `components` (withastro#3430) [ci] release (withastro#3384) i18n(de): Update `page.lastUpdated` translation to be more in‐line with the English translation (withastro#3416) Remove invalid value attribute from `<select>` (withastro#3421) Convert invalid `<div>` child of `<summary>` to `<span>` (withastro#3422) [i18nIgnore] Fix typo in Spanish docs documentation (withastro#3408) chore(deps): update actions/labeler action to v6.0.1 (withastro#3407) [i18nIgnore] Fix duplicate i18n collection definition (withastro#3409) ...

* main: (72 commits) Showcase: Add Saucer (withastro#3454) i18n(ja): resources/themes (withastro#3442) add openstatus showcase (withastro#3436) [ci] release (withastro#3434) Add `head` config validation for `meta` tags with `content` (withastro#3380) Fix Astro Island margins in Markdown (withastro#3340) Ensure tab links span the full tab height (withastro#3427) Move `<summary>` to top of `<details>` for validity (withastro#3423) Run all tests on CI workflow changes (withastro#3433) [ci] format i18n(fr): small rewording and fixes in top-level pages (withastro#3432) i18n(fr): small rewording and fixes in `guides` (withastro#3431) i18n(fr): fix typo and links in `reference` files (withastro#3429) i18n(fr): small rewording in `components` (withastro#3430) [ci] release (withastro#3384) i18n(de): Update `page.lastUpdated` translation to be more in‐line with the English translation (withastro#3416) Remove invalid value attribute from `<select>` (withastro#3421) Convert invalid `<div>` child of `<summary>` to `<span>` (withastro#3422) [i18nIgnore] Fix typo in Spanish docs documentation (withastro#3408) chore(deps): update actions/labeler action to v6.0.1 (withastro#3407) ...

* main: (87 commits) i18n(ja): sync outdated aside page (withastro#3465) i18n(ja): sync outdated icon reference (withastro#3467) chore(deps): update pnpm/action-setup action to v4.2.0 (withastro#3460) Showcase: Add Saucer (withastro#3454) i18n(ja): resources/themes (withastro#3442) add openstatus showcase (withastro#3436) [ci] release (withastro#3434) Add `head` config validation for `meta` tags with `content` (withastro#3380) Fix Astro Island margins in Markdown (withastro#3340) Ensure tab links span the full tab height (withastro#3427) Move `<summary>` to top of `<details>` for validity (withastro#3423) Run all tests on CI workflow changes (withastro#3433) [ci] format i18n(fr): small rewording and fixes in top-level pages (withastro#3432) i18n(fr): small rewording and fixes in `guides` (withastro#3431) i18n(fr): fix typo and links in `reference` files (withastro#3429) i18n(fr): small rewording in `components` (withastro#3430) [ci] release (withastro#3384) i18n(de): Update `page.lastUpdated` translation to be more in‐line with the English translation (withastro#3416) Remove invalid value attribute from `<select>` (withastro#3421) ...

github-actions bot added the 🌟 core Changes to Starlight’s main package label Sep 11, 2025

delucis reviewed Sep 13, 2025

View reviewed changes

delucis reviewed Sep 15, 2025

View reviewed changes

andersk force-pushed the details-summary branch from 9a2ab1f to a18d8c5 Compare September 15, 2025 10:30

Move <summary> to top of <details> for validity

d7964a9

Signed-off-by: Anders Kaseorg <[email protected]>

andersk force-pushed the details-summary branch from a18d8c5 to d7964a9 Compare September 15, 2025 10:31

Tweak changeset

c14d654

delucis approved these changes Sep 15, 2025

View reviewed changes

delucis added the 🌟 patch Change that triggers a patch release label Sep 15, 2025

delucis added 2 commits September 18, 2025 14:30

Move restore point back down below the <summary>

3075d7a

Tweak changeset (agan!)

a127aac

delucis approved these changes Sep 18, 2025

View reviewed changes

delucis merged commit a0d0670 into withastro:main Sep 18, 2025
16 checks passed

astrobot-houston mentioned this pull request Sep 18, 2025

[ci] release #3434

Merged

	<div class="group-label">
	<SidebarRestorePoint />
	<div class="group-label">

Uh oh!

Move <summary> to top of <details> for validity #3423

Move <summary> to top of <details> for validity #3423

Uh oh!

Conversation

andersk commented Sep 11, 2025

Uh oh!

changeset-bot bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

netlify bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for astro-starlight ready!

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

andersk commented Sep 13, 2025

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

delucis Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

delucis Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

andersk commented Sep 15, 2025

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

delucis commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Move `<summary>` to top of `<details>` for validity #3423

Move `<summary>` to top of `<details>` for validity #3423

changeset-bot bot commented Sep 11, 2025 •

edited

Loading

netlify bot commented Sep 11, 2025 •

edited

Loading

delucis commented Sep 15, 2025 •

edited

Loading