feat(outline): list h4 subsections, and expand at every depth - #228
Conversation
The "On this page" panel picked up h2 and h3 only, so a reader inside an h4 saw its h3 marked with nothing listed below it, and a current h3 never opened to show its h4s. The Sphinx panel does both, and 12 of lecture-python-programming's 28 lecture files have h4 sections. The selector now takes h4 as well. `maxdepth` stays 3: upstream renumbers levels from the shallowest heading on the page and keeps `level < maxdepth + 1`, so with an h2 present an h4 is level 3. The one-level `nest` is replaced by a tree of any depth, rendered recursively, and the expansion rule is the Sphinx scrollspy's applied at every level: the current entry's own sub-list and those of all its ancestors are open, while only the current entry itself is marked. h4 entries take one more indent step, keyed on nesting rather than a depth class so the step applies wherever the tree goes. h5 and deeper are still not listed -- the selector does not take them, so a page whose shallowest heading is an h3 does not start listing them either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
- The count of sub-level entries used a descendant selector, so an h4's anchor matched through its h3 ancestor's li and the assertion held even when the h4's own li lost `qe-outline__sub` -- the class the deeper indent rule keys on. The child combinator makes it fail, as verified by mutating the class expression. - The component's own doc block still described the h3-only behaviour it replaced: h3s nesting under h2s, only a parent expanded, and a marked entry that could only be a section or a subsection. It now describes the tree and the rule as they stand, agreeing with the docstrings on `nest`, `openBranches` and `Branches` below it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
|
🎭 Visual regression results
Details
Skipped testsmobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › without JavaScript › drawer-opens-without-javascript |
There was a problem hiding this comment.
🔵 Needs a closer look
It rewrites the outline navigation logic (recursive tree render, expansion, CSS specificity) that renders on every lecture page, and correctness rests on browser-only Playwright tests that cannot be executed in this review environment.
Pull request overview
This PR widens the "On this page" outline panel to list h4 subsections (previously only h2/h3), matching the Sphinx book-theme panel and closing #208. It rebuilds the outline as a tree of arbitrary depth, applies the Sphinx scrollspy expansion rule at every level (the current entry's own sub-list plus all its ancestors are open, while only the current entry is marked), and adds a deeper indent step for h4 entries.
Changes:
Outline.tsx: selector widened tomain h2, main h3, main h4;nest()rewritten with an ancestor stack to build a tree of any depth; newopenBranches()computes the open set (current + ancestors); new recursiveBranchescomponent replaces the hardcoded two-level render; updated doc block.styles/quantecon.css: added a nesting-keyed indent rule (.qe-outline__sub .qe-outline__sub a) so h4s indent one step further than h3s, using logical padding for RTL.- Tests/docs: fixture gains two h4s; visual tests assert seven entries, per-level indent, child-combinator sub count, and expand/collapse at h3/h4;
docs/layout.mdandCHANGELOG.mdupdated.
File summaries
| File | Description |
|---|---|
| app/components/Outline.tsx | Core logic: widen selector, tree nest(), openBranches(), recursive Branches render, updated doc block |
| styles/quantecon.css | Deeper indent step for h4 entries keyed on nesting depth |
| tests/visual/theme.spec.ts | Asserts 7 entries, per-level indent, expansion/collapse for h3/h4, child-combinator sub count |
| tests/visual/fixture-no-thebe/outline.md | Adds two h4 subsections under the first h3 to exercise depth-3 nesting |
| docs/layout.md | Documents h2–h4 listing, per-level indent, and expand-at-every-depth behavior |
| CHANGELOG.md | Adds an Unreleased "Added" entry describing the outline change |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
#221, #222, #224, #225, #226 and #227 all landed entries under `## [Unreleased]` while this was open. This branch adds one `### Added` entry, so it folds into that list rather than opening a second section, and it gains its PR link beside the issue link, which is what CONTRIBUTING.md:225-227 asks for. `docs/layout.md`, `styles/quantecon.css` and `tests/visual/theme.spec.ts` auto-merged. Each was checked rather than assumed: the branch's h4 indent rule sits beside main's output-image centring rule, the branch's outline assertions beside main's footer, launch, centring and canonical tests, and the branch's h2-to-h4 documentation beside main's default-footer section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#221, #222, #224, #225, #226, #227 and #228 all landed `[Unreleased]` entries while this was open. This branch adds a `### Fixed` entry, so it folds into main's existing Fixed section (output image centring, base-URL head links) rather than opening a second one, and it gains its PR link beside the issue link per CONTRIBUTING.md:225-227. `docs/configuration.md`, `docs/index.md` and `docs/layout.md` auto-merged, and each was checked rather than assumed: layout.md holds this branch's Header and Widths rewrites beside #228's h2-to-h4 outline text and #221's default-footer section; index.md holds #226's Pages-table row beside this branch's rewritten closing paragraph; configuration.md holds the rows from all four PRs that touched its table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #208, implementing the 2026-09-11 decision (h2 to h4 in a tree of any depth; Sphinx expansion at every level;
contents_autoexpand: falseout of scope).Problem, re-verified before the work
Measured against main at
d7ec6cd, the base this branch forked from. Main has since moved to2ac66a378(six PRs landed while this was open) and this branch now carries a merge of it; see "Merged with main" below.useHeaders('main h2, main h3', 3)never selected h4 elements,nest()built one child level, andexpandedIdonly looked among top-level branches — so an h3 could never be the expanded item. On the fixture's/outlinepage that meant five entries where the Sphinx panel shows seven, with nothing under a current h3.Change
main h2, main h3, main h4.maxdepthstays 3 — upstream renumbers levels from the shallowest heading and keepslevel < maxdepth + 1, so with an h2 present an h4 is level 3 (read from@myst-theme/site'suseHeaders, not assumed)nest()now nests each heading under the nearest preceding shallower one, to any depth; a heading with nothing shallower before it starts its own branch, so a page beginning at h3 still rendersopenBranches()opens the current entry's own sub-list and those of all its ancestors — the Sphinx scrollspy's rule, applied at every level. Marking is untouched: only the current entry carriesaria-current.qe-outline__sub .qe-outline__sub a) rather than a depth class, with logical padding so RTL indents from the righth5 and deeper stay unlisted: the selector does not take them, so even a page whose shallowest heading is an h3 does not start listing them.
Tests
tests/visual/fixture-no-thebe/outline.mdgains two h4s under its first h3. The two outline tests now assert seven entries with their own enumerators (1.1.1. A level-four subsection), the h4s staying closed while only their h2 is current, a current h3 opening its h4s, a current h4 marked with its h3 and h2 expanded-but-unmarked, and everything collapsing again on scrolling away.Every new assertion is mutation-tested. Reverting the selector, making
openBranchesadd only the current id, making it add only ancestors, and deleting the indent rule each fail a specific assertion.Adversarial review, and what it found
Four lenses (correctness, rendered behaviour in a browser, accessibility/visual, test quality and docs), each finding refuted by two independent skeptics. 2 findings survived — both fixed in the second commit, both about my own work rather than the feature:
li.qe-outline__sub ais a descendant selector, so an h4's anchor matched through its h3 ancestor's<li>— the count of 4 held even when the h4's own<li>lostqe-outline__sub, which is the class the indent rule keys on. Now a child combinator; I re-ran the reviewer's exact mutation and it fails 4-vs-2 where it previously passed.Everything else was refuted, including the correctness lens's probes at level-jumping pages (h2 → h4 with no h3), pages with no h2 at all, duplicate heading ids and
openBranchesperformance.Not silently capped: findings rated low severity or over the five-per-lens verification cap were not put through refutation.
No pixels move
The rule only adds depth where h4s exist, and no snapshot page has one — no baseline refresh needed,
-darwinincluded.That is worth stating with evidence rather than as a claim, because the fixture this PR adds two h4s to is
/outline. It holds for two independent reasons: the seven baselines per project arefeatures,history-open,intro,lists,notebook,rtlandsidebar-open— there is nooutline.png— and the four tests in theOn this page outlineblock contain notoHaveScreenshotat all, being DOM assertions throughout.Merged with main
#221, #222, #224, #225, #226 and #227 all landed
[Unreleased]entries while this was open. This branch adds a single### Addedentry, so it folds into that list rather than opening a second section, leaving the section ordered Added (footer,{raw}rewriter, migration checklist, canonical link, this outline change), Changed (breaking launch change), Fixed (output image centring, base-URL head links). Merged ate488387ef.docs/layout.md,styles/quantecon.cssandtests/visual/theme.spec.tsauto-merged, and each was verified rather than assumed: the h4 indent rule sits beside main's output-image centring rule, the outline assertions beside main's footer, launch, centring and canonical tests, and the h2-to-h4 documentation beside main's default-footer section.Review
Copilot raised one comment, on the changelog entry linking the issue but not the PR. Addressed in
e488387ef: CONTRIBUTING.md:227 does ask for the PR link, so the entry now carries both. Its supporting claim — that every other entry in the file carries apull/NNNlink — is not accurate, and the reply records the actual figure: ten entries carry only an issue link, across five released versions.The review also noted that correctness here rests on browser tests it could not execute. Those were run locally against a build of the merged branch: 45 passed, 0 failed across the Chromium projects, with all four outline tests passing on
desktop-chromeand the mobile entries skipping by design, since the margin column is desktop-only. That distinction was checked explicitly — a green summary would mean little if the tests for this feature had been skipped rather than run. Two further claims were verified structurally instead of trusted: the diff never touchesEntry, soaria-currentremains keyed onheading.id === currentIdand exactly one entry can be marked; and the class-less h4 indent rule does survive Tailwind's content scan into the built bundle (public/build/_assets/app-*.css).Housekeeping note
I ran this branch in a separate git worktree so the #207 review could keep the main checkout. Two of this review's agents worked in that same worktree concurrently and briefly left it mutated; one of them noticed, redid its verification in a private worktree, and restored the shared one. I verified the worktree was clean at
46d87c5with the correctdepth > 0before making these fixes. Nothing contaminated is in this branch — but if I run parallel reviews again I'll give each agent its own checkout.That disclosure was taken at face value and then checked independently: the branch's diff against its merge-base is exactly the six files described and nothing else,
tscis clean, the unit suite is green, and the browser tests were executed locally (below). Inspection plus execution both agree the branch holds only this work.🤖 Generated with Claude Code
https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
Generated by Claude Code