From 417a757a01886927b3b045fd6ef3990e5ea447dc Mon Sep 17 00:00:00 2001 From: JakeSCahill Date: Fri, 18 Sep 2026 16:22:03 +0100 Subject: [PATCH 1/2] Fix the wrong "On this page" entry on an anchor jump, and tables squeezed by a code block Two things the property reference pages surfaced. Both were pre-existing; the property links work just made in-page jumps common enough to notice. **The wrong TOC entry.** An anchor jump was offset twice: html carries scroll-padding-top, and the headings carried scroll-margin-top on top of it. The browser parks a linked heading at the sum, while the scroll-spy's activation line is scroll-padding-top alone, so the heading landed below the line and the pass picked the heading above it. Measured on a real page: parked at 180px, activation line at 104px, so clicking a link to rpc_server highlighted recovery_mode_enabled. Fixed at the root: one offset, on html, with the headings' 16/24px of breathing room folded into it. The browser and the scroll-spy now read the same value, and a linked heading parks level with the line (119px against 120px). Verified in a browser on the built pages: four targets, all correct after the change, all wrong before it. Also made hash navigation authoritative rather than inferred. onScroll cannot see an in-page click at all, because a hash change does not always move the scroll position enough to fire a scroll event, so hashchange only ever opened the collapsible group and never moved the highlight. It now activates from the hash, which also keeps this correct if the offsets ever drift apart again. Pulled the copy-pasted clear/add dance out of three call sites into setActive(). **Tables squeezed by a code block.** white-space: pre-wrap was already there, marked NEEDS REVIEW, and is not enough: it wraps between tokens but cannot break inside one, so a single long identifier still sets the cell's min-content width. `cloud_storage_inventory_hash_path_directory:` is 44 characters and did exactly that, taking the table to 542px inside a 354px phone and crushing the label column to 95px. overflow-wrap: anywhere is the keyword that lowers min-content, so the token breaks only when there is genuinely no room; break-word would not have helped. Measured with the real markup: 354px and no overflow, label column back to 118px. Two of the toc-collapsible tests asserted the old behaviour on purpose, with the message "the scroll pass picks the heading above". They now assert the target is active and the heading above it is not; their group assertions are unchanged. --- package.json | 5 +- src/css/doc-bump.css | 3 + src/css/doc.css | 43 +-- src/js/02-on-this-page.js | 112 ++++---- .../toc-active-anchor.test.js | 265 ++++++++++++++++++ tests/toc-collapsible/toc-collapsible.test.js | 24 +- 6 files changed, 372 insertions(+), 80 deletions(-) create mode 100644 tests/toc-active-anchor/toc-active-anchor.test.js diff --git a/package.json b/package.json index 9716cda2..f5644b04 100644 --- a/package.json +++ b/package.json @@ -95,10 +95,11 @@ "test:nav-lazy-buckets": "node --test tests/nav-lazy-buckets/*.test.js", "test:has-code-blocks": "node --test tests/has-code-blocks/*.test.js", "test:docs-page-weight": "node --test tests/docs-page-weight/*.test.js", - "test:all": "npm run test:playground && npm run test:interactive && npm run test:negative-cache && npm run test:head-meta && npm run test:property-tooltips && npm run test:signin-nudge && npm run test:chat-panel-navigation && npm run test:toc-collapsible && npm run test:nav-scroll && npm run test:kapa-source-groups && npm run test:tooltip-touch && npm run test:tooltip-single-open && npm run test:lazy-askai && npm run test:nav-lazy-buckets && npm run test:has-code-blocks && npm run test:docs-page-weight && npm run test:anon-quota", + "test:all": "npm run test:playground && npm run test:interactive && npm run test:negative-cache && npm run test:head-meta && npm run test:property-tooltips && npm run test:signin-nudge && npm run test:chat-panel-navigation && npm run test:toc-collapsible && npm run test:toc-active-anchor && npm run test:nav-scroll && npm run test:kapa-source-groups && npm run test:tooltip-touch && npm run test:tooltip-single-open && npm run test:lazy-askai && npm run test:nav-lazy-buckets && npm run test:has-code-blocks && npm run test:docs-page-weight && npm run test:anon-quota", "build:wasm": "cd blobl-editor/wasm && GOOS=js GOARCH=wasm go build -o ../../src/static/blobl.wasm .", "copy:wasm-exec": "cp \"$(go env GOROOT)/lib/wasm/wasm_exec.js\" src/js/vendor/", - "serve:playground": "npx serve ." + "serve:playground": "npx serve .", + "test:toc-active-anchor": "node --test tests/toc-active-anchor/*.test.js" }, "dependencies": { "@kapaai/agent-react": "^1.0.1", diff --git a/src/css/doc-bump.css b/src/css/doc-bump.css index d15eb50a..5e14ef54 100644 --- a/src/css/doc-bump.css +++ b/src/css/doc-bump.css @@ -777,9 +777,12 @@ html[data-theme=dark] { margin-top: 0; } +/* Mirrors the doc.css rule; see the note there for why anywhere and not break-word. */ .aa-DetachedOverlay .doc .tableblock pre, +.aa-DetachedOverlay .doc .tableblock pre code, .aa-DetachedOverlay .doc .listingblock.wrap pre { white-space: pre-wrap; + overflow-wrap: anywhere; } .aa-DetachedOverlay .doc div.no-wrap pre code { diff --git a/src/css/doc.css b/src/css/doc.css index 5c12248e..a648ae26 100644 --- a/src/css/doc.css +++ b/src/css/doc.css @@ -1,5 +1,10 @@ html { - scroll-padding-top: calc(var(--navbar-height) + var(--toolbar-height)); /* 114px */ + /* The one place an anchor jump is offset. The "On this page" scroll-spy reads + this same value as its activation line (see src/js/02-on-this-page.js), so + any second offset elsewhere puts a linked heading below the line and + highlights the entry above it. The trailing 16px is the breathing room that + used to live on the headings as scroll-margin-top. */ + scroll-padding-top: calc(var(--navbar-height) + var(--toolbar-height) + 16px); } .doc { @@ -26,7 +31,7 @@ html { @media screen and (min-width: 1024px) { html { - scroll-padding-top: calc(var(--navbar-height) + 32px); /* 88px */ + scroll-padding-top: calc(var(--navbar-height) + 32px + 24px); } .doc { @@ -59,24 +64,11 @@ html { font-weight: var(--heading-font-weight); } -/* Offset anchor links to account for sticky bar (navbar + component indicator) */ -.doc h2[id], -.doc h3[id], -.doc h4[id], -.doc h5[id], -.doc h6[id] { - scroll-margin-top: calc(var(--navbar-height) + 16px); -} - -@media screen and (min-width: 1024px) { - .doc h2[id], - .doc h3[id], - .doc h4[id], - .doc h5[id], - .doc h6[id] { - scroll-margin-top: calc(var(--navbar-height) + 24px); - } -} +/* Anchor-link offset lives on html as scroll-padding-top, not here. A + scroll-margin-top on the headings stacked on top of it: the browser parks a + linked heading at the sum of the two, which is below the scroll-spy's + activation line, so clicking an in-page link highlighted the preceding + entry. See tests/toc-active-anchor. */ .doc h1 { font-size: calc(36 / var(--rem-base) * 1rem); @@ -1713,10 +1705,19 @@ details[open] > summary { display: block; } -/* NEEDS REVIEW prevent pre in table from causing article to exceed bounds */ +/* A code block in a table cell must not set the table's width. + pre-wrap alone was not enough: it wraps between tokens but cannot break + inside one, so a single long identifier still sets the cell's min-content + width and the table outgrows the page. The property reference pages hit this + with `cloud_storage_inventory_hash_path_directory:` at 44 characters, which + is wider than a phone on its own and squeezed the label column to nothing. + overflow-wrap: anywhere is the keyword that lowers min-content, so the token + breaks only when there is genuinely no room; break-word does not. */ .doc .tableblock pre, +.doc .tableblock pre code, .doc .listingblock.wrap pre { white-space: pre-wrap; + overflow-wrap: anywhere; } .doc pre.highlight code, diff --git a/src/js/02-on-this-page.js b/src/js/02-on-this-page.js index 0d9a3bc3..f1ce87b2 100644 --- a/src/js/02-on-this-page.js +++ b/src/js/02-on-this-page.js @@ -69,27 +69,44 @@ if (collapsible) buildCollapsibleGroups(list) + /** + * Make one fragment the active entry, clearing whatever was active before. + * + * The scroll pass can leave several entries active at the bottom of a page, + * so the previous value is either a fragment or an array of them. + * + * @param {string} fragment - The '#id' to activate. Ignored if no entry has it. + */ + function setActive (fragment) { + if (!links[fragment]) return + if (lastActiveFragment && lastActiveFragment !== fragment) { + var previous = Array.isArray(lastActiveFragment) ? lastActiveFragment : [lastActiveFragment] + previous.forEach(function (f) { + if (links[f]) links[f].classList.remove('is-active') + }) + } + links[fragment].classList.add('is-active') + revealGroup(links[fragment]) + lastActiveFragment = fragment + } + + /** + * Hold off the scroll pass for a moment, so it cannot overwrite an activation + * the user just caused. The browser is still settling on the target when a + * click or a hash change lands. + */ + function holdScrollUpdates () { + skipScrollUpdate = true + setTimeout(function () { + skipScrollUpdate = false + }, 100) + } + // Add click handlers to TOC links to immediately highlight clicked item Object.keys(links).forEach(function (fragment) { links[fragment].addEventListener('click', function () { - // Immediately update active state on click - if (lastActiveFragment && lastActiveFragment !== fragment) { - if (Array.isArray(lastActiveFragment)) { - lastActiveFragment.forEach(function (f) { - links[f].classList.remove('is-active') - }) - } else { - links[lastActiveFragment].classList.remove('is-active') - } - } - links[fragment].classList.add('is-active') - revealGroup(links[fragment]) - lastActiveFragment = fragment - // Skip scroll-based updates briefly to prevent flicker during scroll animation - skipScrollUpdate = true - setTimeout(function () { - skipScrollUpdate = false - }, 100) + setActive(fragment) + holdScrollUpdates() }) }) @@ -144,22 +161,8 @@ // Update sidebar TOC active state immediately var fragment = link.getAttribute('href') if (fragment && links[fragment]) { - if (lastActiveFragment && lastActiveFragment !== fragment) { - if (Array.isArray(lastActiveFragment)) { - lastActiveFragment.forEach(function (f) { - links[f].classList.remove('is-active') - }) - } else { - links[lastActiveFragment].classList.remove('is-active') - } - } - links[fragment].classList.add('is-active') - revealGroup(links[fragment]) - lastActiveFragment = fragment - skipScrollUpdate = true - setTimeout(function () { - skipScrollUpdate = false - }, 100) + setActive(fragment) + holdScrollUpdates() } }) }) @@ -170,10 +173,12 @@ window.addEventListener('load', function () { onScroll() - revealHashTarget() + syncToHashTarget() window.addEventListener('scroll', onScroll, { passive: true }) - // In-page links and back/forward change the hash without a load, so reveal on those too - window.addEventListener('hashchange', revealHashTarget) + // In-page links and back/forward change the hash without a load, so the + // highlight has to follow on those too. onScroll alone cannot: it runs on + // scroll, and a hash change does not always produce one. + window.addEventListener('hashchange', syncToHashTarget) // On initial load, scroll active item into view (e.g., when navigating to a hash) scrollActiveIntoView() }) @@ -323,24 +328,37 @@ } /** - * Open the group that holds the entry the URL hash points at, on load and on every hash change. - * Browsers park a linked heading at scroll-padding-top + scroll-margin-top, below the activation - * line onScroll uses, so the heading above it becomes active instead. For the first entry of a - * group that heading belongs to the previous group, which would leave the target's own group - * collapsed. + * Make the entry the URL hash names the active one, on load and on every hash + * change, and open the group holding it. + * + * The hash is authoritative here rather than the scroll position, because the + * two disagree. A browser parks a linked heading at scroll-padding-top plus + * any scroll-margin-top on the heading, and onScroll's activation line is + * scroll-padding-top alone; whenever the heading lands below that line the + * scroll pass picks the heading above it and highlights the wrong entry. The + * stylesheet now offsets once so the two line up, and this keeps the highlight + * correct even if that ever drifts again. + * + * It also covers the case onScroll cannot see at all: an in-page link fires + * hashchange, and a hash change does not always move the scroll position + * enough to produce a scroll event. */ - function revealHashTarget () { + function syncToHashTarget () { var hash = window.location.hash if (!hash) return - var link = links[hash] - if (!link && ~hash.indexOf('%')) { + var fragment = links[hash] ? hash : null + if (!fragment && ~hash.indexOf('%')) { try { - link = links[decodeURIComponent(hash)] + var decoded = decodeURIComponent(hash) + if (links[decoded]) fragment = decoded } catch (e) { return } } - if (link) revealGroup(link) + if (!fragment) return + setActive(fragment) + holdScrollUpdates() + scrollActiveIntoView() } function find (selector, from) { diff --git a/tests/toc-active-anchor/toc-active-anchor.test.js b/tests/toc-active-anchor/toc-active-anchor.test.js new file mode 100644 index 00000000..209a598e --- /dev/null +++ b/tests/toc-active-anchor/toc-active-anchor.test.js @@ -0,0 +1,265 @@ +/** + * Which "On this page" entry is highlighted after an in-page anchor jump. + * + * The bug this pins: a browser parks a linked heading at scroll-padding-top + + * scroll-margin-top, which is BELOW the activation line onScroll compares + * against, so the scroll pass picks the heading above the target and highlights + * the wrong entry. Clicking any in-page property link showed it. + * + * onScroll's geometry is not the contract. The contract is that a hash the URL + * names is the active entry, so these tests assert it at two landing positions: + * level with the activation line, and below it. The second one fails if the + * script goes back to inferring the active entry from pixels alone. + * + * A separate assertion covers the stylesheet, where the double offset came + * from. + */ +const assert = require('node:assert/strict') +const fs = require('node:fs') +const path = require('node:path') +const test = require('node:test') +const vm = require('node:vm') + +const SCRIPT = path.join(__dirname, '../../src/js/02-on-this-page.js') +const CSS = path.join(__dirname, '../../src/css/doc.css') + +// Minimal DOM: enough of createElement/appendChild/querySelector for the TOC builder. +function matches (el, simple) { + const m = simple.match(/^([a-z0-9]*)((?:\.[\w-]+)*)$/i) + if (!m) return false + const tag = m[1] + const classes = m[2] ? m[2].split('.').filter(Boolean) : [] + if (tag && el.tagName !== tag.toUpperCase()) return false + return classes.every((c) => el.classList.contains(c)) +} + +function collect (el, simple, out) { + el.children.forEach((child) => { + if (matches(child, simple)) out.push(child) + collect(child, simple, out) + }) + return out +} + +function makeEl (tag) { + const classes = new Set() + const el = { + tagName: tag.toUpperCase(), + nodeName: tag.toUpperCase(), + children: [], + parentNode: null, + attrs: {}, + dataset: {}, + handlers: {}, + _text: '', + classList: { + add: (...cs) => cs.forEach((c) => classes.add(c)), + remove: (...cs) => cs.forEach((c) => classes.delete(c)), + contains: (c) => classes.has(c), + toggle (c, force) { + const on = force === undefined ? !classes.has(c) : force + if (on) classes.add(c) + else classes.delete(c) + return on + }, + }, + get className () { return [...classes].join(' ') }, + set className (v) { + classes.clear() + v.split(/\s+/).filter(Boolean).forEach((c) => classes.add(c)) + }, + get textContent () { return el._text || el.children.map((c) => c.textContent).join('') }, + set textContent (v) { + el._text = v + el.children = [] + }, + get firstChild () { return el.children[0] || null }, + appendChild (child) { + if (child.parentNode) child.parentNode.removeChild(child) + child.parentNode = el + el.children.push(child) + return child + }, + insertBefore (node, ref) { + if (node.parentNode) node.parentNode.removeChild(node) + node.parentNode = el + const i = ref ? el.children.indexOf(ref) : -1 + if (i < 0) el.children.push(node) + else el.children.splice(i, 0, node) + return node + }, + removeChild (child) { + const i = el.children.indexOf(child) + if (i >= 0) el.children.splice(i, 1) + child.parentNode = null + return child + }, + setAttribute (k, v) { el.attrs[k] = String(v) }, + getAttribute (k) { return k in el.attrs ? el.attrs[k] : null }, + addEventListener (t, fn) { el.handlers[t] = fn }, + querySelector (sel) { return collect(el, sel, [])[0] || null }, + querySelectorAll (sel) { return collect(el, sel, []) }, + } + return el +} + +function heading (level, id, text) { + return { id, nodeName: 'H' + level, textContent: text } +} + +// A reverse-chronological page: years at level 1 (h2), months at level 2 (h3). +const HEADINGS = [ + heading(2, '2026', '2026'), + heading(3, 'september-2026', 'September 2026'), + heading(3, 'august-2026', 'August 2026'), + heading(2, '2025', '2025'), + heading(3, 'december-2025', 'December 2025'), + heading(3, 'november-2025', 'November 2025'), + heading(3, 'october-2025', 'October 2025'), + heading(2, '2024', '2024'), + heading(3, 'december-2024', 'December 2024'), +] + +// Drive the IIFE against a stub DOM and hand back the built list. Headings are copied so scrollTo() +// can position them per test; scrollY and scrollHeight feed the end-of-page check in onScroll, and +// hash is what window.location.hash reports on load. +function run ({ collapsible, headings = HEADINGS, scrollY = 0, scrollHeight = 5000, hash = '' }) { + headings = headings.map((h) => Object.assign({}, h)) + const sidebar = makeEl('aside') + sidebar.dataset = { levels: '2', title: '', collapsible: collapsible ? 'true' : undefined } + const menu = makeEl('div') + menu.className = 'toc-menu' + sidebar.appendChild(menu) + + const article = makeEl('article') + article.parentNode = { querySelectorAll: () => headings } + + const listeners = {} + const context = { + console, + setTimeout: () => 0, + document: { + addEventListener () {}, + getElementById: () => null, + createElement: makeEl, + documentElement: { scrollHeight }, + querySelector: (sel) => { + if (sel === 'aside.toc.sidebar') return sidebar + if (sel === 'article.doc') return article + return null + }, + }, + window: { + addEventListener: (t, fn) => { listeners[t] = fn }, + location: { hash }, + scrollY, + innerHeight: 800, + // A 16px root font and an 80px sticky header (scroll-padding-top) put the activation line at 80px. + getComputedStyle: () => ({ fontSize: '16px', paddingTop: '0px', scrollPaddingTop: '80px' }), + }, + } + vm.runInNewContext(fs.readFileSync(SCRIPT, 'utf8'), context) + const list = menu.children[0] + return { sidebar, menu, list, listeners, headings, win: context.window } +} + +// Document position of each heading: a year heading sits 60px above its first month, and everything +// else is 400px apart, so the layout has the same adjacency as a real What's New page. +function layout (headings) { + let y = 0 + return headings.map((h, i) => { + const pos = y + const next = headings[i + 1] + y += next && parseInt(next.nodeName.slice(1), 10) > parseInt(h.nodeName.slice(1), 10) ? 60 : 400 + return pos + }) +} + +// Pretend the window is scrolled so the heading with this id sits `landing` px from the top of the +// viewport. 80 is the activation line (scroll-padding-top). A deep-linked heading lands lower, at +// scroll-padding-top + scroll-margin-top, which is 165 here and in the real stylesheet. +function scrollTo (headings, id, landing = 80) { + const index = headings.findIndex((h) => h.id === id) + assert.notEqual(index, -1, 'scrollTo target exists: ' + id) + const ys = layout(headings) + headings.forEach((h, i) => { h.getBoundingClientRect = () => ({ top: landing + ys[i] - ys[index] }) }) +} + +// Sidebar links keyed by fragment, so tests can read active state without walking the tree. +function linksByHref (list) { + const out = {} + list.querySelectorAll('a').forEach((a) => { out[a.href] = a }) + return out +} + + +// Landing positions a real browser produces. 80 is the activation line +// (scroll-padding-top in the harness). 165 is where a deep-linked heading +// actually lands today, because the stylesheet adds scroll-margin-top on top of +// scroll-padding-top. +const ON_THE_LINE = 80 +const BELOW_THE_LINE = 165 + +for (const landing of [ON_THE_LINE, BELOW_THE_LINE]) { + test(`a deep-linked heading is the active entry, landing at ${landing}px`, () => { + const { list, listeners, headings } = run({ collapsible: false, hash: '#october-2025' }) + scrollTo(headings, 'october-2025', landing) + listeners.load() + + const links = linksByHref(list) + assert.ok( + links['#october-2025'].classList.contains('is-active'), + 'the heading the URL names is active' + ) + const others = Object.keys(links).filter((h) => h !== '#october-2025' && links[h].classList.contains('is-active')) + assert.deepEqual(others, [], 'no other entry is left active') + }) + + test(`an in-page link fires hashchange and moves the highlight, landing at ${landing}px`, () => { + // This is the reported case: click a property link in the body. No load, no + // TOC click, just a hashchange. + const { list, listeners, headings, win } = run({ collapsible: false, hash: '#november-2025' }) + scrollTo(headings, 'november-2025', landing) + listeners.load() + + win.location.hash = '#december-2024' + scrollTo(headings, 'december-2024', landing) + listeners.hashchange() + + const links = linksByHref(list) + assert.ok(links['#december-2024'].classList.contains('is-active'), 'the new target is active') + assert.ok(!links['#november-2025'].classList.contains('is-active'), 'the old target is not still active') + }) +} + +test('a percent-encoded hash still activates its entry', () => { + const { list, listeners, headings, win } = run({ collapsible: false }) + scrollTo(headings, 'october-2025', BELOW_THE_LINE) + listeners.load() + win.location.hash = '#october%2D2025' + listeners.hashchange() + const links = linksByHref(list) + assert.ok(links['#october-2025'].classList.contains('is-active'), 'decoded hash matches its entry') +}) + +test('a hash that names nothing on the page leaves the highlight alone', () => { + const { list, listeners, headings, win } = run({ collapsible: false }) + scrollTo(headings, 'october-2025', ON_THE_LINE) + listeners.load() + const before = Object.keys(linksByHref(list)).filter((h) => linksByHref(list)[h].classList.contains('is-active')) + win.location.hash = '#not-a-heading' + listeners.hashchange() + const after = Object.keys(linksByHref(list)).filter((h) => linksByHref(list)[h].classList.contains('is-active')) + assert.deepEqual(after, before, 'an unknown hash is ignored rather than clearing the highlight') +}) + +test('the stylesheet offsets an anchor jump once, not twice', () => { + // scroll-padding-top on html already clears the sticky bar for every anchor + // jump. A scroll-margin-top on the headings adds a second offset, which is + // what put a linked heading below the activation line and highlighted the + // entry above it. One offset, in one place, so the browser and onScroll agree. + const css = fs.readFileSync(CSS, 'utf8') + assert.match(css, /html\s*\{[^}]*scroll-padding-top:/, 'html still carries scroll-padding-top') + const headingMargin = css.match(/\.doc h[2-6]\[id\][^{]*\{[^}]*scroll-margin-top:[^}]*\}/g) + assert.equal(headingMargin, null, 'headings do not add a second offset via scroll-margin-top') +}) diff --git a/tests/toc-collapsible/toc-collapsible.test.js b/tests/toc-collapsible/toc-collapsible.test.js index c4127346..22fce7db 100644 --- a/tests/toc-collapsible/toc-collapsible.test.js +++ b/tests/toc-collapsible/toc-collapsible.test.js @@ -294,26 +294,29 @@ test('the load pass activates the entry on the activation line and opens its gro }) test('arriving on a deep link opens the group of the target entry', () => { - // Browsers park a deep-linked heading at scroll-padding-top + scroll-margin-top, below the - // activation line, so the scroll pass marks the heading above it active. For the first entry of - // a group that heading belongs to the previous group and would leave the target's group - // collapsed. The load handler has to open it from the hash instead. + // A deep-linked heading can land below the activation line onScroll compares + // against, in which case the scroll pass would pick the heading above it. The + // load handler activates from the hash instead, so the target is active and + // its group opens. 165 is a landing below the line; see tests/toc-active-anchor + // for the activation contract on its own. const { list, listeners, headings } = run({ collapsible: true, hash: '#december-2025' }) const [y2026, y2025, y2024] = list.children scrollTo(headings, 'december-2025', 165) listeners.load() const links = linksByHref(list) - assert.equal(links['#august-2026'].classList.contains('is-active'), true, 'the scroll pass picks the heading above') - assert.equal(y2025.classList.contains('is-expanded'), true, 'the hash opens the target group anyway') + assert.equal(links['#december-2025'].classList.contains('is-active'), true, 'the hash target is active, not the heading above it') + assert.equal(links['#august-2026'].classList.contains('is-active'), false, 'the heading above the target is not active') + assert.equal(y2025.classList.contains('is-expanded'), true, 'the target group is open') assert.equal(y2025.children[1].getAttribute('aria-expanded'), 'true') assert.equal(y2026.classList.contains('is-expanded'), true) assert.equal(y2024.classList.contains('is-expanded'), false) }) test('changing the hash after load opens the group of the new target', () => { - // An in-page link or back/forward fires hashchange, not load. The scroll pass still picks the - // heading above the target, so the group has to be opened from the new hash. + // An in-page link or back/forward fires hashchange, not load, and the scroll + // pass may not run at all. The highlight and the group both come from the new + // hash. const { list, listeners, headings, win } = run({ collapsible: true }) const [, y2025, y2024] = list.children scrollTo(headings, '2026') @@ -327,8 +330,9 @@ test('changing the hash after load opens the group of the new target', () => { listeners.hashchange() const links = linksByHref(list) - assert.equal(links['#october-2025'].classList.contains('is-active'), true, 'the scroll pass picks the heading above') - assert.equal(y2024.classList.contains('is-expanded'), true, 'the hash opens the target group anyway') + assert.equal(links['#december-2024'].classList.contains('is-active'), true, 'the new hash target is active') + assert.equal(links['#october-2025'].classList.contains('is-active'), false, 'the heading above the target is not active') + assert.equal(y2024.classList.contains('is-expanded'), true, 'the target group is open') assert.equal(y2025.classList.contains('is-expanded'), true) }) From 6cc40e69e7c3306f66564fe79f1603648bcff99b Mon Sep 17 00:00:00 2001 From: JakeSCahill Date: Fri, 18 Sep 2026 19:17:27 +0100 Subject: [PATCH 2/2] Run the new On-this-page test in CI validate-build.yml names each node --test suite explicitly, so a new one is invisible to CI until it is listed. tests/toc-active-anchor was passing locally and would never have run on a PR. head-meta, property-tooltips and negative-cache are in the same position and predate this; left alone here rather than widened in a fix for something else. --- .github/workflows/validate-build.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/validate-build.yml b/.github/workflows/validate-build.yml index b656f411..f07e83a9 100644 --- a/.github/workflows/validate-build.yml +++ b/.github/workflows/validate-build.yml @@ -75,6 +75,8 @@ jobs: run: npm run test:kapa-source-groups - name: Test collapsible TOC groups run: npm run test:toc-collapsible + - name: Test the active On-this-page entry after an anchor jump + run: npm run test:toc-active-anchor - name: Test anonymous Ask AI quota run: npm run test:anon-quota - name: Test nav positioning leaves the page scroll alone