Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions packages/layout-engine/layout-bridge/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,18 @@ export const hitTestTableFragment = (
if (!rowMeasure || !row) continue;

// Find the column at localX using column widths
// IMPORTANT: For rows with rowspan cells from above, the first cell may not start at grid column 0.
// We need to calculate the X offset for columns occupied by rowspans.
const firstCellGridStart = rowMeasure.cells[0]?.gridColumnStart ?? 0;
let colX = 0;
// Calculate X offset for columns before the first cell (occupied by rowspans from above)
if (firstCellGridStart > 0 && tableMeasure.columnWidths) {
for (let col = 0; col < firstCellGridStart && col < tableMeasure.columnWidths.length; col++) {
colX += tableMeasure.columnWidths[col];
}
}
const initialColX = colX;

let colIndex = -1;
// Bounds check: skip if row has no cells
if (rowMeasure.cells.length === 0 || row.cells.length === 0) continue;
Expand All @@ -593,8 +604,13 @@ export const hitTestTableFragment = (
}

if (colIndex === -1) {
// Click is to the right of all columns, use the last column
colIndex = rowMeasure.cells.length - 1;
if (localX < initialColX) {
// Click is in a rowspanned area (left of all cells in this row) - use first cell
colIndex = 0;
} else {
// Click is to the right of all columns - use last cell
colIndex = rowMeasure.cells.length - 1;
}
if (colIndex < 0) continue;
}

Expand Down Expand Up @@ -653,7 +669,7 @@ export const hitTestTableFragment = (
measure: tableMeasure,
pageIndex: pageHit.pageIndex,
cellRowIndex: rowIndex,
cellColIndex: colIndex,
cellColIndex: colIndex, // Use cell array index for PM selection (not gridColIndex)
cellBlock: paragraphBlock,
cellMeasure: paragraphMeasure,
localX: Math.max(0, cellLocalX),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest';
import { clickToPosition, hitTestPage } from '../src/index.ts';
import { clickToPosition, hitTestPage, hitTestTableFragment } from '../src/index.ts';
import type { Layout } from '@superdoc/contracts';
import {
simpleLayout,
Expand All @@ -11,6 +11,9 @@ import {
drawingLayout,
drawingBlock,
drawingMeasure,
rowspanTableLayout,
rowspanTableBlock,
rowspanTableMeasure,
} from './mock-data';

describe('clickToPosition', () => {
Expand Down Expand Up @@ -100,3 +103,58 @@ describe('hitTestPage with pageGap', () => {
expect(result?.pageIndex).toBe(1);
});
});

describe('hitTestTableFragment with rowspan (SD-1626 / IT-22)', () => {
// Table is at x:30, y:60, width:300, height:48
// Row 0: y:60-84 (height 24) - has 3 cells
// Row 1: y:84-108 (height 24) - has 2 cells starting at gridColumnStart=1

it('selects first cell when clicking in rowspanned area, not last cell', () => {
// Table structure:
// Row 0: [Cell A (rowspan=2)] [Cell B] [Cell C]
// Row 1: [Cell D] [Cell E]
//
// When clicking in the rowspanned area (column 0) on row 1,
// the first cell in row 1 (Cell D at index 0) should be selected,
// NOT the last cell (Cell E at index 1).

// Click at x=80 (in column 0 area), y=90 (in row 1)
const pageHit = hitTestPage(rowspanTableLayout, { x: 80, y: 90 });
expect(pageHit).not.toBeNull();

if (pageHit) {
// x=80 -> localX=50 (in rowspanned area, column 0 is 0-100)
// y=90 -> localY=30 (row 1 starts at y=24 relative to table)
const result = hitTestTableFragment(pageHit, [rowspanTableBlock], [rowspanTableMeasure], { x: 80, y: 90 });

expect(result).not.toBeNull();
if (result) {
// Should select first cell (index 0), not last cell (index 1)
expect(result.cellColIndex).toBe(0);
// Row should be 1 (the row we clicked on)
expect(result.cellRowIndex).toBe(1);
}
}
});

it('still selects last cell when clicking right of all columns', () => {
// Click at x=320 (right edge of table but still inside), y=90 (row 1)
// Table ends at x=330, so x=320 is still inside
const pageHit = hitTestPage(rowspanTableLayout, { x: 320, y: 90 });
expect(pageHit).not.toBeNull();

if (pageHit) {
// x=320 -> localX=290 (right of all cells: col0=0-100, col1=100-200, col2=200-300)
// But row 1 cells start at gridColumnStart=1, so they span 100-300
// localX=290 is within cell at gridColumnStart=2 (200-300)
const result = hitTestTableFragment(pageHit, [rowspanTableBlock], [rowspanTableMeasure], { x: 320, y: 90 });

expect(result).not.toBeNull();
if (result) {
// Should select the cell at gridColumnStart=2 (last cell in row 1)
expect(result.cellColIndex).toBe(1); // Last cell in row 1
expect(result.cellRowIndex).toBe(1);
}
}
});
});
150 changes: 150 additions & 0 deletions packages/layout-engine/layout-bridge/test/mock-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,153 @@ export const tableLayout: Layout = {
},
],
};

// Mock data for table with rowspan (SD-1626 / IT-22)
// Table structure:
// Row 0: [Cell A (rowspan=2)] [Cell B] [Cell C]
// Row 1: [Cell D] [Cell E] <- Row 1 cells start at gridColumnStart=1
const rowspanTableParagraph = {
kind: 'paragraph',
id: 'rowspan-cell-para',
runs: [{ text: 'Cell', fontFamily: 'Arial', fontSize: 14, pmStart: 1, pmEnd: 5 }],
} as const;

export const rowspanTableBlock: FlowBlock = {
kind: 'table',
id: 'rowspan-table-0',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-a',
blocks: [rowspanTableParagraph],
attrs: { rowspan: 2, padding: { top: 2, bottom: 2, left: 4, right: 4 } },
},
{ id: 'cell-b', blocks: [rowspanTableParagraph], attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } } },
{ id: 'cell-c', blocks: [rowspanTableParagraph], attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } } },
],
},
{
id: 'row-1',
cells: [
// No cell at column 0 - occupied by rowspan from above
{ id: 'cell-d', blocks: [rowspanTableParagraph], attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } } },
{ id: 'cell-e', blocks: [rowspanTableParagraph], attrs: { padding: { top: 2, bottom: 2, left: 4, right: 4 } } },
],
},
],
};

export const rowspanTableMeasure: Measure = {
kind: 'table',
rows: [
{
height: 24,
cells: [
{
width: 100,
height: 48,
gridColumnStart: 0,
colSpan: 1,
rowSpan: 2,
blocks: [
{
kind: 'paragraph',
lines: [
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 18 },
],
totalHeight: 18,
},
],
},
{
width: 100,
height: 24,
gridColumnStart: 1,
blocks: [
{
kind: 'paragraph',
lines: [
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 18 },
],
totalHeight: 18,
},
],
},
{
width: 100,
height: 24,
gridColumnStart: 2,
blocks: [
{
kind: 'paragraph',
lines: [
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 18 },
],
totalHeight: 18,
},
],
},
],
},
{
height: 24,
cells: [
// Row 1 cells start at gridColumnStart=1 (column 0 is occupied by rowspan)
{
width: 100,
height: 24,
gridColumnStart: 1,
blocks: [
{
kind: 'paragraph',
lines: [
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 18 },
],
totalHeight: 18,
},
],
},
{
width: 100,
height: 24,
gridColumnStart: 2,
blocks: [
{
kind: 'paragraph',
lines: [
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 18 },
],
totalHeight: 18,
},
],
},
],
},
],
columnWidths: [100, 100, 100],
totalWidth: 300,
totalHeight: 48,
};

export const rowspanTableLayout: Layout = {
pageSize: { w: 400, h: 500 },
pages: [
{
number: 1,
fragments: [
{
kind: 'table',
blockId: 'rowspan-table-0',
fromRow: 0,
toRow: 2,
x: 30,
y: 60,
width: 300,
height: 48,
},
],
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,103 @@ describe('TableSelectionUtilities', () => {
const result = getCellPosFromTableHit(hit, state.doc, blocks);
expect(result).not.toBe(null);
});

it('IT-22: handles rowspan correctly - cellColIndex should be cell array index not grid column', () => {
// Create table with vertically merged cells (rowspan)
// Row 0: [A1 (rowspan=3)] [B1] [C1]
// Row 1: [B2] [C2] <- only 2 cells in array, but grid has 3 columns
// Row 2: [B3] [C3]
const cell = (text: string, rowspan = 1, colspan = 1) =>
tableSchema.node('tableCell', { colspan, rowspan }, [
tableSchema.node('paragraph', null, [tableSchema.text(text)]),
]);
const row = (...cells: ReturnType<typeof cell>[]) => tableSchema.node('tableRow', null, cells);

const table = tableSchema.node('table', null, [
row(cell('A1', 3), cell('B1'), cell('C1')), // First row: 3 cells, A1 spans 3 rows
row(cell('B2'), cell('C2')), // Second row: 2 cells (A column occupied by rowspan)
row(cell('B3'), cell('C3')), // Third row: 2 cells (A column occupied by rowspan)
]);
const doc = tableSchema.node('doc', null, [table]);
const state = EditorState.create({ schema: tableSchema, doc });

const blocks: FlowBlock[] = [{ kind: 'table', id: '0-table', rows: [] }];

// When clicking on cell C2 (grid column 2, but array index 1 in row 1),
// cellColIndex should be 1 (the array index), not 2 (the grid column).
// The fix in hitTestTableFragment now correctly returns cell array index.
const hit: TableHitResult = {
block: { id: '0-table', kind: 'table' } as FlowBlock,
cellRowIndex: 1, // Second row
cellColIndex: 1, // Cell array index (C2 is at index 1 in row 1's cell array)
fragment: {} as TableHitResult['fragment'],
pageIndex: 0,
};

const result = getCellPosFromTableHit(hit, state.doc, blocks);
// Should successfully find the cell position using array index
expect(result).not.toBe(null);
expect(typeof result).toBe('number');

// Verify it's the correct cell (C2) by checking the node at that position
// nodeAt() returns the node starting at that position
if (result !== null) {
const cellNode = state.doc.nodeAt(result);
expect(cellNode).not.toBe(null);
expect(cellNode!.type.name).toBe('tableCell');
// C2 should contain text "C2"
expect(cellNode!.textContent).toBe('C2');
}
});

it('IT-22: handles selecting last column in row with rowspan from previous row', () => {
// This test verifies the specific bug reported in IT-22:
// When a table has rowspan, clicking the last column in affected rows
// would fail because the grid column index exceeded the cell array bounds.
const cell = (text: string, rowspan = 1, colspan = 1) =>
tableSchema.node('tableCell', { colspan, rowspan }, [
tableSchema.node('paragraph', null, [tableSchema.text(text)]),
]);
const row = (...cells: ReturnType<typeof cell>[]) => tableSchema.node('tableRow', null, cells);

// 5-row table with first column merged
const table = tableSchema.node('table', null, [
row(cell('A', 5), cell('B1'), cell('C1')),
row(cell('B2'), cell('C2')),
row(cell('B3'), cell('C3')),
row(cell('B4'), cell('C4')),
row(cell('B5'), cell('C5')),
]);
const doc = tableSchema.node('doc', null, [table]);
const state = EditorState.create({ schema: tableSchema, doc });

const blocks: FlowBlock[] = [{ kind: 'table', id: '0-table', rows: [] }];

// Click on last column (C5) in last row
// Grid column would be 2, but array index is 1 (only 2 cells in row 4)
const hit: TableHitResult = {
block: { id: '0-table', kind: 'table' } as FlowBlock,
cellRowIndex: 4, // Last row (index 4)
cellColIndex: 1, // Last cell in array (C5 is at index 1)
fragment: {} as TableHitResult['fragment'],
pageIndex: 0,
};

const result = getCellPosFromTableHit(hit, state.doc, blocks);
// Before the fix, this would return null because it would look for
// array index 2 which doesn't exist in a row with only 2 cells
expect(result).not.toBe(null);
expect(typeof result).toBe('number');

// Verify it's the correct cell (C5) by checking the node at that position
// nodeAt() returns the node starting at that position
if (result !== null) {
const cellNode = state.doc.nodeAt(result);
expect(cellNode).not.toBe(null);
expect(cellNode!.type.name).toBe('tableCell');
expect(cellNode!.textContent).toBe('C5');
}
});
});

describe('shouldUseCellSelection', () => {
Expand Down
Loading