From f120cbfddacc87f62d265f93cf23826cd0581302 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Tue, 26 May 2026 11:20:28 -0300 Subject: [PATCH 1/4] fix: table selection not providing a feedback --- .../pointer-events/EditorInputManager.ts | 47 +--- .../selection-isolating-boundary.test.js | 209 ------------------ 2 files changed, 1 insertion(+), 255 deletions(-) delete mode 100644 packages/super-editor/src/editors/v1/core/presentation-editor/tests/selection-isolating-boundary.test.js 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 035e9a763d..3d028b4279 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 @@ -995,44 +995,6 @@ export class EditorInputManager { return calculateExtendedSelection(layoutState?.blocks ?? [], anchor, head, mode); } - /** - * When the drag anchor is outside an isolating node (table), prevent the head - * from resolving inside one. If the head is inside a table cell, clamp it to - * just before or after the table boundary (depending on drag direction). - * - * Selections that span PAST a table (anchor before, head after) are allowed — - * only positions resolving INSIDE the table are clamped. - */ - #clampHeadAtIsolatingBoundary(doc: ProseMirrorNode, anchor: number, head: number): number { - const forward = head >= anchor; - - try { - const $head = doc.resolve(head); - // Find the outermost isolating ancestor. Walk from innermost to outermost, - // tracking the shallowest isolating depth. Using the outermost ensures that - // we clamp to just before/after the entire table, not to a boundary between - // cells within the same table. - let isolatingDepth = -1; - for (let d = $head.depth; d > 0; d--) { - const node = $head.node(d); - if (node.type.spec.isolating || node.type.spec.tableRole === 'table') { - isolatingDepth = d; - } - } - - if (isolatingDepth > 0) { - const boundary = forward ? $head.before(isolatingDepth) : $head.after(isolatingDepth); - const near = Selection.near(doc.resolve(boundary), forward ? -1 : 1); - if (near instanceof TextSelection) return near.head; - return anchor; - } - } catch { - /* position resolution failed */ - } - - return head; - } - #shouldUseCellSelection(currentTableHit: TableHitResult | null): boolean { return shouldUseCellSelectionFromHelper(currentTableHit, this.#cellAnchor, this.#cellDragMode); } @@ -2463,14 +2425,7 @@ export class EditorInputManager { // Text selection mode const anchor = this.#dragAnchor!; - let head = hit.pos; - - // When the drag started outside a table, prevent the head from entering an isolating - // node (table). If the head resolves inside a table, ProseMirror-tables' appendTransaction - // converts the TextSelection into a CellSelection, causing the anchor to jump. - if (!this.#cellAnchor) { - head = this.#clampHeadAtIsolatingBoundary(doc, anchor, head); - } + const head = hit.pos; const { selAnchor, selHead } = this.#calculateExtendedSelection(anchor, head, this.#dragExtensionMode); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/selection-isolating-boundary.test.js b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/selection-isolating-boundary.test.js deleted file mode 100644 index 842de3ffff..0000000000 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/selection-isolating-boundary.test.js +++ /dev/null @@ -1,209 +0,0 @@ -/** - * Tests for selection behavior at isolating node boundaries (tables). - * - * When drag-selecting text in a paragraph near a table, the selection head - * must not resolve inside the table. If it does, ProseMirror-tables' - * appendTransaction converts the TextSelection into a CellSelection, - * causing the anchor to jump into the table — visually the selection - * "flickers" or jumps away from where the user started dragging. - * - * These tests verify the behavioral contract: - * 1. A TextSelection from a paragraph position to inside a table should - * be clamped so the head stays outside the table. - * 2. A TextSelection that spans PAST a table (anchor before, head after) - * should be allowed — only heads INSIDE isolating nodes are clamped. - */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { TextSelection, Selection } from 'prosemirror-state'; -import { initTestEditor } from '@tests/helpers/helpers.js'; - -/** - * Replicates the clamping logic from EditorInputManager.#clampHeadAtIsolatingBoundary. - * Extracted here as a pure function for testability. - */ -function clampHeadAtIsolatingBoundary(doc, anchor, head) { - const forward = head >= anchor; - - try { - const $head = doc.resolve(head); - // Find the outermost isolating ancestor - let isolatingDepth = -1; - for (let d = $head.depth; d > 0; d--) { - const node = $head.node(d); - if (node.type.spec.isolating || node.type.spec.tableRole === 'table') { - isolatingDepth = d; - } - } - - if (isolatingDepth > 0) { - const boundary = forward ? $head.before(isolatingDepth) : $head.after(isolatingDepth); - const near = Selection.near(doc.resolve(boundary), forward ? -1 : 1); - if (near instanceof TextSelection) return near.head; - return anchor; - } - } catch { - /* position resolution failed */ - } - - return head; -} - -// Document structure: -// doc -// paragraph "Before the table" (positions ~1-19) -// table (isolating node) -// tableRow -// tableCell -// paragraph "Cell content" -// tableCell -// paragraph "Cell two" -// paragraph "After the table" (positions after table end) -const docJson = { - type: 'doc', - content: [ - { - type: 'paragraph', - content: [{ type: 'run', content: [{ type: 'text', text: 'Before the table' }] }], - }, - { - type: 'table', - content: [ - { - type: 'tableRow', - content: [ - { - type: 'tableCell', - attrs: { colwidth: [100] }, - content: [ - { - type: 'paragraph', - content: [{ type: 'run', content: [{ type: 'text', text: 'Cell content' }] }], - }, - ], - }, - { - type: 'tableCell', - attrs: { colwidth: [100] }, - content: [ - { - type: 'paragraph', - content: [{ type: 'run', content: [{ type: 'text', text: 'Cell two' }] }], - }, - ], - }, - ], - }, - ], - }, - { - type: 'paragraph', - content: [{ type: 'run', content: [{ type: 'text', text: 'After the table' }] }], - }, - ], -}; - -describe('selection clamping at isolating boundaries (SD-2024)', () => { - let editor; - let doc; - let tableStart; - let tableEnd; - let beforeParaStart; - let afterParaStart; - - beforeAll(() => { - ({ editor } = initTestEditor({ loadFromSchema: true, content: docJson })); - doc = editor.state.doc; - - // Find table boundaries - doc.descendants((node, pos) => { - if (node.type.name === 'table') { - tableStart = pos; - tableEnd = pos + node.nodeSize; - } - }); - - // First paragraph content starts at pos 1+1 = 2 (doc boundary + paragraph boundary) - // But we need a text position — let's find it - doc.descendants((node, pos) => { - if (beforeParaStart == null && node.isText && node.text.includes('Before')) { - beforeParaStart = pos; - } - }); - - // Find text position after table - let pastTable = false; - doc.descendants((node, pos) => { - if (node.type.name === 'table') pastTable = true; - if (afterParaStart == null && pastTable && node.isText && node.text.includes('After')) { - afterParaStart = pos; - } - }); - }); - - afterAll(() => { - editor.destroy(); - }); - - it('document has expected structure: paragraph, table, paragraph', () => { - const topLevelTypes = []; - doc.forEach((node) => topLevelTypes.push(node.type.name)); - expect(topLevelTypes).toEqual(['paragraph', 'table', 'paragraph']); - }); - - it('clamps head when it resolves inside a table (forward drag)', () => { - // Simulate: anchor in first paragraph, head inside a table cell - const anchor = beforeParaStart + 3; // somewhere in "Before the table" - const headInsideTable = tableStart + 5; // inside the table - - const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headInsideTable); - - // Clamped head should be outside the table (before it) - expect(clamped).toBeLessThan(tableStart); - // Should still be a valid text position - expect(clamped).toBeGreaterThan(0); - }); - - it('clamps head when it resolves inside a table (backward drag)', () => { - // Simulate: anchor after table, head inside table (dragging backwards) - const anchor = afterParaStart + 3; - const headInsideTable = tableStart + 5; - - const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headInsideTable); - - // Clamped head should be outside the table (after it) - expect(clamped).toBeGreaterThan(tableEnd - 1); - }); - - it('does NOT clamp head when it is outside the table (after it)', () => { - // Simulate: anchor in first paragraph, head in paragraph after table - // This selection spans PAST the table — should be allowed - const anchor = beforeParaStart + 3; - const headAfterTable = afterParaStart + 3; - - const clamped = clampHeadAtIsolatingBoundary(doc, anchor, headAfterTable); - - // Head should remain unchanged — it's not inside the table - expect(clamped).toBe(headAfterTable); - }); - - it('does NOT clamp head when it is in the same paragraph as anchor', () => { - // Simulate: both anchor and head in the first paragraph - const anchor = beforeParaStart + 1; - const head = beforeParaStart + 10; - - const clamped = clampHeadAtIsolatingBoundary(doc, anchor, head); - - // Head should remain unchanged - expect(clamped).toBe(head); - }); - - it('TextSelection spanning past table includes content after table', () => { - // Verify that ProseMirror allows a TextSelection from before to after a table - const anchor = beforeParaStart + 3; - const head = afterParaStart + 3; - - const sel = TextSelection.create(doc, anchor, head); - expect(sel.from).toBeLessThan(tableStart); - expect(sel.to).toBeGreaterThan(tableEnd - 1); - }); -}); From 1285dca2dcd65b84dc31693db010c8cdaea584bb Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Tue, 26 May 2026 15:09:38 -0300 Subject: [PATCH 2/4] test: added tests --- .../EditorInputManager.dragIntoTable.test.ts | 263 ++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragIntoTable.test.ts diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragIntoTable.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragIntoTable.test.ts new file mode 100644 index 0000000000..2e4272aa8c --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragIntoTable.test.ts @@ -0,0 +1,263 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { TextSelection } from 'prosemirror-state'; + +import { + EditorInputManager, + type EditorInputDependencies, + type EditorInputCallbacks, +} from '../pointer-events/EditorInputManager.js'; + +/** + * Behavior tests for SD-2676: drag selection must keep updating as the pointer + * sweeps downward through a multi-row table. + * + * The regression introduced by PR #2205 pinned `head` to the position just + * before/after the outermost isolating ancestor (the table). That made every + * pointermove inside the table dispatch a selection with the SAME `head`, so + * the highlight froze. These tests assert the opposite: each pointermove past + * the drag threshold dispatches a new selection whose `head` follows the + * pointer-resolved position, so the highlight updates continuously across + * multiple rows and over long downward distances. + */ + +const resolverHits: Array<{ + pos: number; + layoutEpoch: number; + pageIndex: number; + blockId: string; + column: number; + lineIndex: number; +}> = []; +let resolverIndex = 0; + +vi.mock('../input/PositionHitResolver.js', () => ({ + resolvePointerPositionHit: vi.fn(() => { + const hit = resolverHits[Math.min(resolverIndex, resolverHits.length - 1)]; + resolverIndex += 1; + return hit; + }), +})); + +vi.mock('@superdoc/layout-bridge', () => ({ + clickToPosition: vi.fn(), + getFragmentAtPosition: vi.fn(() => null), +})); + +vi.mock('prosemirror-state', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + TextSelection: { + ...original.TextSelection, + create: vi.fn(() => ({ + $from: { parent: { inlineContent: true } }, + empty: true, + })), + }, + }; +}); + +describe('EditorInputManager - drag selection through a multi-row table (SD-2676)', () => { + let manager: EditorInputManager; + let viewportHost: HTMLElement; + let visibleHost: HTMLElement; + let scrollContainer: HTMLElement; + let mockEditor: { + isEditable: boolean; + state: { + doc: { content: { size: number } }; + tr: { setSelection: ReturnType }; + selection: { $anchor?: null }; + storedMarks?: unknown; + }; + view: { dispatch: ReturnType; dom: HTMLElement; hasFocus: ReturnType }; + emit: ReturnType; + }; + let mockDeps: EditorInputDependencies; + let mockCallbacks: EditorInputCallbacks; + + beforeEach(() => { + resolverHits.length = 0; + resolverIndex = 0; + + scrollContainer = document.createElement('div'); + scrollContainer.style.overflowY = 'auto'; + scrollContainer.style.height = '600px'; + + visibleHost = document.createElement('div'); + visibleHost.className = 'presentation-editor'; + viewportHost = document.createElement('div'); + viewportHost.className = 'presentation-editor__viewport'; + visibleHost.appendChild(viewportHost); + scrollContainer.appendChild(visibleHost); + document.body.appendChild(scrollContainer); + + Object.defineProperty(scrollContainer, 'clientHeight', { value: 600, configurable: true }); + Object.defineProperty(scrollContainer, 'clientWidth', { value: 400, configurable: true }); + Object.defineProperty(scrollContainer, 'scrollHeight', { value: 600, configurable: true }); + Object.defineProperty(scrollContainer, 'scrollWidth', { value: 400, configurable: true }); + scrollContainer.getBoundingClientRect = () => + ({ top: 0, bottom: 600, left: 0, right: 400, width: 400, height: 600 }) as DOMRect; + + viewportHost.setPointerCapture = vi.fn(); + viewportHost.releasePointerCapture = vi.fn(); + viewportHost.hasPointerCapture = vi.fn(() => true); + + mockEditor = { + isEditable: true, + state: { + doc: { content: { size: 1000 } }, + tr: { setSelection: vi.fn().mockReturnThis() }, + selection: { $anchor: null }, + }, + view: { + dispatch: vi.fn(), + dom: document.createElement('div'), + hasFocus: vi.fn(() => true), + }, + emit: vi.fn(), + }; + + mockDeps = { + getActiveEditor: vi.fn(() => mockEditor as unknown as ReturnType), + getEditor: vi.fn(() => mockEditor as unknown as ReturnType), + getLayoutState: vi.fn(() => ({ layout: {} as never, blocks: [], measures: [] })), + getEpochMapper: vi.fn(() => ({ + // Identity mapping: head pos passed through unchanged so we can verify + // the value the manager hands to TextSelection.create directly. + mapPosFromLayoutToCurrentDetailed: vi.fn((pos: number) => ({ ok: true, pos, toEpoch: 1 })), + })) as unknown as EditorInputDependencies['getEpochMapper'], + getViewportHost: vi.fn(() => viewportHost), + getVisibleHost: vi.fn(() => visibleHost), + getLayoutMode: vi.fn(() => 'vertical'), + getHeaderFooterSession: vi.fn(() => null), + getPageGeometryHelper: vi.fn(() => null), + getZoom: vi.fn(() => 1), + isViewLocked: vi.fn(() => false), + getDocumentMode: vi.fn(() => 'editing'), + getPageElement: vi.fn(() => null), + isSelectionAwareVirtualizationEnabled: vi.fn(() => false), + }; + + mockCallbacks = { + normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ + x: clientX, + y: clientY, + pageIndex: 0, + pageLocalY: clientY, + })), + updateSelectionVirtualizationPins: vi.fn(), + scheduleSelectionUpdate: vi.fn(), + notifyDragSelectionEnded: vi.fn(), + // No table hits — the drag never enters CellSelection mode. This isolates + // the regression to the text-selection drag path. + hitTestTable: vi.fn(() => null), + }; + + manager = new EditorInputManager(); + manager.setDependencies(mockDeps); + manager.setCallbacks(mockCallbacks); + manager.bind(); + }); + + afterEach(() => { + manager.destroy(); + document.body.removeChild(scrollContainer); + vi.clearAllMocks(); + }); + + function getPointerEventImpl(): typeof PointerEvent | typeof MouseEvent { + return ( + (globalThis as unknown as { PointerEvent?: typeof PointerEvent; MouseEvent: typeof MouseEvent }).PointerEvent ?? + globalThis.MouseEvent + ); + } + + function dispatch(type: 'pointerdown' | 'pointermove' | 'pointerup', clientX: number, clientY: number): void { + const Impl = getPointerEventImpl(); + viewportHost.dispatchEvent( + new Impl(type, { + bubbles: true, + cancelable: true, + clientX, + clientY, + button: 0, + buttons: type === 'pointerup' ? 0 : 1, + } as PointerEventInit), + ); + } + + function pushHit(pos: number): void { + resolverHits.push({ pos, layoutEpoch: 1, pageIndex: 0, blockId: '', column: 0, lineIndex: -1 }); + } + + function selectionCallArgs(): Array<[unknown, number, number | undefined]> { + const calls = (TextSelection.create as unknown as ReturnType).mock.calls as Array; + return calls.map((args) => [args[0], args[1] as number, args[2] as number | undefined]); + } + + it('updates the selection head on each pointermove while sweeping downward through table rows', () => { + // Simulated positions: anchor in paragraph above the table (pos=10), then + // four progressively deeper hits as the pointer traverses four rows of a + // multi-row table (pos=120, 220, 320, 420). + pushHit(10); // pointerdown anchor (paragraph above table) + pushHit(120); // first move, row 1 + pushHit(220); // row 2 + pushHit(320); // row 3 + pushHit(420); // row 4 + + dispatch('pointerdown', 100, 20); + // Sweep downward; each move must exceed the 5px drag threshold relative to + // the start position so every pointermove triggers a selection dispatch. + dispatch('pointermove', 100, 80); + dispatch('pointermove', 100, 160); + dispatch('pointermove', 100, 240); + dispatch('pointermove', 100, 320); + + const args = selectionCallArgs(); + // Pointerdown places the caret (one call). Then four pointermoves each + // dispatch one extended selection — five total. + expect(args.length).toBe(5); + + // First call is the pointerdown caret placement — TextSelection.create + // is invoked with a single position (no head argument) when seating the + // caret. The drag-extension calls below are what carry an explicit head. + expect(args[0][1]).toBe(10); + + // Each drag extension keeps the same anchor (10) and the head must follow + // the resolved hit position — i.e. NOT pinned to a single table-boundary + // value. Heads must be strictly increasing as the pointer sweeps downward + // through successive rows. + const dragHeads = args.slice(1).map(([, , head]) => head); + expect(dragHeads).toEqual([120, 220, 320, 420]); + + // Sanity: anchor never jumps during the drag. + for (const [, anchor] of args.slice(1)) { + expect(anchor).toBe(10); + } + }); + + it('keeps extending the selection across a long downward drag (regression guard)', () => { + // Twelve successive hits spanning ~600 doc positions — represents a long + // sweep through a tall table. The pre-fix behavior would pin head to the + // table boundary, so all dragHeads would collapse to a single value. + pushHit(50); // anchor (paragraph above table) + for (let i = 1; i <= 12; i += 1) { + pushHit(50 + i * 50); + } + + dispatch('pointerdown', 100, 20); + for (let i = 1; i <= 12; i += 1) { + dispatch('pointermove', 100, 20 + i * 40); + } + + const args = selectionCallArgs(); + const dragHeads = args.slice(1).map(([, , head]) => head); + + // Every move past the threshold produced a distinct head — selection + // updates continuously instead of freezing at a clamped boundary. + expect(new Set(dragHeads).size).toBe(dragHeads.length); + expect(dragHeads).toEqual(dragHeads.slice().sort((a, b) => a - b)); + expect(dragHeads[dragHeads.length - 1]).toBeGreaterThan(dragHeads[0]); + }); +}); From f0a3351ea5116ad39953a807268e678526cde37f Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 28 May 2026 15:03:56 -0300 Subject: [PATCH 3/4] test: added behavior tests --- ...drag-selection-into-table-feedback.spec.ts | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts diff --git a/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts b/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts new file mode 100644 index 0000000000..355a12d94d --- /dev/null +++ b/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts @@ -0,0 +1,220 @@ +import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', showSelection: true } }); + +/** + * SD-2676: Table selection must give live feedback while dragging. + * + * Regression history: PR #2205 (SD-2024) clamped the drag `head` to the table + * boundary whenever the pointer resolved inside an isolating node. That froze + * the selection highlight while the pointer swept through table rows — the + * highlight only resumed once the pointer left the table. SD-2676 removed the + * clamp so the head follows the pointer position continuously. + * + * These behavior tests drive a real pointer drag (the bug is a UI drag + * interaction) and assert the selection keeps growing — and the highlight keeps + * being painted — while the pointer remains inside the table. The pre-fix bug + * would surface as the selection size plateauing at the table boundary. + */ + +const ROWS = 4; +const COLS = 2; + +/** Count visible selection overlay rects (the painted highlight). */ +async function getSelectionOverlayRectCount(superdoc: SuperDocFixture): Promise { + return superdoc.page.evaluate(() => { + const overlay = document.querySelector('.presentation-editor__selection-layer--local'); + if (!overlay) return 0; + let count = 0; + for (const child of overlay.children) { + const rect = child.getBoundingClientRect(); + if (rect.width > 0 && rect.height > 0) count++; + } + return count; + }); +} + +/** Label used for the cell at (row, col), 1-indexed. */ +function cellLabel(row: number, col: number): string { + return `R${row}C${col} content`; +} + +/** + * Paragraph above + a fully populated multi-row table. Every cell carries text + * so each row has real line geometry to resolve a pointer hit against. + */ +async function setupParagraphAboveAndPopulatedTable(superdoc: SuperDocFixture) { + await superdoc.type('Paragraph above the table'); + await superdoc.newLine(); + await superdoc.waitForStable(); + + await superdoc.executeCommand('insertTable', { rows: ROWS, cols: COLS, withHeaderRow: false }); + await superdoc.waitForStable(); + + // Cursor lands in the first cell after insertTable. Tab navigates forward; + // Tab on the LAST cell would add a new row, so stop before the final Tab. + const totalCells = ROWS * COLS; + for (let i = 0; i < totalCells; i++) { + const row = Math.floor(i / COLS) + 1; + const col = (i % COLS) + 1; + await superdoc.type(cellLabel(row, col)); + if (i < totalCells - 1) await superdoc.press('Tab'); + } + await superdoc.waitForStable(); + + await superdoc.assertTableExists(ROWS, COLS); +} + +interface DragSample { + x: number; + y: number; + size: number; + rects: number; +} + +/** Press at (startX, startY), sweep to (endX, endY) in `steps`, sampling state. */ +async function dragSampling( + superdoc: SuperDocFixture, + startX: number, + startY: number, + endX: number, + endY: number, + steps: number, +): Promise { + const samples: DragSample[] = []; + await superdoc.page.mouse.move(startX, startY); + await superdoc.page.mouse.down(); + for (let i = 1; i <= steps; i++) { + const x = startX + ((endX - startX) * i) / steps; + const y = startY + ((endY - startY) * i) / steps; + await superdoc.page.mouse.move(x, y); + // Let the layout/paint pipeline catch up so each sample reflects the move. + await superdoc.page.waitForTimeout(40); + const sel = await superdoc.getSelection(); + const rects = await getSelectionOverlayRectCount(superdoc); + samples.push({ x, y, size: sel.to - sel.from, rects }); + } + await superdoc.page.mouse.up(); + await superdoc.waitForStable(); + return samples; +} + +test.describe('drag selection live feedback in tables (SD-2676)', () => { + test('dragging from above down through table rows keeps the highlight updating', async ({ superdoc }) => { + await setupParagraphAboveAndPopulatedTable(superdoc); + + const paragraphLine = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: 'Paragraph above the table' }) + .first(); + const paragraphBox = await paragraphLine.boundingBox(); + if (!paragraphBox) throw new Error('Paragraph line not visible'); + + const tableFragment = superdoc.page.locator('.superdoc-table-fragment').first(); + const tableBox = await tableFragment.boundingBox(); + if (!tableBox) throw new Error('Table fragment not visible'); + + // Anchor in the paragraph above, sweep down to near the bottom of the table. + // End the sweep within the last content row — not on the table's bottom + // border, where the resolved hit lands on the boundary and re-collapses. + const startX = paragraphBox.x + 20; + const startY = paragraphBox.y + paragraphBox.height / 2; + const endX = tableBox.x + tableBox.width / 2; + const endY = tableBox.y + tableBox.height * 0.85; + + const samples = await dragSampling(superdoc, startX, startY, endX, endY, 12); + + // Samples taken while the pointer is within the table's vertical span. + const inTable = samples.filter((s) => s.y >= tableBox.y); + expect(inTable.length).toBeGreaterThan(2); + + // Core regression guard: this drag stays a TextSelection (the anchor is + // outside the table), so its size must keep growing while the pointer + // sweeps through rows — not freeze at the boundary. The pre-fix bug pinned + // the head to the table boundary, collapsing every in-table sample to a + // single size. More than one distinct size, and a maximum that exceeds the + // first in-table sample, proves the highlight kept updating. + const inTableSizes = inTable.map((s) => s.size); + expect(new Set(inTableSizes).size).toBeGreaterThan(1); + expect(Math.max(...inTableSizes)).toBeGreaterThan(inTableSizes[0]); + + // The painted highlight must be present whenever there is a selection. + for (const s of samples) { + if (s.size > 0) expect(s.rects).toBeGreaterThan(0); + } + + // Final selection is non-collapsed. + const finalSel = await superdoc.getSelection(); + expect(finalSel.to - finalSel.from).toBeGreaterThan(0); + expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); + }); + + test('dragging downward starting inside the table selects cell content', async ({ superdoc }) => { + await setupParagraphAboveAndPopulatedTable(superdoc); + + const topCell = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: cellLabel(1, 1) }) + .first(); + const bottomCell = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: cellLabel(ROWS, 1) }) + .first(); + const topBox = await topCell.boundingBox(); + const bottomBox = await bottomCell.boundingBox(); + if (!topBox || !bottomBox) throw new Error('Table cell lines not visible'); + + const startX = topBox.x + 6; + const startY = topBox.y + topBox.height / 2; + const endX = bottomBox.x + 6; + const endY = bottomBox.y + bottomBox.height / 2; + + const samples = await dragSampling(superdoc, startX, startY, endX, endY, 10); + + // Dragging from inside the table must actually produce a selection — the + // second symptom in SD-2676 was that no text got selected at all. + const finalSel = await superdoc.getSelection(); + expect(finalSel.to - finalSel.from).toBeGreaterThan(0); + expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); + + // A same-table drag resolves to a CellSelection, whose paint expands across + // rows as the pointer descends. The painted rect count is the faithful + // signal that the highlight keeps updating (the CellSelection from/to span + // stays constant). Locks in the bug report's second symptom — dragging from + // inside the table must select content — against future regressions. + const activeRects = samples.filter((s) => s.size > 0).map((s) => s.rects); + expect(new Set(activeRects).size).toBeGreaterThan(1); + }); + + test('dragging upward starting inside the table selects cell content', async ({ superdoc }) => { + await setupParagraphAboveAndPopulatedTable(superdoc); + + const topCell = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: cellLabel(1, 1) }) + .first(); + const bottomCell = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: cellLabel(ROWS, 1) }) + .first(); + const topBox = await topCell.boundingBox(); + const bottomBox = await bottomCell.boundingBox(); + if (!topBox || !bottomBox) throw new Error('Table cell lines not visible'); + + // Start in the bottom row, drag up to the top row. + const startX = bottomBox.x + 6; + const startY = bottomBox.y + bottomBox.height / 2; + const endX = topBox.x + 6; + const endY = topBox.y + topBox.height / 2; + + const samples = await dragSampling(superdoc, startX, startY, endX, endY, 10); + + const finalSel = await superdoc.getSelection(); + expect(finalSel.to - finalSel.from).toBeGreaterThan(0); + expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); + + // The highlight expands across rows as the pointer ascends. + const activeRects = samples.filter((s) => s.size > 0).map((s) => s.rects); + expect(new Set(activeRects).size).toBeGreaterThan(1); + }); +}); From 453a58cdc4bc879ec41aaec6bcd0826b7af8a7f0 Mon Sep 17 00:00:00 2001 From: Gabriel Chittolina Date: Thu, 28 May 2026 16:08:40 -0300 Subject: [PATCH 4/4] test: fix tests --- ...drag-selection-into-table-feedback.spec.ts | 77 +++++++++++++++---- 1 file changed, 60 insertions(+), 17 deletions(-) diff --git a/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts b/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts index 355a12d94d..f56bdf7713 100644 --- a/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts +++ b/tests/behavior/tests/selection/drag-selection-into-table-feedback.spec.ts @@ -34,6 +34,30 @@ async function getSelectionOverlayRectCount(superdoc: SuperDocFixture): Promise< }); } +/** + * The committed selection's span and the document text it covers. + * + * `text` joins every selection range, not just `from`..`to`: a CellSelection's + * `from`/`to` span only the head cell, while the selected cells live in + * `.ranges`. Joining the ranges yields the full selected content for both a + * TextSelection (one range = the whole span) and a CellSelection (one range per + * selected cell). + */ +async function getSelectionInfo( + superdoc: SuperDocFixture, +): Promise<{ type: string; from: number; to: number; head: number; text: string }> { + return superdoc.page.evaluate(() => { + const { state } = (window as any).editor; + const s = state.selection; + const text = s.ranges + .map((r: { $from: { pos: number }; $to: { pos: number } }) => + state.doc.textBetween(r.$from.pos, r.$to.pos, ' ', ' '), + ) + .join(' '); + return { type: s.constructor.name, from: s.from, to: s.to, head: s.head, text }; + }); +} + /** Label used for the cell at (row, col), 1-indexed. */ function cellLabel(row: number, col: number): string { return `R${row}C${col} content`; @@ -114,13 +138,22 @@ test.describe('drag selection live feedback in tables (SD-2676)', () => { const tableBox = await tableFragment.boundingBox(); if (!tableBox) throw new Error('Table fragment not visible'); - // Anchor in the paragraph above, sweep down to near the bottom of the table. - // End the sweep within the last content row — not on the table's bottom - // border, where the resolved hit lands on the boundary and re-collapses. + // End the drag squarely on the LAST row's first-column text. Aiming at a + // real cell line (not the table's horizontal/vertical seams) is essential: + // a resting point over a column gap or row border resolves to the table + // boundary and the head snaps back out of the table. The sweep stays in the + // left column the whole way down, so the pointer is always over cell text. + const lastRowCell = superdoc.page + .locator('.superdoc-line') + .filter({ hasText: cellLabel(ROWS, 1) }) + .first(); + const lastRowCellBox = await lastRowCell.boundingBox(); + if (!lastRowCellBox) throw new Error('Last-row cell line not visible'); + const startX = paragraphBox.x + 20; const startY = paragraphBox.y + paragraphBox.height / 2; - const endX = tableBox.x + tableBox.width / 2; - const endY = tableBox.y + tableBox.height * 0.85; + const endX = lastRowCellBox.x + lastRowCellBox.width / 2; + const endY = lastRowCellBox.y + lastRowCellBox.height / 2; const samples = await dragSampling(superdoc, startX, startY, endX, endY, 12); @@ -128,23 +161,28 @@ test.describe('drag selection live feedback in tables (SD-2676)', () => { const inTable = samples.filter((s) => s.y >= tableBox.y); expect(inTable.length).toBeGreaterThan(2); - // Core regression guard: this drag stays a TextSelection (the anchor is - // outside the table), so its size must keep growing while the pointer + // Live-feedback guard: this drag stays a TextSelection (the anchor is + // outside the table), so its size must keep changing while the pointer // sweeps through rows — not freeze at the boundary. The pre-fix bug pinned - // the head to the table boundary, collapsing every in-table sample to a - // single size. More than one distinct size, and a maximum that exceeds the - // first in-table sample, proves the highlight kept updating. + // the head to the table boundary, collapsing every in-table sample to one + // size. More than one distinct in-table size proves the highlight updated. const inTableSizes = inTable.map((s) => s.size); expect(new Set(inTableSizes).size).toBeGreaterThan(1); - expect(Math.max(...inTableSizes)).toBeGreaterThan(inTableSizes[0]); // The painted highlight must be present whenever there is a selection. for (const s of samples) { if (s.size > 0) expect(s.rects).toBeGreaterThan(0); } - // Final selection is non-collapsed. - const finalSel = await superdoc.getSelection(); + // End-state guard (the part that matters to the user): once the pointer + // comes to rest inside the table and the button is released, the committed + // selection must actually reach into the table — spanning from the + // paragraph through the cells the pointer crossed, down to the last row. + // Pre-fix, the head was clamped to the boundary and this collapsed back to + // the paragraph alone. + const finalSel = await getSelectionInfo(superdoc); + expect(finalSel.text).toContain(cellLabel(1, 1)); // reached the first row + expect(finalSel.text).toContain(cellLabel(ROWS, 1)); // through to the last row expect(finalSel.to - finalSel.from).toBeGreaterThan(0); expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); }); @@ -171,10 +209,13 @@ test.describe('drag selection live feedback in tables (SD-2676)', () => { const samples = await dragSampling(superdoc, startX, startY, endX, endY, 10); - // Dragging from inside the table must actually produce a selection — the - // second symptom in SD-2676 was that no text got selected at all. - const finalSel = await superdoc.getSelection(); + // Dragging from inside the table must actually produce a selection that + // spans the rows the pointer crossed — the second symptom in SD-2676 was + // that no text got selected at all. + const finalSel = await getSelectionInfo(superdoc); expect(finalSel.to - finalSel.from).toBeGreaterThan(0); + expect(finalSel.text).toContain(cellLabel(1, 1)); + expect(finalSel.text).toContain(cellLabel(ROWS, 1)); expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); // A same-table drag resolves to a CellSelection, whose paint expands across @@ -209,8 +250,10 @@ test.describe('drag selection live feedback in tables (SD-2676)', () => { const samples = await dragSampling(superdoc, startX, startY, endX, endY, 10); - const finalSel = await superdoc.getSelection(); + const finalSel = await getSelectionInfo(superdoc); expect(finalSel.to - finalSel.from).toBeGreaterThan(0); + expect(finalSel.text).toContain(cellLabel(1, 1)); + expect(finalSel.text).toContain(cellLabel(ROWS, 1)); expect(await getSelectionOverlayRectCount(superdoc)).toBeGreaterThan(0); // The highlight expands across rows as the pointer ascends.