diff --git a/src/report/client.ts b/src/report/client.ts index 159e1fe..1aaf058 100644 --- a/src/report/client.ts +++ b/src/report/client.ts @@ -40,7 +40,6 @@ export function mountReport( void mountedDiffs.initialRender.then(finishInitialRender) const { mounted } = mountedDiffs wireLayout(mounted) - wireGlobalFolds() prepareForPrint() } @@ -273,6 +272,19 @@ function wireSectionFolds() { fold.addEventListener('toggle', sync) sync() } + document.querySelectorAll('[data-section-fold-all]').forEach((button) => { + button.addEventListener('click', (event) => { + event.preventDefault() + event.stopPropagation() + const section = button.closest('.section') + if (!section) return + const open = button.dataset.sectionFoldAll === 'unfold' + for (const details of section.querySelectorAll('details.file')) { + details.open = open + } + button.focus({ preventScroll: true }) + }) + }) } function wireFragments(initialRender: Promise) { @@ -329,61 +341,10 @@ function wireLayout(mounted: MountedDiff[]) { }) } -// The global control is one toggle: it folds every section while any is open and -// unfolds them once they are all closed. It only touches `open` on existing section -// folds, so the report data is untouched and a single section still toggles natively. function sectionFolds(): HTMLDetailsElement[] { return [...document.querySelectorAll('details.section-fold')] } -// The focused child of a fold that just closed is no longer rendered, but browsers can -// leave focus on it. Only the fold's own summary row stays visible. -function hiddenByClosedFold(element: Element): boolean { - for (let node: Element | null = element; node; node = node.parentElement) { - if (!node.matches('details:not([open])')) continue - const summary = node.querySelector(':scope > summary') - // A closed details hides its content but not its own summary row, so keep - // walking: an outer closed details can still hide that summary. - if (summary && (summary === element || summary.contains(element))) continue - return true - } - return false -} - -function wireGlobalFolds() { - const button = document.querySelector('[data-fold-all]') - if (!button) return - const label = button.querySelector('[data-fold-all-label]') - const folds = sectionFolds() - if (folds.length === 0) return - - const allFolded = () => folds.every((fold) => !fold.open) - const sync = () => { - const folded = allFolded() - if (label) label.textContent = folded ? 'Unfold all' : 'Fold all' - button.setAttribute( - 'aria-label', - folded ? 'Unfold all review sections' : 'Fold all review sections', - ) - } - - button.addEventListener('click', () => { - // Read focus before closing: a browser blurs a descendant the moment its - // ancestor details closes, so afterwards activeElement may already be body. - const active = document.activeElement - const unfold = allFolded() - for (const fold of folds) fold.open = unfold - if (active instanceof HTMLElement && hiddenByClosedFold(active)) { - const section = active.closest('details.section-fold') - const summary = section?.querySelector('.section-toggle') - ;(summary ?? button).focus({ preventScroll: true }) - } - sync() - }) - for (const fold of folds) fold.addEventListener('toggle', sync) - sync() -} - function prepareForPrint() { window.addEventListener('beforeprint', () => { for (const details of document.querySelectorAll('details')) { diff --git a/src/report/render.ts b/src/report/render.ts index e16b00c..0b4ac10 100644 --- a/src/report/render.ts +++ b/src/report/render.ts @@ -76,8 +76,7 @@ function renderReportBody( ` - const foldAll = `` - const readingControls = `
${layoutForm}${foldAll}
` + const readingControls = `
${layoutForm}
` const reviewMap = renderReviewMap( document.sections.map((section, index) => ({ title: section.title, @@ -178,7 +177,7 @@ function renderSection(
${label} Renamed · content unchanged
` } - return `
+ return `
${label} +${stats.additions} −${stats.deletions}
` @@ -195,7 +194,7 @@ function renderSection( const markup = `
- ${escapeHtml(section.title)} + ${escapeHtml(section.title)} ${steps.join('\n')}
` @@ -488,7 +487,10 @@ main { max-width: none; min-width: 0; margin: 0; padding: 22px 28px 72px; } .section-title-index { font-family: ui-monospace, SFMono-Regular, Menlo, monospace; font-size: .82em; } .section-title-text { min-width: 0; color: inherit; text-decoration: none; } .section-title-text:hover { text-decoration: underline; } -.section-toggle:focus-visible, .section-title-text:focus-visible, .permalink:focus-visible { outline: 2px solid var(--accent); outline-offset: 3px; } +.section-fold-controls { display: flex; gap: 6px; margin-left: auto; font-size: 12px; font-weight: 500; } +.section-fold-all { border: 1px solid #cbd7cd; border-radius: 5px; padding: 4px 7px; color: #4d6654; background: #f7faf7; cursor: pointer; font: inherit; white-space: nowrap; } +.section-fold-all:hover { color: var(--accent); border-color: #8eaa95; background: #eef5ef; } +.section-toggle:focus-visible, .section-title-text:focus-visible, .section-fold-all:focus-visible, .permalink:focus-visible { outline: 2px solid var(--accent); outline-offset: 3px; } .section-fold > summary::-webkit-details-marker { display: none; } .section-fold[open] > summary { border-bottom-color: var(--border); } .prose { color: #3c4d41; font-size: 14px; } diff --git a/test/report-dom.test.ts b/test/report-dom.test.ts index 3285707..ba9dacc 100644 --- a/test/report-dom.test.ts +++ b/test/report-dom.test.ts @@ -800,8 +800,8 @@ describe('report browser client', () => { expect(doc.querySelector('[data-copy-fragment]')).toBeNull() expect(doc.querySelector('a[href="#change-001"]')).toBeNull() - summary.click() - expect(fold.open).toBe(true) + button.dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) as unknown as Event) + expect(fold.open).toBe(false) const click = new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) title.dispatchEvent(click as unknown as Event) expect(click.defaultPrevented).toBe(false) @@ -1023,7 +1023,7 @@ describe('report browser client', () => { ) test( - 'one control folds and unfolds every section and individual folds still work', + 'each section control folds and unfolds only its own section details', async () => { const value = document([ section(simplePatch('one', 'one!'), 'First section'), @@ -1037,51 +1037,27 @@ describe('report browser client', () => { const dataBefore = dataScript.textContent runReportClient() - const button = doc.querySelector('[data-fold-all]')! const folds = [...doc.querySelectorAll('details.section-fold')] - const label = () => button.querySelector('[data-fold-all-label]')?.textContent - const aria = () => button.getAttribute('aria-label') expect(folds).toHaveLength(2) expect(folds.every((fold) => fold.open)).toBe(true) - expect(label()).toBe('Fold all') - expect(aria()).toBe('Fold all review sections') - - button.click() - expect(folds.every((fold) => !fold.open)).toBe(true) - expect(label()).toBe('Unfold all') - expect(aria()).toBe('Unfold all review sections') - - button.click() - expect(folds.every((fold) => fold.open)).toBe(true) - expect(label()).toBe('Fold all') - expect(aria()).toBe('Fold all review sections') + expect([...doc.querySelectorAll('details.file')].every((file) => !file.open)).toBe(true) expect(dataScript.textContent).toBe(dataBefore) - - // Individual section buttons still fold and unfold after the global action, - // and their toggle drives the global label. - folds[0]! - .querySelector('.section-toggle')! - .dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) as unknown as Event) - expect(folds[0]!.open).toBe(false) + const first = doc.querySelector('.section')! + first.querySelector('[data-section-fold-all="unfold"]')!.click() + expect(folds[0]!.open).toBe(true) expect(folds[1]!.open).toBe(true) - expect(label()).toBe('Fold all') - - folds[1]! - .querySelector('.section-toggle')! - .dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }) as unknown as Event) - expect(folds.every((fold) => !fold.open)).toBe(true) - expect(label()).toBe('Unfold all') - - button.click() + expect(first.querySelector('details.file')?.open).toBe(true) + first.querySelector('[data-section-fold-all="fold"]')!.click() expect(folds.every((fold) => fold.open)).toBe(true) - expect(label()).toBe('Fold all') + expect(first.querySelector('details.file')?.open).toBe(false) + expect(doc.querySelectorAll('.step-text')).toHaveLength(2) }, 120000, ) test( - 'folding all moves focus from a hidden child to the visible section row', + 'folding all keeps section content and the control visible', async () => { const value = document([ { @@ -1100,28 +1076,13 @@ describe('report browser client', () => { runReportClient() const firstSection = doc.querySelector('.section')! - const focused = firstSection.querySelector( - '.step-actions > a', - )! - focused.focus() - expect(doc.activeElement).toBe(focused) - - // Real browsers blur a focused descendant the moment its ancestor details - // closes; happy-dom keeps focus, so simulate that blur to prove the global - // action still restores focus on the surviving section row. - for (const fold of doc.querySelectorAll('details.section-fold')) { - fold.addEventListener('toggle', () => { - if (!fold.open && fold.contains(focused)) focused.blur() - }) - } - - doc.querySelector('[data-fold-all]')!.click() + const foldAll = firstSection.querySelector('[data-section-fold-all="fold"]')! + foldAll.click() const folds = [...doc.querySelectorAll('details.section-fold')] - expect(folds.every((fold) => !fold.open)).toBe(true) - expect(firstSection.querySelector('details')?.open).toBe(false) - const summary = firstSection.querySelector('.section-toggle')! - expect(doc.activeElement).toBe(summary) + expect(folds.every((fold) => fold.open)).toBe(true) + expect(firstSection.querySelector('.step-text')?.textContent).toContain('First.') + expect(doc.activeElement).toBe(foldAll) }, 120000, ) @@ -1146,17 +1107,12 @@ describe('report browser client', () => { fileSummary.focus() expect(doc.activeElement).toBe(fileSummary) - for (const fold of doc.querySelectorAll('details.section-fold')) { - fold.addEventListener('toggle', () => { - if (!fold.open && fold.contains(fileSummary)) fileSummary.blur() - }) - } + const foldAll = firstSection.querySelector('[data-section-fold-all="fold"]')! + foldAll.click() - doc.querySelector('[data-fold-all]')!.click() - - const sectionSummary = firstSection.querySelector('.section-toggle')! - expect(doc.activeElement).toBe(sectionSummary) - expect(sectionSummary.closest('details')?.open).toBe(false) + expect(doc.activeElement).toBe(foldAll) + expect(firstSection.querySelector('.section-fold')?.open).toBe(true) + expect(file.open).toBe(false) }, 120000, ) @@ -1174,26 +1130,20 @@ describe('report browser client', () => { const doc = dom.document as unknown as Document runReportClient() - const button = doc.querySelector('[data-fold-all]')! const folds = [...doc.querySelectorAll('details.section-fold')] - const label = () => button.querySelector('[data-fold-all-label]')?.textContent // A section summary is never hidden by its own fold, so focus must not move. const summary = folds[0]!.querySelector(':scope > summary')! summary.focus() + const button = doc.querySelector('[data-section-fold-all="unfold"]')! button.click() - expect(doc.activeElement).toBe(summary) - - // Mixed state: any open section means the global control folds all. - folds[1]!.open = true - expect(label()).toBe('Fold all') - button.click() - expect(folds.every((fold) => !fold.open)).toBe(true) - expect(label()).toBe('Unfold all') + expect(doc.activeElement).toBe(button) - button.click() + const secondFile = folds[1]!.querySelector('details.file')! + secondFile.open = true + folds[0]!.querySelector('[data-section-fold-all="fold"]')!.click() expect(folds.every((fold) => fold.open)).toBe(true) - expect(label()).toBe('Fold all') + expect(secondFile.open).toBe(true) }, 120000, ) diff --git a/test/report.test.ts b/test/report.test.ts index 2515c7f..5b74f4a 100644 --- a/test/report.test.ts +++ b/test/report.test.ts @@ -338,16 +338,16 @@ describe('renderReport shell', () => { expect(html).toContain('value="unified" checked') }) - test('the review map carries one global fold control after the layout toggle', () => { + test('each open section has local fold controls and files start collapsed', () => { const html = renderReport(document([section(simplePatch(), 'Plain')]), stubClient) - const form = html.indexOf('data-layout-form') - const fold = html.indexOf('data-fold-all') - const label = html.indexOf('>Review map<') - expect(fold).toBeGreaterThan(form) - expect(label).toBeGreaterThan(fold) - expect(html).toContain('aria-label="Fold all review sections"') - expect(html).toContain('data-fold-all-label>Fold all<') + expect(html).not.toContain('data-fold-all') + expect(html).toContain('data-section-fold-all="unfold"') + expect(html).toContain('data-section-fold-all="fold"') + expect(html).toContain('aria-label="Unfold all in section 01"') + expect(html).toContain('aria-label="Fold all in section 01"') + expect(html).toContain('
') + expect(html).toContain('
') }) test('review map lists every section in document order with zero-padded anchors and counts', () => { diff --git a/test/visual/__screenshots__/desktop/focused-control.png b/test/visual/__screenshots__/desktop/focused-control.png index a8616ac..7741f12 100644 Binary files a/test/visual/__screenshots__/desktop/focused-control.png and b/test/visual/__screenshots__/desktop/focused-control.png differ diff --git a/test/visual/__screenshots__/desktop/print.png b/test/visual/__screenshots__/desktop/print.png index 921b90d..66dafac 100644 Binary files a/test/visual/__screenshots__/desktop/print.png and b/test/visual/__screenshots__/desktop/print.png differ diff --git a/test/visual/__screenshots__/desktop/report.png b/test/visual/__screenshots__/desktop/report.png index eabb9d2..064ac43 100644 Binary files a/test/visual/__screenshots__/desktop/report.png and b/test/visual/__screenshots__/desktop/report.png differ diff --git a/test/visual/__screenshots__/desktop/review-map.png b/test/visual/__screenshots__/desktop/review-map.png index aebb794..ac92a42 100644 Binary files a/test/visual/__screenshots__/desktop/review-map.png and b/test/visual/__screenshots__/desktop/review-map.png differ diff --git a/test/visual/__screenshots__/desktop/section.png b/test/visual/__screenshots__/desktop/section.png index 7c206ee..64afbb8 100644 Binary files a/test/visual/__screenshots__/desktop/section.png and b/test/visual/__screenshots__/desktop/section.png differ diff --git a/test/visual/__screenshots__/narrow/focused-control.png b/test/visual/__screenshots__/narrow/focused-control.png index cc7c496..bc1d309 100644 Binary files a/test/visual/__screenshots__/narrow/focused-control.png and b/test/visual/__screenshots__/narrow/focused-control.png differ diff --git a/test/visual/__screenshots__/narrow/print.png b/test/visual/__screenshots__/narrow/print.png index 21c3c41..01a6df9 100644 Binary files a/test/visual/__screenshots__/narrow/print.png and b/test/visual/__screenshots__/narrow/print.png differ diff --git a/test/visual/__screenshots__/narrow/report.png b/test/visual/__screenshots__/narrow/report.png index 2becf0c..9173ab0 100644 Binary files a/test/visual/__screenshots__/narrow/report.png and b/test/visual/__screenshots__/narrow/report.png differ diff --git a/test/visual/__screenshots__/narrow/review-map.png b/test/visual/__screenshots__/narrow/review-map.png index d15d8b9..6085444 100644 Binary files a/test/visual/__screenshots__/narrow/review-map.png and b/test/visual/__screenshots__/narrow/review-map.png differ diff --git a/test/visual/__screenshots__/narrow/section.png b/test/visual/__screenshots__/narrow/section.png index ee46776..2716a0d 100644 Binary files a/test/visual/__screenshots__/narrow/section.png and b/test/visual/__screenshots__/narrow/section.png differ diff --git a/test/visual/__screenshots__/phone/focused-control.png b/test/visual/__screenshots__/phone/focused-control.png index d84b697..38a9836 100644 Binary files a/test/visual/__screenshots__/phone/focused-control.png and b/test/visual/__screenshots__/phone/focused-control.png differ diff --git a/test/visual/__screenshots__/phone/print.png b/test/visual/__screenshots__/phone/print.png index b620a7e..31baafa 100644 Binary files a/test/visual/__screenshots__/phone/print.png and b/test/visual/__screenshots__/phone/print.png differ diff --git a/test/visual/__screenshots__/phone/report.png b/test/visual/__screenshots__/phone/report.png index 9dc5f78..646e085 100644 Binary files a/test/visual/__screenshots__/phone/report.png and b/test/visual/__screenshots__/phone/report.png differ diff --git a/test/visual/__screenshots__/phone/review-map.png b/test/visual/__screenshots__/phone/review-map.png index b1df8c8..e601c77 100644 Binary files a/test/visual/__screenshots__/phone/review-map.png and b/test/visual/__screenshots__/phone/review-map.png differ diff --git a/test/visual/__screenshots__/phone/section.png b/test/visual/__screenshots__/phone/section.png index 8e149fd..e586417 100644 Binary files a/test/visual/__screenshots__/phone/section.png and b/test/visual/__screenshots__/phone/section.png differ diff --git a/test/visual/__screenshots__/tablet/focused-control.png b/test/visual/__screenshots__/tablet/focused-control.png index ce398f9..838a5db 100644 Binary files a/test/visual/__screenshots__/tablet/focused-control.png and b/test/visual/__screenshots__/tablet/focused-control.png differ diff --git a/test/visual/__screenshots__/tablet/print.png b/test/visual/__screenshots__/tablet/print.png index e340358..a9271fb 100644 Binary files a/test/visual/__screenshots__/tablet/print.png and b/test/visual/__screenshots__/tablet/print.png differ diff --git a/test/visual/__screenshots__/tablet/report.png b/test/visual/__screenshots__/tablet/report.png index a431cbe..2291de1 100644 Binary files a/test/visual/__screenshots__/tablet/report.png and b/test/visual/__screenshots__/tablet/report.png differ diff --git a/test/visual/__screenshots__/tablet/review-map.png b/test/visual/__screenshots__/tablet/review-map.png index fc0fd0c..4d7b3bd 100644 Binary files a/test/visual/__screenshots__/tablet/review-map.png and b/test/visual/__screenshots__/tablet/review-map.png differ diff --git a/test/visual/__screenshots__/tablet/section.png b/test/visual/__screenshots__/tablet/section.png index 2578aa7..667d984 100644 Binary files a/test/visual/__screenshots__/tablet/section.png and b/test/visual/__screenshots__/tablet/section.png differ diff --git a/test/visual/report.visual.ts b/test/visual/report.visual.ts index 0bbdf89..b1d1324 100644 --- a/test/visual/report.visual.ts +++ b/test/visual/report.visual.ts @@ -59,20 +59,26 @@ test.describe('report visuals', () => { await expect(page.locator('nav')).toHaveScreenshot('review-map.png') }) - // Keyboard focus styling was previously asserted as the `.fold-all:focus-visible` - // stylesheet text. Tab reaches the fold control first, so a keyboard-driven focus + // Keyboard focus styling is asserted on the section-local fold control. + // Tab reaches the section control first, so a keyboard-driven focus // captures the real ring instead of the rule that draws it. test('keyboard focus ring on the fold control', async ({ page }) => { + await page.locator('.section-title-text').first().focus() await page.keyboard.press('Tab') const focused = await page.evaluate(() => document.activeElement?.className ?? '') - if (!focused.includes('fold-all')) { + if (!focused.includes('section-fold-all')) { throw new Error(`Expected the fold control to receive focus, got ${focused}`) } - await expect(page.locator('nav')).toHaveScreenshot('focused-control.png') + await expect(page.locator('main .section').first()).toHaveScreenshot('focused-control.png') }) // One section captures the title row, prose measure, and the file fold. test('section', async ({ page }) => { + await page.locator('main .section').first().locator('details.section-fold').evaluate((element) => { + ;(element as HTMLDetailsElement).open = true + for (const detail of element.querySelectorAll('details.file')) detail.open = true + }) + await expect(page.locator('main .section').first().locator('.step-text')).toBeVisible() await expect(page.locator('main .section').first()).toHaveScreenshot('section.png') }) @@ -116,6 +122,10 @@ test.describe('report visuals', () => { }) test('print report', async ({ page }) => { + await page.locator('details').evaluateAll((details) => { + for (const detail of details) (detail as HTMLDetailsElement).open = true + }) + await expect(page.locator('main .step-text').first()).toBeVisible() await page.emulateMedia({ media: 'print' }) await expect(page).toHaveScreenshot('print.png', { fullPage: true }) })