diff --git a/packages/layout-engine/layout-bridge/src/position-hit.ts b/packages/layout-engine/layout-bridge/src/position-hit.ts index a660b04c85..233c2410d3 100644 --- a/packages/layout-engine/layout-bridge/src/position-hit.ts +++ b/packages/layout-engine/layout-bridge/src/position-hit.ts @@ -567,6 +567,12 @@ export const hitTestTableFragment = ( return 0; }; + let nearestParagraphHit: + | (Omit & { + distance: number; + }) + | null = null; + for (let i = 0; i < cellBlocks.length && i < cellBlockMeasures.length; i++) { const cellBlock = cellBlocks[i]; const cellBlockMeasure = cellBlockMeasures[i]; @@ -585,8 +591,7 @@ export const hitTestTableFragment = ( const paragraphMeasure = cellBlockMeasure as ParagraphMeasure; const isWithinBlock = cellLocalY >= blockStartY && cellLocalY < blockEndY; - const isLastParagraph = i === Math.min(cellBlocks.length, cellBlockMeasures.length) - 1; - if (isWithinBlock || isLastParagraph) { + if (isWithinBlock) { const unclampedLocalY = cellLocalY - blockStartY; const localYWithinBlock = Math.max(0, Math.min(unclampedLocalY, Math.max(blockHeight, 0))); return { @@ -603,8 +608,35 @@ export const hitTestTableFragment = ( }; } + const distanceToBlock = cellLocalY < blockStartY ? blockStartY - cellLocalY : Math.max(0, cellLocalY - blockEndY); + if (!nearestParagraphHit || distanceToBlock < nearestParagraphHit.distance) { + const unclampedLocalY = cellLocalY - blockStartY; + nearestParagraphHit = { + cellBlock: paragraphBlock, + cellMeasure: paragraphMeasure, + localX: Math.max(0, cellLocalX), + localY: Math.max(0, Math.min(unclampedLocalY, Math.max(blockHeight, 0))), + distance: distanceToBlock, + }; + } + blockStartY = blockEndY; } + + if (nearestParagraphHit) { + return { + fragment: tableFragment, + block: tableBlock, + measure: tableMeasure, + pageIndex: pageHit.pageIndex, + cellRowIndex: rowIndex, + cellColIndex: colIndex, + cellBlock: nearestParagraphHit.cellBlock, + cellMeasure: nearestParagraphHit.cellMeasure, + localX: nearestParagraphHit.localX, + localY: nearestParagraphHit.localY, + }; + } } return null; diff --git a/packages/layout-engine/layout-bridge/test/clickToPosition.test.ts b/packages/layout-engine/layout-bridge/test/clickToPosition.test.ts index 8e9bd017fe..c041ea657c 100644 --- a/packages/layout-engine/layout-bridge/test/clickToPosition.test.ts +++ b/packages/layout-engine/layout-bridge/test/clickToPosition.test.ts @@ -496,6 +496,118 @@ describe('clickToPosition: table cell empty space', () => { expect(result!.pos).toBeGreaterThanOrEqual(50); expect(result!.blockId).toBe('table-block'); }); + + it('chooses the nearest paragraph when clicking empty space before cell text', () => { + const firstParagraph: FlowBlock = { + kind: 'paragraph', + id: 'cell-para-1', + runs: [{ text: 'First paragraph', fontFamily: 'Arial', fontSize: 14, pmStart: 50, pmEnd: 65 }], + }; + + const secondParagraph: FlowBlock = { + kind: 'paragraph', + id: 'cell-para-2', + runs: [{ text: 'Second paragraph', fontFamily: 'Arial', fontSize: 14, pmStart: 65, pmEnd: 81 }], + }; + + const multiParaTableBlock: FlowBlock = { + kind: 'table', + id: 'table-gap-block', + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0', + blocks: [firstParagraph, secondParagraph], + attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } }, + }, + ], + }, + ], + }; + + const multiParaTableMeasure: Measure = { + kind: 'table', + rows: [ + { + height: 60, + cells: [ + { + width: 200, + height: 60, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 15, + width: 120, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }, + { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 16, + width: 130, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }, + ], + }, + ], + }, + ], + columnWidths: [200], + totalWidth: 200, + totalHeight: 60, + }; + + const gapLayout: Layout = { + pageSize: { w: 400, h: 500 }, + pages: [ + { + number: 1, + fragments: [ + { + kind: 'table', + blockId: 'table-gap-block', + fromRow: 0, + toRow: 1, + x: 30, + y: 70, + width: 200, + height: 60, + }, + ], + }, + ], + }; + + const result = clickToPosition(gapLayout, [multiParaTableBlock], [multiParaTableMeasure], { x: 50, y: 71 }); + + expect(result).not.toBeNull(); + expect(result!.blockId).toBe('table-gap-block'); + expect(result!.pos).toBeGreaterThanOrEqual(50); + expect(result!.pos).toBeLessThanOrEqual(65); + }); }); describe('clickToPosition: table cell on page 2 (multi-page)', () => { diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/input/ClickSelectionUtilities.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/input/ClickSelectionUtilities.ts index 23b68a4075..ef27ac7e5f 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/input/ClickSelectionUtilities.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/input/ClickSelectionUtilities.ts @@ -68,10 +68,13 @@ export function getFirstTextPosition(doc: ProseMirrorNode | null): number { } let validPos = 1; + let found = false; doc.nodesBetween(0, doc.content.size, (node, pos) => { + if (found) return false; if (node.isTextblock) { validPos = pos + 1; + found = true; return false; } return true; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts index 2c82bf6bef..f03bbec374 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts @@ -1162,9 +1162,9 @@ export class EditorInputManager { } } - // Handle click outside text content + // Handle click outside text content — keep cursor and scroll position unchanged. if (!rawHit) { - this.#focusEditorAtFirstPosition(); + this.#focusEditor(); return; } diff --git a/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.test.ts b/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.test.ts index 17976385d4..21d34e664f 100644 --- a/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.test.ts +++ b/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.test.ts @@ -218,6 +218,47 @@ describe('DomPointerMapping', () => { }); }); + it('resolves through a nested table wrapper when the click lands between lines', () => { + container.innerHTML = ` +
+
+
+
+
+
+ Upper line +
+
+
+
+ Lower line +
+
+
+
+
+
+ `; + + const page = container.querySelector('.superdoc-page') as HTMLElement; + const tableFragment = container.querySelector('.superdoc-table-fragment') as HTMLElement; + const cell = container.querySelector('.superdoc-table-cell') as HTMLElement; + const content = container.querySelector('.cell-content') as HTMLElement; + const lines = container.querySelectorAll('.superdoc-line') as NodeListOf; + const upperRect = lines[0].getBoundingClientRect(); + const lowerRect = lines[1].getBoundingClientRect(); + const gapY = upperRect.bottom + Math.max(1, (lowerRect.top - upperRect.bottom) / 3); + + withMockedElementsFromPoint( + [content, cell, tableFragment, page, container, document.body, document.documentElement], + () => { + const result = clickToPositionDom(container, upperRect.left + 5, gapY); + expect(result).toBeGreaterThanOrEqual(5); + expect(result).toBeLessThanOrEqual(15); + }, + ); + }); + it('returns a position when a line IS in the hit chain', () => { container.innerHTML = `
diff --git a/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.ts b/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.ts index a94024f075..fbd8aa60c1 100644 --- a/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.ts +++ b/packages/super-editor/src/editors/v1/dom-observer/DomPointerMapping.ts @@ -189,6 +189,14 @@ export function clickToPositionDom(domContainer: HTMLElement, clientX: number, c return resolveLineAtX(hitChainLine, clientX); } + if (fragmentEl.classList.contains(CLASS.tableFragment)) { + const scopedContainer = findScopedLineContainer(hitChain, fragmentEl); + if (scopedContainer) { + log('Resolving table click from scoped line container'); + return resolveLineInContainer(scopedContainer, clientX, clientY); + } + } + // For table fragments without a direct line hit, defer to geometry // (hitTestTableFragment resolves the correct cell by column). if (fragmentEl.classList.contains(CLASS.tableFragment)) { @@ -278,6 +286,18 @@ function resolveFragment(fragmentEl: HTMLElement, viewX: number, viewY: number): return resolveLineAtX(lineEl, viewX); } +function resolveLineInContainer(containerEl: HTMLElement, viewX: number, viewY: number): number | null { + const lineEls = Array.from(containerEl.querySelectorAll(`.${CLASS.line}`)) as HTMLElement[]; + if (lineEls.length === 0) { + return null; + } + + const lineEl = findLineAtY(lineEls, viewY); + if (!lineEl) return null; + + return resolveLineAtX(lineEl, viewX); +} + /** * Given a known line element, resolves the PM position at the given X * coordinate. @@ -364,12 +384,21 @@ function resolvePositionInLine( function findLineAtY(lineEls: HTMLElement[], viewY: number): HTMLElement | null { if (lineEls.length === 0) return null; + let nearest: HTMLElement = lineEls[0]; + let minDistance = Infinity; + for (const lineEl of lineEls) { const r = lineEl.getBoundingClientRect(); if (viewY >= r.top && viewY <= r.bottom) return lineEl; + + const distance = viewY < r.top ? r.top - viewY : Math.max(0, viewY - r.bottom); + if (distance < minDistance) { + minDistance = distance; + nearest = lineEl; + } } - return lineEls[lineEls.length - 1]; + return nearest; } /** @@ -397,6 +426,19 @@ function findSpanAtX(spanEls: HTMLElement[], viewX: number): HTMLElement | null return nearest; } +function findScopedLineContainer(hitChain: Element[], fragmentEl: HTMLElement): HTMLElement | null { + for (const el of hitChain) { + if (!(el instanceof HTMLElement)) continue; + if (el === fragmentEl) break; + if (!fragmentEl.contains(el)) continue; + if (el.querySelector(`.${CLASS.line}`)) { + return el; + } + } + + return null; +} + // --------------------------------------------------------------------------- // Character-level position resolution // --------------------------------------------------------------------------- diff --git a/tests/behavior/tests/navigation/click-scroll-jump.spec.ts b/tests/behavior/tests/navigation/click-scroll-jump.spec.ts new file mode 100644 index 0000000000..2136786f11 --- /dev/null +++ b/tests/behavior/tests/navigation/click-scroll-jump.spec.ts @@ -0,0 +1,196 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import type { Page } from '@playwright/test'; +import { test, expect } from '../../fixtures/superdoc.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOC_PATH = path.resolve(__dirname, '../../test-data/tables/sd-2356-click-scroll-jump.docx'); + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available'); + +test.use({ config: { toolbar: 'full' } }); + +async function getScrollTop(page: Page): Promise { + return page.evaluate(() => { + let el: Element | null = document.querySelector('.superdoc-page[data-page-index]'); + while (el) { + el = el.parentElement; + if ( + el && + el.scrollHeight > el.clientHeight + 100 && + (getComputedStyle(el).overflowY === 'auto' || getComputedStyle(el).overflowY === 'scroll') + ) { + return el.scrollTop; + } + } + return window.scrollY; + }); +} + +test('@behavior SD-2356: clicking page margin should not jump scroll position', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(3000); + + const page = superdoc.page; + + // Wait for multiple pages to be rendered + await expect(page.locator('.superdoc-page[data-page-index]').first()).toBeVisible({ + timeout: 15_000, + }); + const pageCount = await page.locator('.superdoc-page[data-page-index]').count(); + expect(pageCount).toBeGreaterThanOrEqual(3); + + // Step 1: Place the cursor at "This agreement dated" on page 2 + const textPos = await superdoc.findTextPos('This agreement dated'); + await superdoc.setTextSelection(textPos, textPos); + await superdoc.waitForStable(); + + const selBefore = await superdoc.getSelection(); + expect(selBefore.from).toBe(textPos); + + // Step 2: Scroll down so page 3 is visible, without moving the cursor + const page3Index = 2; + await page.evaluate((idx) => { + const pages = document.querySelectorAll('.superdoc-page[data-page-index]'); + const page3 = pages[idx] as HTMLElement; + if (!page3) throw new Error(`Page ${idx} not found`); + page3.scrollIntoView({ block: 'start' }); + }, page3Index); + await superdoc.waitForStable(500); + + const scrollBefore = await getScrollTop(page); + + // Step 3: Click on the top margin area of page 3 (above the header) + const page3Locator = page.locator('.superdoc-page[data-page-index]').nth(page3Index); + const page3Box = await page3Locator.boundingBox(); + expect(page3Box).not.toBeNull(); + + await page.mouse.click(page3Box!.x + page3Box!.width / 2, page3Box!.y + 15); + await superdoc.waitForStable(1000); + + const scrollAfter = await getScrollTop(page); + const scrollDelta = Math.abs(scrollAfter - scrollBefore); + expect( + scrollDelta, + `Scroll jumped by ${scrollDelta}px after clicking page margin — expected no significant scroll change`, + ).toBeLessThan(100); +}); + +test('@behavior SD-2356: clicking into table area should not jump scroll position', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(3000); + + const page = superdoc.page; + + await expect(page.locator('.superdoc-page[data-page-index]').first()).toBeVisible({ + timeout: 15_000, + }); + + // Place cursor at start of page 2 + const textPos = await superdoc.findTextPos('This agreement dated'); + await superdoc.setTextSelection(textPos, textPos); + await superdoc.waitForStable(); + + // Scroll to make the definitions table visible + const defsText = page.locator('text=DEFINITIONS AND INTERPRETATIONS').first(); + await defsText.scrollIntoViewIfNeeded(); + await superdoc.waitForStable(500); + + const scrollBefore = await getScrollTop(page); + + // Click into a table cell + const tableCell = page.locator('text=Business Day').first(); + const cellBox = await tableCell.boundingBox(); + + if (cellBox) { + await page.mouse.click(cellBox.x + 5, cellBox.y + 5); + } else { + const viewport = page.viewportSize()!; + await page.mouse.click(viewport.width / 2, viewport.height / 2); + } + await superdoc.waitForStable(1000); + + const scrollAfter = await getScrollTop(page); + const scrollDelta = Math.abs(scrollAfter - scrollBefore); + expect( + scrollDelta, + `Scroll jumped by ${scrollDelta}px after clicking table cell — expected no significant scroll change`, + ).toBeLessThan(100); +}); + +test('@behavior SD-2356: clicking gap between paragraphs in table should not jump scroll', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(3000); + + const page = superdoc.page; + + await expect(page.locator('.superdoc-page[data-page-index]').first()).toBeVisible({ + timeout: 15_000, + }); + + // Step 1: Place cursor to the left of "company that will be owned in substantially the same" + const targetText = 'company that will be owned in substantially the same'; + const textPos = await superdoc.findTextPos(targetText); + await superdoc.setTextSelection(textPos, textPos); + await superdoc.waitForStable(); + + // Scroll so both text areas are visible + const targetLocator = page.locator(`text=${targetText}`).first(); + await targetLocator.scrollIntoViewIfNeeded(); + await superdoc.waitForStable(500); + + // Step 2: Find the gap between the bullet paragraph ending with + // "exchange of similar or better standing," and the paragraph starting + // with "provided, however, that a transaction..." — both are in the same + // table cell on page 5. + const gapCoords = await page.evaluate(() => { + const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_TEXT); + let aboveRect: DOMRect | null = null; + let belowRect: DOMRect | null = null; + + while (walker.nextNode()) { + const text = walker.currentNode.textContent || ''; + // Match the visible (painted) instances — they have finite x coordinates + if (text.includes('exchange of similar or better standing,')) { + const el = walker.currentNode.parentElement; + if (!el) continue; + const rect = el.getBoundingClientRect(); + // Skip hidden ProseMirror DOM (has negative x) + if (rect.x < 0) continue; + aboveRect = rect; + } + if (text.includes('provided, however, that a transaction')) { + const el = walker.currentNode.parentElement; + if (!el) continue; + const rect = el.getBoundingClientRect(); + if (rect.x < 0) continue; + belowRect = rect; + } + } + + if (!aboveRect || !belowRect) return null; + + return { + gapY: (aboveRect.bottom + belowRect.top) / 2, + gapX: aboveRect.left + 100, + gapSize: belowRect.top - aboveRect.bottom, + }; + }); + + expect(gapCoords).not.toBeNull(); + expect(gapCoords!.gapSize).toBeGreaterThan(0); + + const scrollBefore = await getScrollTop(page); + + await page.mouse.click(gapCoords!.gapX, gapCoords!.gapY); + await superdoc.waitForStable(1000); + + const scrollAfter = await getScrollTop(page); + const scrollDelta = Math.abs(scrollAfter - scrollBefore); + + expect( + scrollDelta, + `Scroll jumped by ${scrollDelta}px after clicking paragraph gap — expected no significant scroll change`, + ).toBeLessThan(100); +});