From c21af9acdd6a08a9c0e58df90873744a1cd46c7f Mon Sep 17 00:00:00 2001 From: Hoang Pham Date: Wed, 16 Sep 2026 16:40:23 +0700 Subject: [PATCH] feat(comparison): open rendered changes in full documents Keep Changes when there are no rendered differences. Limit initial positioning, stop on reader input, and show documents before measuring navigation targets. Assisted-by: Codex:gpt-6-astra Signed-off-by: Hoang Pham --- playwright/comparison/comparison.spec.ts | 119 +++++++++++- .../comparison/support/comparisonHarness.ts | 9 +- src/components/MarkdownContentComparison.vue | 95 ++++++++-- .../createMarkdownContentComparison.spec.ts | 172 +++++++++++++++++- 4 files changed, 366 insertions(+), 29 deletions(-) diff --git a/playwright/comparison/comparison.spec.ts b/playwright/comparison/comparison.spec.ts index 2e4eab7f681..2b363fea2f9 100644 --- a/playwright/comparison/comparison.spec.ts +++ b/playwright/comparison/comparison.spec.ts @@ -24,9 +24,98 @@ const retainedTableCellEdit: ComparisonContents = { } test.describe('Text comparison production bundle acceptance', () => { + test('chooses the initial view from rendered edits without taking focus', async ({ comparison, page }) => { + await page.evaluate(() => { + const opener = document.createElement('button') + opener.id = 'comparison-opener' + opener.textContent = 'Open comparison' + document.body.append(opener) + opener.focus() + }) + for (const [before, after, view] of [ + ['Before', 'After', 'Full documents'], + ['Same text', '**Same text**', 'Full documents'], + ['Same text', 'Same text', 'Changes'], + ['*Same text*', '_Same text_', 'Changes'], + ]) { + await comparison.mount({ before, after, detached: true }) + await expect(page.getByRole('tab', { name: view, exact: true })).toHaveAttribute('aria-selected', 'true') + await expect(page.locator('#comparison-opener')).toBeFocused() + if (view === 'Full documents') { + await expect(page.locator('.text-comparison__documents .ProseMirror')).toHaveCount(2) + await expect(page.locator('[data-comparison-change][aria-current="true"]').first()).toBeVisible() + } else { + await expect(page.locator('.text-comparison__changes [role="status"]')).toBeVisible() + } + await comparison.destroy() + } + }) + + for (const interrupted of [false, true]) { + test(`browser scroll anchoring preserves the settled image comparison position (reader interrupted: ${interrupted})`, async ({ comparison, page }) => { + let release!: () => void + const imageReady = new Promise((resolve) => { + release = resolve + }) + await page.route('**/comparison-opening-image.svg', async (route) => { + await imageReady + await route.fulfill({ contentType: 'image/svg+xml', body: '' }) + }) + const prefix = `![Opening image](/comparison-opening-image.svg)\n\n${Array.from({ length: 80 }, (_value, index) => `Unchanged paragraph ${index}.`).join('\n\n')}` + try { + await comparison.mount({ before: `${prefix}\n\nBefore ending.`, after: `${prefix}\n\nAfter ending.`, detached: true, height: 620 }) + const scroller = page.locator('.text-comparison__document--before .text-comparison__document-scroller') + if (interrupted) { + await scroller.hover() + await page.mouse.wheel(0, -100_000) + await expect.poll(() => scroller.evaluate((element) => element.scrollTop)).toBe(0) + } + await page.evaluate(async () => { + for (let frame = 0; frame < 4; frame++) { + await new Promise(requestAnimationFrame) + } + }) + release() + await expect(page.locator('.text-comparison__document--before img').first()).toHaveJSProperty('naturalHeight', 1600) + if (interrupted) { + await expect.poll(() => scroller.evaluate((element) => element.scrollTop)).toBe(0) + } else { + await expect.poll(() => scroller.evaluate((element) => { + const target = element.querySelector('[data-comparison-change][aria-current="true"]')!.getBoundingClientRect() + const viewport = element.getBoundingClientRect() + return target.top >= viewport.top && target.bottom <= viewport.bottom + })).toBe(true) + } + } finally { + release() + } + }) + } + + for (const width of [1100, 620]) { + test(`opens the first edit after detached child transfer at ${width}px`, async ({ comparison, page }, testInfo) => { + const prefix = Array.from({ length: 80 }, (_value, index) => `Unchanged paragraph ${index}.`).join('\n\n') + const measurement = await comparison.mount({ before: `${prefix}\n\nBefore ending.`, after: `${prefix}\n\nAfter ending.`, detached: true, width, height: 620 }) + await expect(page.getByRole('tab', { name: 'Full documents', exact: true })).toHaveAttribute('aria-selected', 'true') + const geometry = async () => page.locator('.text-comparison__document:visible').evaluateAll((panes) => panes.map((pane) => { + const scroller = pane.querySelector('.text-comparison__document-scroller')! + const target = pane.querySelector('[data-comparison-change][aria-current="true"]')! + const viewport = scroller.getBoundingClientRect() + const change = target.getBoundingClientRect() + return { top: change.top, bottom: change.bottom, viewportTop: viewport.top, viewportBottom: viewport.bottom, scrollTop: scroller.scrollTop } + })) + await testInfo.attach('opening-geometry.json', { body: JSON.stringify({ measurement, panes: await geometry() }), contentType: 'application/json' }) + await expect.poll(async () => (await geometry()).every((pane) => pane.top >= pane.viewportTop && pane.bottom <= pane.viewportBottom)).toBe(true) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() + await page.locator('[data-comparison-select]').first().click() + await expect.poll(async () => (await geometry()).every((pane) => pane.top >= pane.viewportTop && pane.bottom <= pane.viewportBottom)).toBe(true) + }) + } + test('A12: the largest admitted square gap remains precise', async ({ comparison, page }) => { test.setTimeout(180_000) await mountMaximumSquare(comparison) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await expect(page.locator('.text-comparison > [data-comparison-source-fallback]')).toHaveCount(0) await expect(page.locator('[data-comparison-select]')).toHaveCount(80) @@ -35,6 +124,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('T07: one edited retained table column is precise at cell altitude', async ({ comparison, page }) => { await comparison.mount(retainedTableCellEdit) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const change = page.locator('[data-comparison-select]') await expect(change).toHaveCount(1) @@ -48,6 +138,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('T18: a later over-budget table coarsens without corrupting the admitted table plan', async ({ comparison, page }) => { test.setTimeout(180_000) await comparison.mount(tableLedgerFixture()) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const changes = page.locator('[data-comparison-select]') const pages = page.getByRole('navigation', { name: 'Change pages' }) @@ -65,6 +156,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V01: one first-class edit owns one row, ordinal, identity, and complete target set', async ({ comparison, page }) => { await comparison.mount(headingReplacement) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const row = page.locator('[data-comparison-select]') await expect(row).toHaveCount(1) @@ -93,6 +185,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V03: selecting a Changes row activates the identical edit in both Documents panes', async ({ comparison, page }) => { await comparison.mount({ before: 'Old first.\n\nOld second.', after: 'New first.\n\nNew second.' }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const selectedEdit = page.locator('[data-comparison-select]').nth(1) await selectedEdit.click() @@ -107,6 +200,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V04: an empty-side change has no synthetic marker and remains navigable', async ({ comparison, page }) => { await comparison.mount({ before: '# Removed first\n\n# Removed second', after: '' }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').first().click() await expect(page.locator('.text-comparison__document--after [data-comparison-change]')).toHaveCount(0) @@ -121,6 +215,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V05: paired Documents panes preserve independent scroll positions', async ({ comparison, page }) => { const paragraphs = Array.from({ length: 100 }, (_, index) => `Paragraph ${index}.`).join('\n\n') await comparison.mount({ before: `Old first.\n\n${paragraphs}\n\nOld tail.`, after: `New first.\n\n${paragraphs}\n\nNew tail.` }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').first().click() const beforeScroller = page.locator('.text-comparison__document--before .text-comparison__document-scroller') @@ -138,6 +233,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V06: responsive single-pane Documents retain side and selection state', async ({ comparison, page }) => { await comparison.mount({ before: 'Old first.\n\nOld second.', after: 'New first.\n\nNew second.', width: 620 }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').nth(1).click() await expect(page.locator('.text-comparison')).toHaveClass(/text-comparison--single/) @@ -162,7 +258,6 @@ test.describe('Text comparison production bundle acceptance', () => { test('AUD-24: narrow Documents show the side that contains a one-sided edit', async ({ comparison, page }) => { await comparison.mount({ before: '', after: '# Added first\n\n# Added second', width: 620 }) - await page.locator('[data-comparison-select]').first().click() const sideTabs = page.getByRole('tablist', { name: 'Version to display' }) await expect(sideTabs.getByRole('tab', { name: 'After' })).toHaveAttribute('aria-selected', 'true') await expect(page.locator('.text-comparison__document--after [data-comparison-change][aria-current="true"]')).toBeVisible() @@ -170,7 +265,6 @@ test.describe('Text comparison production bundle acceptance', () => { await comparison.destroy() await comparison.mount({ before: '# Removed first\n\n# Removed second', after: '', width: 620 }) - await page.locator('[data-comparison-select]').first().click() const deletionTabs = page.getByRole('tablist', { name: 'Version to display' }) await deletionTabs.getByRole('tab', { name: 'After' }).click() await page.getByRole('tab', { name: 'Changes' }).click() @@ -192,6 +286,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V06b: desktop Changes rows keep the reviewed full-width list presentation', async ({ comparison, page }) => { await comparison.mount({ before: '# Old heading\n\nOld paragraph.', after: '# New heading\n\nNew paragraph.', width: 1100 }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const host = page.locator('#text-comparison-harness') const section = page.locator('.text-comparison__section-toggle').first() @@ -209,6 +304,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('V07: tabs, navigation, focus, and announcements expose accessible state', async ({ comparison, page }) => { await comparison.mount({ before: 'Old first.\n\nOld second.', after: 'New first.\n\nNew second.' }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await comparison.assertAccessibleComparison() await page.locator('[data-comparison-select]').first().click() @@ -232,6 +328,7 @@ test.describe('Text comparison production bundle acceptance', () => { before: `![Before logo](${CORE_LOGO})\n\nOld paragraph.`, after: `![After logo](${CORE_LOGO})\n\nNew paragraph.`, }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const changes = page.locator('[data-comparison-select]') await expect(changes).toHaveCount(2) await changes.first().click() @@ -263,6 +360,7 @@ test.describe('Text comparison production bundle acceptance', () => { before: '| Name |\n| --- |\n| retained |\n| removed |', after: '| Name |\n| --- |\n| retained |', }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').click() const structuralRow = page.locator('tr.text-comparison-change') await expect(structuralRow).toHaveCount(1) @@ -315,7 +413,7 @@ test.describe('Text comparison production bundle acceptance', () => { for (let iteration = 0; iteration < 2; iteration++) { const measurement = await comparison.mount({ before: `Before ${iteration}`, after: `After ${iteration}` }) expect(measurement.rootCount).toBe(1) - expect(measurement.proseMirrorCount).toBe(0) + expect(measurement.proseMirrorCount).toBe(2) await page.getByRole('tab', { name: 'Full documents' }).click() await expect(page.locator('.ProseMirror')).toHaveCount(2) await comparison.destroy(2) @@ -343,7 +441,6 @@ test.describe('Text comparison production bundle acceptance', () => { test('F06: projection failure mounts Source without partial Documents', async ({ comparison, page }) => { await comparison.forceProjectionFailure() await comparison.mount({ before: 'Projection before', after: 'Projection after' }) - await page.getByRole('tab', { name: 'Full documents' }).click() const fallback = page.locator('[data-comparison-source-fallback]') await expect(fallback).toContainText('Projection before') @@ -355,6 +452,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('F11: normal comparison modes emit no unexplained browser or network failures', async ({ comparison, page }) => { comparison.resetCapture() await comparison.mount({ before: 'Old content.', after: '**New content.**' }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').click() await page.getByRole('tab', { name: 'Markdown source' }).click() await expect(page.locator('[data-source-hunk]')).toBeVisible() @@ -372,9 +470,10 @@ test.describe('Text comparison production bundle acceptance', () => { await attachMeasurement(testInfo, 'near-line-floor', measurement, { weightedDebit: 0 }) }) - test('AUD-02: pre-mount selection and filtering initialize both Documents decoration plugins', async ({ comparison, page }) => { + test('AUD-02: selection and filtering update both mounted Documents decoration plugins', async ({ comparison, page }) => { for (const width of [1000, 620]) { await comparison.mount({ before: 'Old first.\n\nOld second.', after: 'New first.\n\nNew second.', width }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').nth(1).click() for (const side of ['before', 'after']) { await expect(page.locator(`.text-comparison__document--${side} [data-comparison-change="change-1"][aria-current="true"]`)).toHaveCount(1) @@ -382,6 +481,7 @@ test.describe('Text comparison production bundle acceptance', () => { await comparison.destroy() await comparison.mount({ before: 'Formatting only.\n\nOld content.', after: '**Formatting only.**\n\nNew content.', width }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.getByRole('checkbox', { name: 'Hide formatting-only changes' }).check() await page.getByRole('tab', { name: 'Full documents' }).click() await expect(page.locator('.text-comparison__documents .text-comparison-change--formatting')).toHaveCount(0) @@ -410,6 +510,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('AUD-09: settled image dialog focus is contained and restored on close', async ({ comparison, page }, testInfo) => { await comparison.mount({ before: `![Before logo](${CORE_LOGO})`, after: `![After logo](${CORE_LOGO})` }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').first().click() const action = page.getByRole('button', { name: 'Open image Before logo' }) await action.focus() @@ -429,6 +530,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('AUD-10: Changes tokens wrap with spacing and selected tabs have visible treatment', async ({ comparison, page }) => { await comparison.mount({ before: 'A short value.', after: '**A substantially longer changed value that must remain readable.**' }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const item = page.locator('.text-comparison__change-item') const content = item.locator('.text-comparison__change-item-content') @@ -467,6 +569,7 @@ test.describe('Text comparison production bundle acceptance', () => { after: '# B1 duplicate-body deletion\n\n| A | B |\n| --- | --- |\n| x | x |\n| x | x |', width: 340, }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await expect(page.locator('.text-comparison')).toHaveClass(/text-comparison--single/) const narrowItem = page.locator('.text-comparison__change-item').first() const narrowLabel = narrowItem.getByText('Table column removed', { exact: true }) @@ -490,6 +593,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('AUD-11: revealing a responsive hidden side locates the already-current edit', async ({ comparison, page }) => { const middle = Array.from({ length: 120 }, (_, index) => `Stable paragraph ${index}.`).join('\n\n') await comparison.mount({ before: `Old first.\n\n${middle}\n\nOld tail.`, after: `New first.\n\n${middle}\n\nNew tail.`, width: 620, height: 360 }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').nth(1).click() await expect(page.locator('.text-comparison')).toHaveClass(/text-comparison--single/) @@ -562,6 +666,7 @@ test.describe('Text comparison production bundle acceptance', () => { test('AUD-18: read-only image action is named, focusable, rendered, and operable with Enter', async ({ comparison, page }) => { await comparison.mount({ before: `![Before logo](${CORE_LOGO})`, after: `![After logo](${CORE_LOGO})` }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').first().click() const action = page.getByRole('button', { name: 'Open image Before logo' }) @@ -588,6 +693,7 @@ test.describe('Text comparison production bundle acceptance', () => { }) }) await comparison.mount({ before: '![Before document](.attachments.123/document.pdf)', after: '![After document](.attachments.123/document.pdf)', fileId: 123 }) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() await page.locator('[data-comparison-select]').first().click() const action = page.getByRole('button', { name: 'Open attachment Before document' }) @@ -605,7 +711,7 @@ test.describe('Text comparison production bundle acceptance', () => { await expect(page.locator('.text-comparison')).toBeVisible() await expect(page.locator('[data-comparison-source-fallback]')).toHaveCount(0) await expect(page.locator('[data-comparison-select]')).toHaveCount(1) - await expect(page.locator('.ProseMirror')).toHaveCount(0) + await expect(page.locator('.ProseMirror')).toHaveCount(2) }) test('AUD-22: complete Source fallback responds to host width instead of viewport width', async ({ comparison, page }) => { @@ -721,6 +827,7 @@ test.describe('Text comparison production bundle acceptance', () => { async function assertFormattingFilterMove(comparison: ComparisonHarness, page: Page, contents: ComparisonContents, direction: 'next' | 'previous') { await comparison.mount(contents) + await page.getByRole('tab', { name: 'Changes', exact: true }).click() const rows = page.locator('[data-comparison-select]') const formatting = rows.filter({ hasText: /Bold changed/ }) await expect(formatting).toHaveCount(1) diff --git a/playwright/comparison/support/comparisonHarness.ts b/playwright/comparison/support/comparisonHarness.ts index 201a0a7b75e..43d3c5c75cb 100644 --- a/playwright/comparison/support/comparisonHarness.ts +++ b/playwright/comparison/support/comparisonHarness.ts @@ -17,6 +17,7 @@ export interface ComparisonContents { rejectLoaded?: boolean width?: number height?: number + detached?: boolean } export interface ComparisonMeasurement { @@ -177,17 +178,18 @@ export class ComparisonHarness { if (contents.rejectLoaded) { this.allowFailure(/acceptance forced loaded callback failure/) } - return this.page.evaluate(async ({ before, after, fileId, rejectLoaded = false, width = 1100, height = 760 }) => { + return this.page.evaluate(async ({ before, after, fileId, rejectLoaded = false, width = 1100, height = 760, detached = false }) => { const state = window.__textComparisonAcceptance const host = document.querySelector('#text-comparison-harness')! host.style.inlineSize = `${width}px` host.style.blockSize = `${height}px` const started = performance.now() let loadedCallbackCalls = 0 + const mount = detached ? document.createElement('div') : host const instance = await window.OCA.Text.createMarkdownContentComparison({ afterContent: after, beforeContent: before, - el: host, + el: mount, fileId, noLazyImages: true, onLoaded: rejectLoaded @@ -197,6 +199,9 @@ export class ComparisonHarness { } : undefined, }) + if (detached) { + host.replaceChildren(...mount.childNodes) + } const durationMilliseconds = performance.now() - started state.instances.push(instance) await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve()))) diff --git a/src/components/MarkdownContentComparison.vue b/src/components/MarkdownContentComparison.vue index a7ed89846fa..c0fd40f863f 100644 --- a/src/components/MarkdownContentComparison.vue +++ b/src/components/MarkdownContentComparison.vue @@ -8,7 +8,11 @@ ref="root" class="text-comparison" :class="`text-comparison--${layoutMode}`" - :aria-label="t('text', 'Version comparison')"> + :aria-label="t('text', 'Version comparison')" + @wheel.passive="cancelInitialLocation" + @touchstart.passive="cancelInitialLocation" + @pointerdown="cancelInitialLocation" + @keydown="cancelInitialLocation">

{{ announcement }}

@@ -237,6 +241,10 @@ const SourceView = shallowRef(null) let observer: ResizeObserver | null = null let didReady = false let destroyed = false +let pendingInitialLocation = false +let initialLocationFrame: number | null = null +let initialGeometry = '' +let initialLocationAttempts = 0 const resolver = props.fileId ? new AttachmentResolver({ @@ -296,6 +304,11 @@ try { plugins[side] = [decoration.plugin] pluginKeys[side] = decoration.key } + if (model.edits.length) { + view.value = 'documents' + showDocuments.value = true + pendingInitialLocation = true + } } catch { activateFallback() } @@ -321,7 +334,10 @@ watch(activeIds, (ids) => { ) updateDecorations() }) -watch(currentId, refreshDocuments) +watch(currentId, () => { + updateDecorations() + locateCurrent(true) +}) watch(view, (next) => { if (next === 'source' && !SourceView.value) { loadSource() @@ -340,20 +356,24 @@ onMounted(() => { = (entry?.contentRect.width ?? root.value?.clientWidth ?? 760) < 760 ? 'single' : 'paired' - if (nextLayout === layoutMode.value) { - return - } + const changed = nextLayout !== layoutMode.value layoutMode.value = nextLayout - locateCurrent(true) + if (pendingInitialLocation) { + requestInitialLocation() + } else if (changed) { + locateCurrent(true) + } }) } if (root.value) { observer?.observe(root.value) } + requestInitialLocation() ready() }) onBeforeUnmount(() => { destroyed = true + cancelInitialLocation() observer?.disconnect() observer = null destroyEditors() @@ -380,22 +400,72 @@ function updateDecorations() { } function refreshDocuments() { updateDecorations() - locateCurrent(true) + requestInitialLocation() +} +/** + * End the opening phase and discard queued measurements. + */ +function cancelInitialLocation() { + pendingInitialLocation = false + if (initialLocationFrame !== null) { + cancelAnimationFrame(initialLocationFrame) + initialLocationFrame = null + } +} +/** + * Locate after usable attachment, allowing at most three opening layout corrections. + */ +function requestInitialLocation() { + if (!pendingInitialLocation || initialLocationFrame !== null || typeof requestAnimationFrame === 'undefined') { + return + } + nextTick(() => { + if (!pendingInitialLocation || initialLocationFrame !== null) { + return + } + initialLocationFrame = requestAnimationFrame(async () => { + initialLocationFrame = null + if (!pendingInitialLocation || !root.value?.isConnected) { + return + } + const visible = sides.filter((side) => layoutMode.value === 'paired' || activeSide.value === side) + if (visible.some((side) => { + const scroller = sideElements[side].scroller + return !scroller?.clientWidth || !scroller.clientHeight || !editors[side]?.view.dom.isConnected + })) { + return + } + const geometry = visible.map((side) => { + const scroller = sideElements[side].scroller! + return `${side}:${scroller.clientWidth}:${scroller.clientHeight}:${scroller.scrollHeight}` + }).join('|') + if (geometry === initialGeometry || initialLocationAttempts === 3) { + cancelInitialLocation() + return + } + initialGeometry = geometry + initialLocationAttempts++ + await locateCurrent(true, 'auto') + requestInitialLocation() + }) + }) } function selectEdit(id: string) { + cancelInitialLocation() currentId.value = id if (!isPureFormatting(model.edits.find((edit) => edit.id === id)!)) { setView('documents') } } function move(offset: number) { + cancelInitialLocation() currentId.value = moveCurrentId( activeIds.value, currentId.value, offset, ) } -function locateCurrent(selectVisibleSide = false) { +function locateCurrent(selectVisibleSide = false, behavior = scrollBehavior()) { const edit = model.edits.find(({ id }) => id === currentId.value) if (!edit) { return @@ -408,7 +478,7 @@ function locateCurrent(selectVisibleSide = false) { activeSide.value = other } } - nextTick(() => { + return nextTick(() => { for (const side of ['before', 'after'] as const) { const editor = editors[side] const { pane, scroller } = sideElements[side] @@ -417,7 +487,7 @@ function locateCurrent(selectVisibleSide = false) { pane, scroller, edit.primary.id, - scrollBehavior(), + behavior, () => { if (!editor || editor.isDestroyed) { return null @@ -436,6 +506,7 @@ function ready() { } } function activateFallback() { + cancelInitialLocation() failure.value = true destroyEditors() } @@ -453,14 +524,16 @@ async function loadSource() { } } function setView(next: View) { + cancelInitialLocation() + view.value = next if (next === 'documents') { showDocuments.value = true locateCurrent(true) } - view.value = next nextTick(() => tabRefs.get(next)?.focus()) } function setSide(side: Side) { + cancelInitialLocation() activeSide.value = side locateCurrent() nextTick(() => sideElements[side].tab?.focus()) diff --git a/src/tests/comparison/createMarkdownContentComparison.spec.ts b/src/tests/comparison/createMarkdownContentComparison.spec.ts index 53c3fd57479..76186f1b130 100644 --- a/src/tests/comparison/createMarkdownContentComparison.spec.ts +++ b/src/tests/comparison/createMarkdownContentComparison.spec.ts @@ -27,9 +27,142 @@ afterAll(() => { } }) -afterEach(() => vi.restoreAllMocks()) +afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) describe('Markdown comparison factory fallback and lifecycle', () => { + it.each([1100, 620].flatMap((width) => ['wheel', 'touchstart', 'pointerdown', 'keydown', 'settled geometry', 'three attempts'] + .map((stop) => ({ width, stop }))))('waits for usable attachment at $width px and stops opening corrections on $stop', async ({ width: initialWidth, stop }) => { + let width = initialWidth + let height = 0 + let resize!: () => void + const disconnected = vi.fn() + const frames = new Map() + let frameId = 0 + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + frames.set(++frameId, callback) + return frameId + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => frames.delete(id)) + vi.stubGlobal('ResizeObserver', class { + constructor(private readonly callback: ResizeObserverCallback) {} + + observe(target: Element) { + resize = () => this.callback([{ target, contentRect: { width, height } } as ResizeObserverEntry], this) + } + + disconnect = disconnected + unobserve() {} + }) + vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(function(this: HTMLElement) { + return this.isConnected ? width : 0 + }) + vi.spyOn(HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(function(this: HTMLElement) { + return this.isConnected ? height : 0 + }) + vi.spyOn(HTMLElement.prototype, 'scrollHeight', 'get').mockReturnValue(2000) + const scroll = vi.spyOn(HTMLElement.prototype, 'scrollTo').mockClear() + const frame = async () => { + await nextTick() + const queued = [...frames.values()] + frames.clear() + for (const callback of queued) { + await callback(0) + } + await nextTick() + } + const detached = document.createElement('div') + const host = document.createElement('div') + const instance = await createMarkdownContentComparison({ beforeContent: 'Before', afterContent: 'After', el: detached }) + try { + resize() + await frame() + expect(scroll).not.toHaveBeenCalled() + document.body.append(host) + host.replaceChildren(...detached.childNodes) + resize() + await frame() + expect(scroll).not.toHaveBeenCalled() + + height = 300 + resize() + await frame() + expect(scroll).toHaveBeenCalled() + expect(scroll.mock.calls.every(([options]) => (options as ScrollToOptions).behavior === 'auto')).toBe(true) + const firstCalls = scroll.mock.calls.length + width -= 40 + height = 260 + resize() + await frame() + expect(scroll).toHaveBeenCalledTimes(firstCalls * 2) + if (initialWidth < 760) { + const hidden = host.querySelector('.text-comparison__document--after .text-comparison__document-scroller') + expect(scroll.mock.contexts).not.toContain(hidden) + } + + if (stop === 'three attempts') { + height = 240 + resize() + await frame() + expect(scroll).toHaveBeenCalledTimes(firstCalls * 3) + height = 220 + resize() + await frame() + expect(scroll).toHaveBeenCalledTimes(firstCalls * 3) + } else if (stop === 'settled geometry') { + await frame() + expect(scroll).toHaveBeenCalledTimes(firstCalls * 2) + } else { + host.querySelector('.text-comparison')!.dispatchEvent(new Event(stop)) + } + expect(frames.size).toBe(0) + const callsAfterStop = scroll.mock.calls.length + height = 240 + resize() + await frame() + expect(scroll).toHaveBeenCalledTimes(callsAfterStop) + instance.destroy() + await frame() + expect(frames.size).toBe(0) + expect(disconnected).toHaveBeenCalledOnce() + } finally { + instance.destroy() + host.remove() + } + }) + + it.each([ + ['ordinary', 'Before', 'After'], + ['formatting-only', 'Same text', '**Same text**'], + ])('opens %s changes in Documents without moving focus', async (_kind, beforeContent, afterContent) => { + const opener = document.createElement('button') + document.body.append(opener) + opener.focus() + const el = document.createElement('div') + const onLoaded = vi.fn() + const instance = await createMarkdownContentComparison({ beforeContent, afterContent, el, onLoaded }) + + expect(el.querySelector('[role="tab"][aria-selected="true"]')?.textContent?.trim()).toBe('Full documents') + expect(el.querySelectorAll('.text-comparison__documents .ProseMirror')).toHaveLength(2) + expect(el.querySelector('[data-comparison-change][aria-current="true"]')).not.toBeNull() + expect(document.activeElement).toBe(opener) + expect(onLoaded).toHaveBeenCalledOnce() + instance.destroy() + instance.destroy() + opener.remove() + }) + + it('opens identical documents in Changes with the no-differences message', async () => { + const el = document.createElement('div') + const instance = await createMarkdownContentComparison({ beforeContent: 'Same', afterContent: 'Same', el }) + expect(el.querySelector('[role="tab"][aria-selected="true"]')?.textContent?.trim()).toBe('Changes') + expect(el.querySelector('[role="status"]')?.textContent).toContain('No differences.') + expect(el.querySelectorAll('.ProseMirror')).toHaveLength(0) + instance.destroy() + }) + it('V09 reports syntax-only Markdown as no semantic edit and opens Source', async () => { const el = document.createElement('div') const instance = await createMarkdownContentComparison({ @@ -77,10 +210,6 @@ describe('Markdown comparison factory fallback and lifecycle', () => { const afterContent = 'Complete projection after' const el = document.createElement('div') const instance = await createMarkdownContentComparison({ beforeContent, afterContent, el }) - const fullDocuments = [...el.querySelectorAll('[role="tab"]')] - .find(({ textContent }) => textContent?.trim() === 'Full documents') - - fullDocuments!.click() await vi.waitFor(() => { expect(el.querySelector('[data-comparison-source-fallback]')).not.toBeNull() }) @@ -104,7 +233,8 @@ describe('Markdown comparison factory fallback and lifecycle', () => { it('keeps both document editors alive while switching views', async () => { const el = document.createElement('div') const instance = await createMarkdownContentComparison({ beforeContent: 'Before', afterContent: 'After', el }) - expect(el.querySelectorAll('.ProseMirror')).toHaveLength(0) + const editors = [...el.querySelectorAll('.ProseMirror')] + expect(editors).toHaveLength(2) expect(el.querySelectorAll('[data-comparison-source-fallback]')).toHaveLength(0) const selectTab = async (label: string) => { const tab = [...el.querySelectorAll('[role="tab"]')] @@ -114,16 +244,38 @@ describe('Markdown comparison factory fallback and lifecycle', () => { await nextTick() } - await selectTab('Full documents') - expect(el.querySelectorAll('.ProseMirror')).toHaveLength(2) await selectTab('Changes') + await selectTab('Markdown source') await selectTab('Full documents') expect(el.querySelector('.text-comparison > [data-comparison-source-fallback]')).toBeNull() - expect(el.querySelectorAll('.ProseMirror')).toHaveLength(2) + expect([...el.querySelectorAll('.ProseMirror')]).toEqual(editors) instance.destroy() }) + it('measures Documents after revealing its persistent editors during navigation', async () => { + const el = document.createElement('div') + const instance = await createMarkdownContentComparison({ beforeContent: 'Before', afterContent: 'After', el }) + const selectTab = async (label: string) => { + const tab = [...el.querySelectorAll('[role="tab"]')] + .find(({ textContent }) => textContent?.trim() === label)! + tab.click() + await nextTick() + } + try { + await selectTab('Changes') + const measuredDisplays: string[] = [] + vi.spyOn(HTMLElement.prototype, 'scrollTo').mockImplementation(function(this: HTMLElement) { + measuredDisplays.push(this.closest('.text-comparison__documents')!.style.display) + }) + await selectTab('Full documents') + expect(measuredDisplays).toHaveLength(2) + expect(measuredDisplays).not.toContain('none') + } finally { + instance.destroy() + } + }) + it('omits duplicate heading anchors from the two comparison documents', async () => { const el = document.createElement('div') const instance = await createMarkdownContentComparison({ @@ -143,7 +295,7 @@ describe('Markdown comparison factory fallback and lifecycle', () => { instance.destroy() }) - it('AUD-02 publishes the current selection when Documents mounts lazily', async () => { + it('AUD-02 updates the current selection in the mounted Documents', async () => { const originalScrollTo = HTMLElement.prototype.scrollTo const scrollTo = vi.fn() HTMLElement.prototype.scrollTo = scrollTo