STA-262: Make review title permalinks native and stable after async rendering

src/report/client.ts

@@ -222,7 +222,9 @@ function revealFragment(final: boolean) {
   }
 
   if (!final) return
-  target.scrollIntoView?.({ block: 'center' })
+  // Align the target's leading edge (its title or marker). Centering a target
+  // taller than the viewport would push that edge far off-screen.
+  target.scrollIntoView?.({ block: 'start' })
   const targets = new Set(generatedTargets())
   const focusTarget = [...document.querySelectorAll<HTMLButtonElement>('[data-copy-fragment]')]
     .find((button) => {
@@ -239,7 +241,7 @@ const feedbackTimers = new WeakMap<HTMLButtonElement, ReturnType<typeof setTimeo
 
 function showCopyFeedback(button: HTMLButtonElement, success: boolean) {
   const label = button.querySelector<HTMLElement>('[data-copy-label]')
-  const original = label?.dataset.originalLabel ?? label?.textContent ?? 'Link'
+  const original = label?.dataset.originalLabel ?? label?.textContent ?? 'Copy'
   if (label) {
     label.dataset.originalLabel = original
     label.textContent = success ? 'Copied' : 'Failed'
@@ -319,7 +321,12 @@ function wireFragments(initialRender: Promise<void>) {
     if (!(origin instanceof Element)) return
 
     const link = origin.closest<HTMLAnchorElement>('a[href^="#"]')
-    if (link && new URL(link.href).hash === window.location.hash) navigate()
+    if (link && new URL(link.href).hash === window.location.hash) {
+      // A section permalink lives inside its <summary>; the browser's default
+      // action can toggle that fold after the click dispatch. Let it settle, then
+      // reopen ancestors and realign the title on the next task.
+      setTimeout(navigate, 0)
+    }
   })
   navigate()
   void initialRender.then(() => {

src/report/shell.ts

@@ -154,11 +154,11 @@ function renderSection(
       .filter((change) => change.canonical)
       .map(
         (change) =>
-          `<span class="change-target" id="${change.fragment}" data-target-kind="change">${renderCopyLink(change.fragment, `Copy link to change ${change.id}`, change.id)}</span>`,
+          `<span class="change-target" id="${change.fragment}" data-target-kind="change">${renderPermalink(change.fragment, `Permalink to change ${change.id}`, change.id)}${renderCopyLink(change.fragment, `Copy link to change ${change.id}`)}</span>`,
       )
       .join('')
     const actions = `<div class="step-actions">
-    ${renderCopyLink(stepTarget.fragment, `Copy link to step ${stepIndex + 1} in ${section.title}`)}${changeTargets}
+    ${renderPermalink(stepTarget.fragment, `Permalink to step ${stepIndex + 1} in ${section.title}`, 'Link')}${renderCopyLink(stepTarget.fragment, `Copy link to step ${stepIndex + 1} in ${section.title}`)}${changeTargets}
   </div>`
     if (step.diff === undefined) {
       return `<div class="step" id="${stepTarget.fragment}" data-step-index="${stepIndex}" data-target-kind="step">${actions}${textMarkup}</div>`
@@ -198,7 +198,7 @@ function renderSection(
 
   const markup = `<section class="section" id="${target.fragment}" data-section-index="${index}" data-target-kind="section">
   <details class="section-fold" open>
-    <summary class="section-title"><span class="section-title-index">${sectionIndex(index)}</span><span class="section-title-text">${escapeHtml(section.title)}</span>${renderCopyLink(target.fragment, `Copy link to section ${section.title}`)}</summary>
+    <summary class="section-title"><span class="section-title-index">${sectionIndex(index)}</span><span class="section-title-text">${escapeHtml(section.title)}</span>${renderPermalink(target.fragment, `Permalink to section ${section.title}`, 'Link')}${renderCopyLink(target.fragment, `Copy link to section ${section.title}`)}</summary>
 ${steps.join('\n')}
   </details>
 </section>`
@@ -229,7 +229,11 @@ ${links}
 </nav>`
 }
 
-function renderCopyLink(fragment: string, label: string, text = 'Link'): string {
+function renderPermalink(fragment: string, label: string, text: string): string {
+  return `<a class="permalink" href="#${fragment}" aria-label="${escapeHtml(label)}">${escapeHtml(text)}</a>`
+}
+
+function renderCopyLink(fragment: string, label: string, text = 'Copy'): string {
   return `<button type="button" class="copy-link" data-copy-fragment="${fragment}" aria-label="${escapeHtml(label)}"><span data-copy-label>${escapeHtml(text)}</span></button>`
 }
 
@@ -456,7 +460,7 @@ main { max-width: none; min-width: 0; margin: 0; padding: 22px 28px 72px; }
 .step { scroll-margin-top: 18px; }
 .step-actions { display: flex; flex-wrap: wrap; justify-content: flex-end; gap: 5px; padding: 8px 12px 0; }
 .step-text { max-width: 900px; padding: 8px 20px; font-size: 17px; }
-.copy-link {
+.copy-link, .permalink {
   padding: 3px 7px;
   border: 1px solid #c4d1c6;
   border-radius: 5px;
@@ -465,11 +469,12 @@ main { max-width: none; min-width: 0; margin: 0; padding: 22px 28px 72px; }
   font: 600 10px/1.35 ui-monospace, SFMono-Regular, Menlo, monospace;
   cursor: pointer;
 }
-.copy-link:hover { color: var(--accent); border-color: #8eaa95; background: #eef5ef; }
-.copy-link:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; }
+.permalink { text-decoration: none; }
+.copy-link:hover, .permalink:hover { color: var(--accent); border-color: #8eaa95; background: #eef5ef; }
+.copy-link:focus-visible, .permalink:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; }
 .copy-link[data-copy-state="success"] { color: var(--accent); border-color: #8eaa95; }
 .copy-link[data-copy-state="failure"] { color: #a1262f; border-color: #d7a4a8; }
-.change-target { scroll-margin-top: 18px; }
+.change-target { display: inline-flex; align-items: center; gap: 5px; scroll-margin-top: 18px; }
 .copy-status {
   position: fixed;
   width: 1px;
@@ -590,6 +595,9 @@ main { max-width: none; min-width: 0; margin: 0; padding: 22px 28px 72px; }
   .review-controls { display: flex; gap: 6px; justify-content: flex-end; margin: 0; }
   .layout-form { flex: 1 1 auto; max-width: 220px; margin: 0 0 0 auto; }
   .fold-all { flex: none; padding: 5px 9px; font-size: 12px; }
+  /* The aligned fragment target must clear the sticky strip, so push its
+     scroll-margin past the strip plus breathing room. */
+  .section, .step, .change-target { scroll-margin-top: 64px; }
   main { padding: 14px 10px 50px; }
 }
 @media (max-width: 520px) {
@@ -600,7 +608,7 @@ main { max-width: none; min-width: 0; margin: 0; padding: 22px 28px 72px; }
   .section-fold > summary { font-size: 17px; }
 }
 @media print {
-  .layout-form, .copy-link { display: none; }
+  .layout-form, .copy-link, .permalink { display: none; }
   .review-map { display: none; }
   .review-workspace { display: block; }
   .report-cover { box-shadow: none; break-inside: avoid; }

test/report-dom.test.ts

@@ -372,6 +372,189 @@ describe('report browser client', () => {
     expect(doc.activeElement).toBe(target.querySelector('[data-copy-fragment="change-001"]'))
   })
 
+  test('a direct load aligns a viewport-taller section title instead of centering the section', async () => {
+    const value = document([section(simplePatch(), 'Long section')])
+    const fragment = reportTargets(value)[0]!.fragment
+    const dom = loadReport(renderReport(value, clientBundle), {
+      url: `https://reports.example/r/report-id#${fragment}`,
+    })
+    const doc = dom.document as unknown as Document
+    const sectionTarget = doc.getElementById(fragment) as HTMLElement
+
+    // Model the browser scroll: the section starts far down the document and is
+    // taller than the viewport, exactly the shape reported in the ticket.
+    const viewportHeight = 577
+    const sectionTop = 124_086
+    const sectionHeight = 58_144
+    const optionsSeen: ScrollIntoViewOptions[] = []
+    let scrollTop = 0
+    sectionTarget.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      optionsSeen.push(options ?? {})
+      const block = options?.block ?? 'start'
+      scrollTop =
+        block === 'center'
+          ? sectionTop + sectionHeight / 2 - viewportHeight / 2
+          : block === 'end'
+            ? sectionTop + sectionHeight - viewportHeight
+            : sectionTop
+    }) as typeof sectionTarget.scrollIntoView
+
+    runReportClient()
+    await waitFor(() => optionsSeen.length > 0)
+
+    expect(optionsSeen).toEqual([{ block: 'start' }])
+    // The section title is the section's leading edge; after alignment it sits at
+    // the top of the viewport instead of thousands of pixels above it.
+    const titleTopInViewport = sectionTop - scrollTop
+    expect(titleTopInViewport).toBeGreaterThanOrEqual(0)
+    expect(titleTopInViewport).toBeLessThan(viewportHeight)
+  })
+
+  test('clicking a permalink for the current fragment repositions it without centering', async () => {
+    const value = document([section(simplePatch(), 'Reveal me')])
+    const fragment = reportTargets(value)[0]!.fragment
+    const dom = loadReport(renderReport(value, clientBundle), {
+      url: `https://reports.example/r/report-id#${fragment}`,
+    })
+    const doc = dom.document as unknown as Document
+    const sectionTarget = doc.getElementById(fragment) as HTMLElement
+    const sectionFold = sectionTarget.querySelector<HTMLDetailsElement>('.section-fold')!
+    const calls: ScrollIntoViewOptions[] = []
+    sectionTarget.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      calls.push(options ?? {})
+    }) as typeof sectionTarget.scrollIntoView
+
+    runReportClient()
+    await waitFor(() => calls.length === 1)
+
+    const anchor = doc.querySelector<HTMLAnchorElement>(`a.permalink[href="#${fragment}"]`)
+    expect(anchor).not.toBeNull()
+    expect(anchor!.tagName).toBe('A')
+    expect(anchor!.getAttribute('href')).toBe(`#${fragment}`)
+
+    // Simulate the settled result of any summary toggle before the correction.
+    sectionFold.open = false
+    calls.length = 0
+    anchor!.dispatchEvent(
+      new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) as unknown as Event,
+    )
+    await waitFor(() => calls.length === 1)
+
+    expect(sectionFold.open).toBe(true)
+    expect(calls[0]).toEqual({ block: 'start' })
+  })
+
+  test('clicking a folded step permalink opens its ancestors and aligns the step', async () => {
+    const value = document([
+      {
+        title: 'Navigate by link',
+        steps: [{ text: 'Target this step.', diff: simplePatch() }],
+      },
+    ])
+    const fragment = reportTargets(value)[0]!.steps[0]!.fragment
+    const dom = loadReport(renderReport(value, clientBundle), {
+      url: 'https://reports.example/r/report-id',
+    })
+    const doc = dom.document as unknown as Document
+    runReportClient()
+    const step = doc.getElementById(fragment) as HTMLElement
+    const sectionFold = step.closest<HTMLDetailsElement>('.section-fold')!
+    const fileFold = step.querySelector<HTMLDetailsElement>('details.file')!
+    sectionFold.open = false
+    fileFold.open = false
+    const calls: ScrollIntoViewOptions[] = []
+    step.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      calls.push(options ?? {})
+    }) as typeof step.scrollIntoView
+
+    const anchor = doc.querySelector<HTMLAnchorElement>(`a.permalink[href="#${fragment}"]`)!
+    expect(anchor.tagName).toBe('A')
+    anchor.dispatchEvent(
+      new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) as unknown as Event,
+    )
+
+    await waitFor(() => calls.length === 1)
+    expect(sectionFold.open).toBe(true)
+    expect(fileFold.open).toBe(true)
+    expect(step.scrollIntoView).toBeDefined()
+    expect(calls[0]).toEqual({ block: 'start' })
+  })
+
+  test('a target after a large async diff is aligned only after every render settles', async () => {
+    const large = Array.from({ length: 400 }, (_, index) => `-line ${index}\n+LINE ${index}`).join(
+      '\n',
+    )
+    const bigPatch = [
+      'diff --git a/big.ts b/big.ts',
+      '--- a/big.ts',
+      '+++ b/big.ts',
+      '@@ -1,400 +1,400 @@',
+      large,
+      '',
+    ].join('\n')
+    const value = document([
+      section(bigPatch, 'Large first section'),
+      { title: 'Later target', steps: [{ text: 'The real target.', diff: simplePatch() }] },
+    ])
+    const targets = reportTargets(value)
+    const fragment = targets[1]!.steps[0]!.fragment
+    const dom = loadReport(renderReport(value, clientBundle), {
+      url: `https://reports.example/r/report-id#${fragment}`,
+    })
+    const doc = dom.document as unknown as Document
+    const first = doc.getElementById(targets[0]!.fragment) as HTMLElement
+    const second = doc.getElementById(fragment) as HTMLElement
+    const calls: { element: Element; options: ScrollIntoViewOptions }[] = []
+    first.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      calls.push({ element: first, options: options ?? {} })
+    }) as typeof first.scrollIntoView
+    second.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      calls.push({ element: second, options: options ?? {} })
+    }) as typeof second.scrollIntoView
+
+    runReportClient()
+    await waitFor(() => calls.length > 0)
+
+    expect(calls).toHaveLength(1)
+    expect(calls[0]!.element).toBe(second)
+    expect(calls[0]!.options).toEqual({ block: 'start' })
+  })
+
+  test('focusing either title action leaves the corrected target position alone', async () => {
+    const value = document([section(simplePatch(), 'Focus target')])
+    const fragment = reportTargets(value)[0]!.fragment
+    const dom = loadReport(renderReport(value, clientBundle), {
+      url: `https://reports.example/r/report-id#${fragment}`,
+    })
+    const doc = dom.document as unknown as Document
+    const sectionTarget = doc.getElementById(fragment) as HTMLElement
+    const calls: ScrollIntoViewOptions[] = []
+    sectionTarget.scrollIntoView = ((options?: ScrollIntoViewOptions) => {
+      calls.push(options ?? {})
+    }) as typeof sectionTarget.scrollIntoView
+
+    const button = doc.querySelector<HTMLButtonElement>(`[data-copy-fragment="${fragment}"]`)!
+    const focusOptions: (FocusOptions | undefined)[] = []
+    const originalFocus = button.focus.bind(button)
+    button.focus = ((options?: FocusOptions) => {
+      focusOptions.push(options)
+      originalFocus(options)
+    }) as typeof button.focus
+
+    runReportClient()
+    await waitFor(() => calls.length === 1)
+
+    // The client's own correction focuses the copy action without scrolling.
+    expect(focusOptions).toEqual([{ preventScroll: true }])
+    expect(doc.activeElement).toBe(button)
+
+    // Focusing the native permalink must not add a competing scroll either.
+    const anchor = doc.querySelector<HTMLAnchorElement>(`a.permalink[href="#${fragment}"]`)!
+    anchor.focus()
+    expect(doc.activeElement).toBe(anchor)
+    expect(calls).toHaveLength(1)
+  })
+
   test('waits for every initial render and honors a hashchange while mounting', async () => {
     const value = document([
       section(simplePatch('one', 'one!'), 'Delayed first'),