Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion e2e/outline-anchors.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test } from '@playwright/test'
import { expect, type Page, test } from '@playwright/test'

test('outline links align headings below the sticky header, including repeated clicks', async ({
page,
Expand Down Expand Up @@ -39,3 +39,41 @@ test('outline links align headings below the sticky header, including repeated c
.toBeLessThan(2)
}
})

const headingOffset = (page: Page, id: string) =>
page.evaluate((id) => {
const heading = document.getElementById(id)
if (!heading || window.scrollY === 0) return Infinity
const padding =
Number.parseFloat(getComputedStyle(document.documentElement).scrollPaddingTop) || 0
const margin = Number.parseFloat(getComputedStyle(heading).scrollMarginTop) || 0
return Math.abs(heading.getBoundingClientRect().top - padding - margin)
}, id)

// The router's own hash scroll misses this heading, which streams in after the route commit.
for (const { from, link, id } of [
{
from: '/docs/accounts',
link: 'article a[href$="/create#create-a-configurable-account"]',
id: 'create-a-configurable-account',
},
]) {
test(`links from ${from} align #${id} after client-side navigation`, async ({ page }) => {
await page.setViewportSize({ width: 1440, height: 900 })
await page.goto(from)
// Let React hydrate, so the click goes through the router, not a document load.
await page.waitForLoadState('networkidle')
await page.evaluate(() => {
;(window as Window & { __clientNavigation?: boolean }).__clientNavigation = true
})
await page.locator(link).first().click()
await expect(page).toHaveURL(new RegExp(`#${id}$`))
await expect.poll(() => headingOffset(page, id), { timeout: 10_000 }).toBeLessThan(2)
// A full page load would pass through the browser's native hash scroll instead.
expect(
await page.evaluate(
() => (window as Window & { __clientNavigation?: boolean }).__clientNavigation,
),
).toBe(true)
})
}
90 changes: 90 additions & 0 deletions src/components/HashScrollOnNavigate.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
'use client'

import { useEffect, useRef } from 'react'
import { useRouter } from 'waku'

const SETTLE_MS = 4000

function targetFor(hash: string) {
const raw = hash.replace(/^#/, '')
if (!raw) return null
let id = raw
try {
id = decodeURIComponent(raw)
} catch {}
return document.getElementById(id) ?? document.getElementById(raw)
}

/**
* Align the URL's heading after a client-side navigation to another page.
*
* Waku and Vocs scroll to the hash in the commit that changes the route, but a
* docs page's content streams in after that commit. The heading is missing then,
* so the router scrolls to the top and nothing retries. This waits for the
* heading to mount, aligns it below the sticky header (scroll-padding-top), and
* keeps it aligned while demos above it finish rendering. It stops at the first
* user scroll input, on back/forward navigation, or after SETTLE_MS.
*/
export function HashScrollOnNavigate() {
const { path, hash } = useRouter()
const previousPath = useRef<string | null>(null)
const popstate = useRef(false)

useEffect(() => {
const onPopstate = () => {
popstate.current = true
}
window.addEventListener('popstate', onPopstate)
return () => window.removeEventListener('popstate', onPopstate)
}, [])

useEffect(() => {
const pathChanged = previousPath.current !== null && previousPath.current !== path
previousPath.current = path
const wasPopstate = popstate.current
popstate.current = false
// A full page load is handled by the browser, a same-page hash by Vocs, and
// back/forward by Vocs' scroll restoration.
if (!pathChanged || wasPopstate) return
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
const currentHash = hash || window.location.hash
if (!currentHash || currentHash === '#') return

let done = false
let frame = 0
const align = () => {
cancelAnimationFrame(frame)
frame = requestAnimationFrame(() => {
if (done) return
const target = targetFor(currentHash)
if (!target) return
const padding =
Number.parseFloat(getComputedStyle(document.documentElement).scrollPaddingTop) || 0
const margin = Number.parseFloat(getComputedStyle(target).scrollMarginTop) || 0
if (Math.abs(target.getBoundingClientRect().top - padding - margin) > 1)
target.scrollIntoView({ block: 'start', behavior: 'instant' })
})
}
const stop = () => {
if (done) return
done = true
cancelAnimationFrame(frame)
clearTimeout(timer)
observer.disconnect()
resizeObserver.disconnect()
for (const event of userEvents) window.removeEventListener(event, stop)
}

const observer = new MutationObserver(align)
observer.observe(document.body, { childList: true, subtree: true })
const resizeObserver = new ResizeObserver(align)
resizeObserver.observe(document.body)
const userEvents = ['wheel', 'touchstart', 'keydown', 'mousedown'] as const
Comment on lines +78 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: On slow connections, readers following a cross-page heading link still land at the top whenever the page takes longer than 4 s to stream in. The SETTLE_MS timer starts at the route commit, not when the heading mounts. That commit happens before the content streams. If streaming takes longer than 4000 ms, stop disconnects both observers before the target exists, so nothing ever scrolls to it. Fix: keep waiting until the target first mounts, with a separate, longer cap; start the 4 s keep-aligned window from that point. The wheel/touch/key/mouse stop already protects users who scroll while waiting.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

A reader on a slow mobile network clicks a link from one docs page to a heading on another. The effect in src/components/HashScrollOnNavigate.tsx:41-87 runs at the route commit, before the content has streamed. It starts setTimeout(stop, SETTLE_MS) right away at line 81, with SETTLE_MS = 4000 (line 6). align returns early while targetFor(currentHash) is null (line 59). If the heading arrives after 4000 ms, stop has already disconnected the MutationObserver and ResizeObserver and set done. Nothing re-runs, because path and hash do not change again. The router's scrollTopForMissingHash scroll-to-top stays in place, exactly as on the base branch. So the fix works only when streaming beats a fixed 4 s. Bounding the wait by user input is not the problem: the userEvents listeners at lines 79-80 already stop it as soon as the reader scrolls.

Verification: Pre-existing (partial fix). This happens when the target page's content streams in more than 4 s after the route commit. In src/components/HashScrollOnNavigate.tsx, const timer = setTimeout(stop, SETTLE_MS) (line 81) starts the timer at that moment, with SETTLE_MS = 4000 (line 6). So if the heading mounts after the 4 s timer has fired, no code scrolls to it.

for (const event of userEvents) window.addEventListener(event, stop, { passive: true })
const timer = setTimeout(stop, SETTLE_MS)
align()

return stop
}, [path, hash])

return null
}
3 changes: 3 additions & 0 deletions src/pages/_layout.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { PropsWithChildren } from 'react'
import DocsEntryLayout from '../components/DocsEntryLayout'
import { HashScrollOnNavigate } from '../components/HashScrollOnNavigate'
import '../styles/globals'
import { normalizeProxiedRscFetch } from '../lib/rsc-route-normalization'

Expand Down Expand Up @@ -31,6 +32,8 @@ export default function Layout(
<meta name="twitter:site" content="@tempo" />
{/* biome-ignore lint/security/noDangerouslySetInnerHtml: static bootstrap must run before the RSC client bundle. */}
<script dangerouslySetInnerHTML={{ __html: normalizeProxiedRscFetch }} />
{/* Mounted at the root so it stays mounted when a link crosses between site sections. */}
<HashScrollOnNavigate />
<DocsEntryLayout>{props.children}</DocsEntryLayout>
</>
)
Expand Down
Loading