diff --git a/packages/lexical-playground/__tests__/e2e/Tables.spec.mjs b/packages/lexical-playground/__tests__/e2e/Tables.spec.mjs
index d639bc9e5b3..959d3395395 100644
--- a/packages/lexical-playground/__tests__/e2e/Tables.spec.mjs
+++ b/packages/lexical-playground/__tests__/e2e/Tables.spec.mjs
@@ -665,91 +665,93 @@ test.describe.parallel('Tables', () => {
});
});
- test(`Can select cells using Table selection`, async ({
- page,
- isPlainText,
- isCollab,
- }) => {
- await initialize({isCollab, page});
- test.skip(isPlainText);
+ test(
+ `Can select cells using Table selection`,
+ {
+ tag: '@flaky',
+ },
+ async ({page, isPlainText, isCollab}) => {
+ await initialize({isCollab, page});
+ test.skip(isPlainText);
- await focusEditor(page);
- await insertTable(page, 2, 3);
+ await focusEditor(page);
+ await insertTable(page, 2, 3);
- await fillTablePartiallyWithText(page);
- await selectCellsFromTableCords(
- page,
- {x: 0, y: 0},
- {x: 1, y: 1},
- true,
- false,
- );
+ await fillTablePartiallyWithText(page);
+ await selectCellsFromTableCords(
+ page,
+ {x: 0, y: 0},
+ {x: 1, y: 1},
+ true,
+ false,
+ );
- await assertHTML(
- page,
- html`
-
-
-
- |
- a
- |
-
- bb
- |
-
- cc
- |
-
-
- |
- d
- |
-
- e
- |
-
- f
- |
-
-
-
- `,
- html`
-
-
-
- |
- a
- |
-
- bb
- |
-
- cc
- |
-
-
- |
- d
- |
-
- e
- |
-
- f
- |
-
-
-
- `,
- {ignoreClasses: true},
- );
- });
+ await assertHTML(
+ page,
+ html`
+
+
+
+ |
+ a
+ |
+
+ bb
+ |
+
+ cc
+ |
+
+
+ |
+ d
+ |
+
+ e
+ |
+
+ f
+ |
+
+
+
+ `,
+ html`
+
+
+
+ |
+ a
+ |
+
+ bb
+ |
+
+ cc
+ |
+
+
+ |
+ d
+ |
+
+ e
+ |
+
+ f
+ |
+
+
+
+ `,
+ {ignoreClasses: true},
+ );
+ },
+ );
test(`Can select cells using Table selection via keyboard`, async ({
page,
@@ -1889,9 +1891,10 @@ test.describe.parallel('Tables', () => {
second
-
+ |
@@ -2252,6 +2255,139 @@ test.describe.parallel('Tables', () => {
);
});
+ test('Merge multiple merged cells and then unmerge', async ({
+ page,
+ isPlainText,
+ isCollab,
+ }) => {
+ await initialize({isCollab, page});
+ test.skip(isPlainText);
+
+ await focusEditor(page);
+
+ await insertTable(page, 3, 3);
+
+ await click(page, '.PlaygroundEditorTheme__tableCell');
+ await moveDown(page, 1);
+ await selectCellsFromTableCords(
+ page,
+ {x: 0, y: 0},
+ {x: 0, y: 1},
+ true,
+ true,
+ );
+ await mergeTableCells(page);
+
+ await moveRight(page, 1);
+ await selectCellsFromTableCords(
+ page,
+ {x: 1, y: 0},
+ {x: 2, y: 0},
+ true,
+ true,
+ );
+ await mergeTableCells(page);
+
+ await selectCellsFromTableCords(
+ page,
+ {x: 0, y: 0},
+ {x: 1, y: 0},
+ true,
+ true,
+ );
+ await mergeTableCells(page);
+
+ await assertHTML(
+ page,
+ html`
+
+
+
+
+
+
|
+
+
+ |
+
+ |
+
+
+ |
+
+
+
+ `,
+ );
+
+ await selectCellsFromTableCords(
+ page,
+ {x: 0, y: 0},
+ {x: 0, y: 0},
+ true,
+ true,
+ );
+ await unmergeTableCell(page);
+
+ await assertHTML(
+ page,
+ html`
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ |
+
+ |
+
+
+ |
+
+
+
+ `,
+ );
+ });
+
test('Insert row above (with conflicting merged cell)', async ({
page,
isPlainText,
diff --git a/packages/lexical-playground/src/plugins/MarkdownTransformers/index.ts b/packages/lexical-playground/src/plugins/MarkdownTransformers/index.ts
index de5c1e64824..736267dee54 100644
--- a/packages/lexical-playground/src/plugins/MarkdownTransformers/index.ts
+++ b/packages/lexical-playground/src/plugins/MarkdownTransformers/index.ts
@@ -212,7 +212,10 @@ export const TABLE: ElementTransformer = {
if (!$isTableCellNode(cell)) {
return;
}
- cell.toggleHeaderStyle(TableCellHeaderStates.ROW);
+ cell.setHeaderStyles(
+ TableCellHeaderStates.ROW,
+ TableCellHeaderStates.ROW,
+ );
});
// Remove line
diff --git a/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx b/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx
index 0796241e5e1..60a09ab8a09 100644
--- a/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx
+++ b/packages/lexical-playground/src/plugins/TableActionMenuPlugin/index.tsx
@@ -43,7 +43,6 @@ import {
import * as React from 'react';
import {ReactPortal, useCallback, useEffect, useRef, useState} from 'react';
import {createPortal} from 'react-dom';
-import invariant from 'shared/invariant';
import useModal from '../../hooks/useModal';
import ColorPicker from '../../ui/ColorPicker';
@@ -59,48 +58,6 @@ function computeSelectionCount(selection: TableSelection): {
};
}
-// This is important when merging cells as there is no good way to re-merge weird shapes (a result
-// of selecting merged cells and non-merged)
-function isTableSelectionRectangular(selection: TableSelection): boolean {
- const nodes = selection.getNodes();
- const currentRows: Array = [];
- let currentRow = null;
- let expectedColumns = null;
- let currentColumns = 0;
- for (let i = 0; i < nodes.length; i++) {
- const node = nodes[i];
- if ($isTableCellNode(node)) {
- const row = node.getParentOrThrow();
- invariant(
- $isTableRowNode(row),
- 'Expected CellNode to have a RowNode parent',
- );
- if (currentRow !== row) {
- if (expectedColumns !== null && currentColumns !== expectedColumns) {
- return false;
- }
- if (currentRow !== null) {
- expectedColumns = currentColumns;
- }
- currentRow = row;
- currentColumns = 0;
- }
- const colSpan = node.__colSpan;
- for (let j = 0; j < colSpan; j++) {
- if (currentRows[currentColumns + j] === undefined) {
- currentRows[currentColumns + j] = 0;
- }
- currentRows[currentColumns + j] += node.__rowSpan;
- }
- currentColumns += colSpan;
- }
- }
- return (
- (expectedColumns === null || currentColumns === expectedColumns) &&
- currentRows.every((v) => v === currentRows[0])
- );
-}
-
function $canUnmerge(): boolean {
const selection = $getSelection();
if (
@@ -208,9 +165,7 @@ function TableActionMenu({
const currentSelectionCounts = computeSelectionCount(selection);
updateSelectionCounts(computeSelectionCount(selection));
setCanMergeCells(
- isTableSelectionRectangular(selection) &&
- (currentSelectionCounts.columns > 1 ||
- currentSelectionCounts.rows > 1),
+ currentSelectionCounts.columns > 1 || currentSelectionCounts.rows > 1,
);
}
// Unmerge cell
@@ -407,12 +362,14 @@ function TableActionMenu({
throw new Error('Expected table row');
}
+ const newStyle =
+ tableCellNode.getHeaderStyles() ^ TableCellHeaderStates.ROW;
tableRow.getChildren().forEach((tableCell) => {
if (!$isTableCellNode(tableCell)) {
throw new Error('Expected table cell');
}
- tableCell.toggleHeaderStyle(TableCellHeaderStates.ROW);
+ tableCell.setHeaderStyles(newStyle, TableCellHeaderStates.ROW);
});
clearTableSelection();
@@ -436,6 +393,8 @@ function TableActionMenu({
throw new Error('Expected table cell to be inside of table row.');
}
+ const newStyle =
+ tableCellNode.getHeaderStyles() ^ TableCellHeaderStates.COLUMN;
for (let r = 0; r < tableRows.length; r++) {
const tableRow = tableRows[r];
@@ -455,7 +414,7 @@ function TableActionMenu({
throw new Error('Expected table cell');
}
- tableCell.toggleHeaderStyle(TableCellHeaderStates.COLUMN);
+ tableCell.setHeaderStyles(newStyle, TableCellHeaderStates.COLUMN);
}
clearTableSelection();
onClose();
diff --git a/packages/lexical-table/flow/LexicalTable.js.flow b/packages/lexical-table/flow/LexicalTable.js.flow
index 7227722befa..0c58fc56374 100644
--- a/packages/lexical-table/flow/LexicalTable.js.flow
+++ b/packages/lexical-table/flow/LexicalTable.js.flow
@@ -62,12 +62,12 @@ declare export class TableCellNode extends ElementNode {
getRowSpan(): number;
setRowSpan(rowSpan: number): this;
getTag(): string;
- setHeaderStyles(headerState: TableCellHeaderState): TableCellHeaderState;
+ setHeaderStyles(headerState: TableCellHeaderState, mask?: TableCellHeaderState): TableCellNode;
getHeaderStyles(): TableCellHeaderState;
- setWidth(width: number): ?number;
+ setWidth(width: number): TableCellNode;
getWidth(): ?number;
getBackgroundColor(): null | string;
- setBackgroundColor(newBackgroundColor: null | string): void;
+ setBackgroundColor(newBackgroundColor: null | string): TableCellNode;
toggleHeaderStyle(headerState: TableCellHeaderState): TableCellNode;
hasHeader(): boolean;
updateDOM(prevNode: TableCellNode): boolean;
diff --git a/packages/lexical-table/src/LexicalTableCellNode.ts b/packages/lexical-table/src/LexicalTableCellNode.ts
index 455d39bf6c9..7f752777dc0 100644
--- a/packages/lexical-table/src/LexicalTableCellNode.ts
+++ b/packages/lexical-table/src/LexicalTableCellNode.ts
@@ -69,15 +69,18 @@ export class TableCellNode extends ElementNode {
}
static clone(node: TableCellNode): TableCellNode {
- const cellNode = new TableCellNode(
+ return new TableCellNode(
node.__headerState,
node.__colSpan,
node.__width,
node.__key,
);
- cellNode.__rowSpan = node.__rowSpan;
- cellNode.__backgroundColor = node.__backgroundColor;
- return cellNode;
+ }
+
+ afterCloneFrom(node: this): void {
+ super.afterCloneFrom(node);
+ this.__rowSpan = node.__rowSpan;
+ this.__backgroundColor = node.__backgroundColor;
}
static importDOM(): DOMConversionMap | null {
@@ -96,14 +99,13 @@ export class TableCellNode extends ElementNode {
static importJSON(serializedNode: SerializedTableCellNode): TableCellNode {
const colSpan = serializedNode.colSpan || 1;
const rowSpan = serializedNode.rowSpan || 1;
- const cellNode = $createTableCellNode(
+ return $createTableCellNode(
serializedNode.headerState,
colSpan,
serializedNode.width || undefined,
- );
- cellNode.__rowSpan = rowSpan;
- cellNode.__backgroundColor = serializedNode.backgroundColor || null;
- return cellNode;
+ )
+ .setRowSpan(rowSpan)
+ .setBackgroundColor(serializedNode.backgroundColor || null);
}
constructor(
@@ -190,41 +192,46 @@ export class TableCellNode extends ElementNode {
}
getColSpan(): number {
- return this.__colSpan;
+ return this.getLatest().__colSpan;
}
setColSpan(colSpan: number): this {
- this.getWritable().__colSpan = colSpan;
- return this;
+ const self = this.getWritable();
+ self.__colSpan = colSpan;
+ return self;
}
getRowSpan(): number {
- return this.__rowSpan;
+ return this.getLatest().__rowSpan;
}
setRowSpan(rowSpan: number): this {
- this.getWritable().__rowSpan = rowSpan;
- return this;
+ const self = this.getWritable();
+ self.__rowSpan = rowSpan;
+ return self;
}
getTag(): string {
return this.hasHeader() ? 'th' : 'td';
}
- setHeaderStyles(headerState: TableCellHeaderState): TableCellHeaderState {
+ setHeaderStyles(
+ headerState: TableCellHeaderState,
+ mask: TableCellHeaderState = TableCellHeaderStates.BOTH,
+ ): this {
const self = this.getWritable();
- self.__headerState = headerState;
- return this.__headerState;
+ self.__headerState = (headerState & mask) | (self.__headerState & ~mask);
+ return self;
}
getHeaderStyles(): TableCellHeaderState {
return this.getLatest().__headerState;
}
- setWidth(width: number): number | null | undefined {
+ setWidth(width: number): this {
const self = this.getWritable();
self.__width = width;
- return this.__width;
+ return self;
}
getWidth(): number | undefined {
@@ -235,11 +242,13 @@ export class TableCellNode extends ElementNode {
return this.getLatest().__backgroundColor;
}
- setBackgroundColor(newBackgroundColor: null | string): void {
- this.getWritable().__backgroundColor = newBackgroundColor;
+ setBackgroundColor(newBackgroundColor: null | string): this {
+ const self = this.getWritable();
+ self.__backgroundColor = newBackgroundColor;
+ return self;
}
- toggleHeaderStyle(headerStateToToggle: TableCellHeaderState): TableCellNode {
+ toggleHeaderStyle(headerStateToToggle: TableCellHeaderState): this {
const self = this.getWritable();
if ((self.__headerState & headerStateToToggle) === headerStateToToggle) {
diff --git a/packages/lexical-table/src/LexicalTableSelection.ts b/packages/lexical-table/src/LexicalTableSelection.ts
index f4e94bdc87f..acb80d2a20d 100644
--- a/packages/lexical-table/src/LexicalTableSelection.ts
+++ b/packages/lexical-table/src/LexicalTableSelection.ts
@@ -157,8 +157,8 @@ export class TableSelection implements BaseSelection {
focusCellNodeRect.columnIndex,
);
const stopX = Math.max(
- anchorCellNodeRect.columnIndex,
- focusCellNodeRect.columnIndex,
+ anchorCellNodeRect.columnIndex + anchorCellNodeRect.colSpan - 1,
+ focusCellNodeRect.columnIndex + focusCellNodeRect.colSpan - 1,
);
const startY = Math.min(
@@ -166,8 +166,8 @@ export class TableSelection implements BaseSelection {
focusCellNodeRect.rowIndex,
);
const stopY = Math.max(
- anchorCellNodeRect.rowIndex,
- focusCellNodeRect.rowIndex,
+ anchorCellNodeRect.rowIndex + anchorCellNodeRect.rowSpan - 1,
+ focusCellNodeRect.rowIndex + focusCellNodeRect.rowSpan - 1,
);
return {
@@ -306,7 +306,11 @@ export class TableSelection implements BaseSelection {
}
}
- const nodes: Array = [tableNode];
+ // We use a Map here because merged cells in the grid would otherwise
+ // show up multiple times in the nodes array
+ const nodeMap: Map = new Map([
+ [tableNode.getKey(), tableNode],
+ ]);
let lastRow = null;
for (let i = minRow; i <= maxRow; i++) {
for (let j = minColumn; j <= maxColumn; j++) {
@@ -317,12 +321,16 @@ export class TableSelection implements BaseSelection {
'Expected TableCellNode parent to be a TableRowNode',
);
if (currentRow !== lastRow) {
- nodes.push(currentRow);
+ nodeMap.set(currentRow.getKey(), currentRow);
+ }
+ nodeMap.set(cell.getKey(), cell);
+ for (const child of $getChildrenRecursively(cell)) {
+ nodeMap.set(child.getKey(), child);
}
- nodes.push(cell, ...$getChildrenRecursively(cell));
lastRow = currentRow;
}
}
+ const nodes = Array.from(nodeMap.values());
if (!isCurrentlyReadOnlyMode()) {
this._cachedNodes = nodes;
diff --git a/packages/lexical-table/src/LexicalTableUtils.ts b/packages/lexical-table/src/LexicalTableUtils.ts
index 3293881e823..d3e7d5ab888 100644
--- a/packages/lexical-table/src/LexicalTableUtils.ts
+++ b/packages/lexical-table/src/LexicalTableUtils.ts
@@ -557,7 +557,7 @@ export function $deleteTableRow__EXPERIMENTAL(): void {
const rowNode = grid.getChildAtIndex(row);
invariant(
$isTableRowNode(rowNode),
- 'Expected GridNode childAtIndex(%s) to be RowNode',
+ 'Expected TableNode childAtIndex(%s) to be RowNode',
String(row),
);
rowNode.remove();
@@ -673,19 +673,43 @@ export function $unmergeCell(): void {
const [cell, row, grid] = $getNodeTriplet(anchor);
const colSpan = cell.__colSpan;
const rowSpan = cell.__rowSpan;
+ if (colSpan === 1 && rowSpan === 1) {
+ return;
+ }
+ const [map, cellMap] = $computeTableMap(grid, cell, cell);
+ const {startColumn, startRow} = cellMap;
+ // Create a heuristic for what the style of the unmerged cells should be
+ // based on whether every row or column already had that state before the
+ // unmerge.
+ const baseColStyle = cell.__headerState & TableCellHeaderStates.COLUMN;
+ const colStyles = Array.from({length: colSpan}, (_v, i) => {
+ let colStyle = baseColStyle;
+ for (let rowIdx = 0; colStyle !== 0 && rowIdx < map.length; rowIdx++) {
+ colStyle &= map[rowIdx][i + startColumn].cell.__headerState;
+ }
+ return colStyle;
+ });
+ const baseRowStyle = cell.__headerState & TableCellHeaderStates.ROW;
+ const rowStyles = Array.from({length: rowSpan}, (_v, i) => {
+ let rowStyle = baseRowStyle;
+ for (let colIdx = 0; rowStyle !== 0 && colIdx < map[0].length; colIdx++) {
+ rowStyle &= map[i + startRow][colIdx].cell.__headerState;
+ }
+ return rowStyle;
+ });
+
if (colSpan > 1) {
for (let i = 1; i < colSpan; i++) {
cell.insertAfter(
- $createTableCellNode(TableCellHeaderStates.NO_STATUS).append(
+ $createTableCellNode(colStyles[i] | rowStyles[0]).append(
$createParagraphNode(),
),
);
}
cell.setColSpan(1);
}
+
if (rowSpan > 1) {
- const [map, cellMap] = $computeTableMap(grid, cell, cell);
- const {startColumn, startRow} = cellMap;
let currentRowNode;
for (let i = 1; i < rowSpan; i++) {
const currentRow = startRow + i;
@@ -707,18 +731,18 @@ export function $unmergeCell(): void {
}
}
if (insertAfterCell === null) {
- for (let j = 0; j < colSpan; j++) {
+ for (let j = colSpan - 1; j >= 0; j--) {
$insertFirst(
currentRowNode,
- $createTableCellNode(TableCellHeaderStates.NO_STATUS).append(
+ $createTableCellNode(colStyles[j] | rowStyles[i]).append(
$createParagraphNode(),
),
);
}
} else {
- for (let j = 0; j < colSpan; j++) {
+ for (let j = colSpan - 1; j >= 0; j--) {
insertAfterCell.insertAfter(
- $createTableCellNode(TableCellHeaderStates.NO_STATUS).append(
+ $createTableCellNode(colStyles[j] | rowStyles[i]).append(
$createParagraphNode(),
),
);
@@ -739,8 +763,8 @@ export function $computeTableMap(
cellA,
cellB,
);
- invariant(cellAValue !== null, 'Anchor not found in Grid');
- invariant(cellBValue !== null, 'Focus not found in Grid');
+ invariant(cellAValue !== null, 'Anchor not found in Table');
+ invariant(cellBValue !== null, 'Focus not found in Table');
return [tableMap, cellAValue, cellBValue];
}
@@ -752,52 +776,62 @@ export function $computeTableMapSkipCellCheck(
const tableMap: TableMapType = [];
let cellAValue: null | TableMapValueType = null;
let cellBValue: null | TableMapValueType = null;
- function write(startRow: number, startColumn: number, cell: TableCellNode) {
- const value = {
- cell,
- startColumn,
- startRow,
- };
- const rowSpan = cell.__rowSpan;
- const colSpan = cell.__colSpan;
- for (let i = 0; i < rowSpan; i++) {
- if (tableMap[startRow + i] === undefined) {
- tableMap[startRow + i] = [];
- }
- for (let j = 0; j < colSpan; j++) {
- tableMap[startRow + i][startColumn + j] = value;
- }
- }
- if (cellA !== null && cellA.is(cell)) {
- cellAValue = value;
- }
- if (cellB !== null && cellB.is(cell)) {
- cellBValue = value;
+ function getMapRow(i: number) {
+ let row = tableMap[i];
+ if (row === undefined) {
+ tableMap[i] = row = [];
}
+ return row;
}
- function isEmpty(row: number, column: number) {
- return tableMap[row] === undefined || tableMap[row][column] === undefined;
- }
-
const gridChildren = grid.getChildren();
- for (let i = 0; i < gridChildren.length; i++) {
- const row = gridChildren[i];
+ for (let rowIdx = 0; rowIdx < gridChildren.length; rowIdx++) {
+ const row = gridChildren[rowIdx];
invariant(
$isTableRowNode(row),
- 'Expected GridNode children to be TableRowNode',
+ 'Expected TableNode children to be TableRowNode',
);
- const rowChildren = row.getChildren();
- let j = 0;
- for (const cell of rowChildren) {
+ for (
+ let cell = row.getFirstChild(), colIdx = 0;
+ cell != null;
+ cell = cell.getNextSibling()
+ ) {
invariant(
$isTableCellNode(cell),
'Expected TableRowNode children to be TableCellNode',
);
- while (!isEmpty(i, j)) {
- j++;
+ // Skip past any columns that were merged from a higher row
+ const startMapRow = getMapRow(rowIdx);
+ while (startMapRow[colIdx] !== undefined) {
+ colIdx++;
+ }
+ const value: TableMapValueType = {
+ cell,
+ startColumn: colIdx,
+ startRow: rowIdx,
+ };
+ const {__rowSpan: rowSpan, __colSpan: colSpan} = cell;
+ for (let j = 0; j < rowSpan; j++) {
+ if (rowIdx + j >= gridChildren.length) {
+ // The table is non-rectangular with a rowSpan
+ // below the last in the table.
+ // We should probably handle this with a node transform
+ // to ensure that tables are always rectangular but this
+ // will avoid crashes such as #6584
+ // Note that there are probably still latent bugs
+ // regarding colSpan or general cell count mismatches.
+ break;
+ }
+ const mapRow = getMapRow(rowIdx + j);
+ for (let i = 0; i < colSpan; i++) {
+ mapRow[colIdx + i] = value;
+ }
+ }
+ if (cellA !== null && cellAValue === null && cellA.is(cell)) {
+ cellAValue = value;
+ }
+ if (cellB !== null && cellBValue === null && cellB.is(cell)) {
+ cellBValue = value;
}
- write(i, j, cell);
- j += cell.__colSpan;
}
}
return [tableMap, cellAValue, cellBValue];
@@ -832,7 +866,7 @@ export function $getNodeTriplet(
const grid = row.getParent();
invariant(
$isTableNode(grid),
- 'Expected TableRowNode to have a parent GridNode',
+ 'Expected TableRowNode to have a parent TableNode',
);
return [cell, row, grid];
}