Skip to content

Commit

Permalink
[lexical-table] Bug Fix: Fix crash in $deleteCellHandler (#6650)
Browse files Browse the repository at this point in the history
  • Loading branch information
etrepum authored Sep 20, 2024
1 parent db6454f commit cf43676
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 24 deletions.
42 changes: 42 additions & 0 deletions packages/lexical-playground/__tests__/e2e/Tables.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
copyToClipboard,
deleteTableColumns,
deleteTableRows,
expect,
focusEditor,
html,
initialize,
Expand Down Expand Up @@ -1360,6 +1361,47 @@ test.describe.parallel('Tables', () => {
);
});

test('Can delete all with range selection anchored in table', async ({
page,
isCollab,
isPlainText,
}) => {
test.skip(isPlainText || isCollab);
await initialize({isCollab, page});
await focusEditor(page);
await insertTable(page, 1, 1);
// Remove paragraph before
await moveUp(page);
await page.keyboard.press('Backspace');
await assertHTML(
page,
html`
<table class="PlaygroundEditorTheme__table">
<colgroup><col style="width: 92px" /></colgroup>
<tr>
<th
class="PlaygroundEditorTheme__tableCell PlaygroundEditorTheme__tableCellHeader">
<p class="PlaygroundEditorTheme__paragraph"><br /></p>
</th>
</tr>
</table>
<p class="PlaygroundEditorTheme__paragraph"><br /></p>
`,
);
// Select all but from the table
const modifier = process.platform === 'darwin' ? 'Meta' : 'Control';
await page.keyboard.press(`${modifier}+A`);
// The observer is active
await expect(page.locator('.table-cell-action-button')).toBeVisible();
await page.keyboard.press('Backspace');
await assertHTML(
page,
html`
<p class="PlaygroundEditorTheme__paragraph"><br /></p>
`,
);
});

test(`Horizontal rule inside cell`, async ({page, isPlainText, isCollab}) => {
await initialize({isCollab, page});
test.skip(isPlainText);
Expand Down
47 changes: 23 additions & 24 deletions packages/lexical-table/src/LexicalTableSelectionHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,22 +336,30 @@ export function applyTableHandlers(
event: KeyboardEvent | ClipboardEvent | null,
): boolean => {
const selection = $getSelection();
if (!($isTableSelection(selection) || $isRangeSelection(selection))) {
return false;
}

if (!$isSelectionInTable(selection, tableNode)) {
const nodes = selection ? selection.getNodes() : null;
if (nodes) {
const table = nodes.find(
(node) =>
$isTableNode(node) && node.getKey() === tableObserver.tableNodeKey,
);
if ($isTableNode(table)) {
const parentNode = table.getParent();
if (!parentNode) {
return false;
}
table.remove();
}
}
// If the selection is inside the table but should remove the whole table
// we expand the selection so that both the anchor and focus are outside
// the table and the editor's command listener will handle the delete
const isAnchorInside = tableNode.isParentOf(selection.anchor.getNode());
const isFocusInside = tableNode.isParentOf(selection.focus.getNode());
if (isAnchorInside !== isFocusInside) {
const tablePoint = isAnchorInside ? 'anchor' : 'focus';
const outerPoint = isAnchorInside ? 'focus' : 'anchor';
// Preserve the outer point
const {key, offset, type} = selection[outerPoint];
// Expand the selection around the table
const newSelection =
tableNode[
selection[tablePoint].isBefore(selection[outerPoint])
? 'selectPrevious'
: 'selectNext'
]();
// Restore the outer point of the selection
newSelection[outerPoint].set(key, offset, type);
// Let the base implementation handle the rest
return false;
}

Expand All @@ -363,15 +371,6 @@ export function applyTableHandlers(
tableObserver.clearText();

return true;
} else if ($isRangeSelection(selection)) {
const tableCellNode = $findMatchingParent(
selection.anchor.getNode(),
(n) => $isTableCellNode(n),
);

if (!$isTableCellNode(tableCellNode)) {
return false;
}
}

return false;
Expand Down

0 comments on commit cf43676

Please sign in to comment.