Skip to content

Commit

Permalink
[lexical-table] Bug Fix: Prevent adjacent cell selection on triple-cl…
Browse files Browse the repository at this point in the history
…ick (#7213)

Co-authored-by: Bob Ippolito <[email protected]>
  • Loading branch information
kirandash and etrepum authored Feb 24, 2025
1 parent dfce1f1 commit 82e9477
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 53 deletions.
78 changes: 25 additions & 53 deletions packages/lexical-playground/__tests__/e2e/Selection.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -867,60 +867,32 @@ test.describe.parallel('Selection', () => {
const tripleClickDelay = 50;
await lastCellText.click({clickCount: 3, delay: tripleClickDelay});

if (browserName === 'firefox') {
// Firefox correctly selects the last cell + full text content, unlike Chromium which selects the first cell
const expectedSelection = createHumanReadableSelection(
'the full text of the last cell in the table',
{
anchorOffset: {desc: 'beginning of cell', value: 0},
anchorPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'second cell', value: 1},
{desc: 'first paragraph', value: 0},
{desc: 'first span', value: 0},
{desc: 'beginning of text', value: 0},
],
focusOffset: {desc: 'full text length', value: cellText.length},
focusPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'second cell', value: 1},
{desc: 'first paragraph', value: 0},
{desc: 'first span', value: 0},
{desc: 'beginning of text', value: 0},
],
},
);
await assertSelection(page, expectedSelection);
} else {
// Only the first cell should be selected, and not the entire document
// Ideally the last cell should be selected since that was what was triple-clicked,
// but due to the complex selection logic involved, this was the best that could be done.
// The goal ultimately was to prevent dangerous whole document selection.
// Getting the last cell precisely selected can be done at a later point.
const expectedSelection = createHumanReadableSelection(
'cursor at beginning of the first cell in table',
{
anchorOffset: {desc: 'beginning of cell', value: 0},
anchorPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'first cell', value: 0},
{desc: 'beginning of text', value: 0},
],
focusOffset: {desc: 'beginning of text', value: 0},
focusPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'first cell', value: 0},
{desc: 'beginning of text', value: 0},
],
},
);
// Expect consistent behavior - select the clicked cell's content
const expectedSelection = createHumanReadableSelection(
'the full text of the last cell in the table',
{
anchorOffset: {desc: 'beginning of cell', value: 0},
anchorPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'second cell', value: 1},
{desc: 'first paragraph', value: 0},
{desc: 'first span', value: 0},
{desc: 'beginning of text', value: 0},
],
focusOffset: {desc: 'full text length', value: cellText.length},
focusPath: [
{desc: 'index of table in root', value: 1},
{desc: 'first table row', value: 1},
{desc: 'second cell', value: 1},
{desc: 'first paragraph', value: 0},
{desc: 'first span', value: 0},
{desc: 'beginning of text', value: 0},
],
},
);

await assertSelection(page, expectedSelection);
}
await assertSelection(page, expectedSelection);
});

/**
Expand Down
35 changes: 35 additions & 0 deletions packages/lexical-table/src/LexicalTablePluginHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,23 @@
*/

import {
$findMatchingParent,
$insertFirst,
$insertNodeToNearestRoot,
$unwrapAndFilterDescendants,
mergeRegister,
} from '@lexical/utils';
import {
$createParagraphNode,
$getNearestNodeFromDOMNode,
$getSelection,
$isElementNode,
$isRangeSelection,
$isTextNode,
CLICK_COMMAND,
COMMAND_PRIORITY_EDITOR,
ElementNode,
isDOMNode,
LexicalEditor,
NodeKey,
SELECTION_INSERT_CLIPBOARD_NODES_COMMAND,
Expand Down Expand Up @@ -136,6 +142,29 @@ function $tableTransform(node: TableNode) {
}
}

function $tableClickCommand(event: MouseEvent): boolean {
if (event.detail < 3 || !isDOMNode(event.target)) {
return false;
}
const startNode = $getNearestNodeFromDOMNode(event.target);
if (startNode === null) {
return false;
}
const blockNode = $findMatchingParent(
startNode,
(node): node is ElementNode => $isElementNode(node) && !node.isInline(),
);
if (blockNode === null) {
return false;
}
const rootNode = blockNode.getParent();
if (!$isTableCellNode(rootNode)) {
return false;
}
blockNode.select(0);
return true;
}

/**
* Register a transform to ensure that all TableCellNode have a colSpan and rowSpan of 1.
* This should only be registered when you do not want to support merged cells.
Expand Down Expand Up @@ -276,6 +305,7 @@ export function registerTablePlugin(editor: LexicalEditor): () => void {
if (!editor.hasNodes([TableNode])) {
invariant(false, 'TablePlugin: TableNode is not registered on editor');
}

return mergeRegister(
editor.registerCommand(
INSERT_TABLE_COMMAND,
Expand All @@ -294,6 +324,11 @@ export function registerTablePlugin(editor: LexicalEditor): () => void {
},
COMMAND_PRIORITY_EDITOR,
),
editor.registerCommand(
CLICK_COMMAND,
$tableClickCommand,
COMMAND_PRIORITY_EDITOR,
),
editor.registerNodeTransform(TableNode, $tableTransform),
editor.registerNodeTransform(TableRowNode, $tableRowTransform),
editor.registerNodeTransform(TableCellNode, $tableCellTransform),
Expand Down
23 changes: 23 additions & 0 deletions packages/lexical-table/src/LexicalTableSelectionHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,26 @@ export function applyTableHandlers(
onMouseDown,
tableObserver.listenerOptions,
);
tableObserver.listenersToRemove.add(() => {
tableElement.removeEventListener('mousedown', onMouseDown);
});

const onTripleClick = (event: MouseEvent) => {
if (event.detail >= 3 && isDOMNode(event.target)) {
const targetCell = getDOMCellFromTarget(event.target);
if (targetCell !== null) {
event.preventDefault();
}
}
};
tableElement.addEventListener(
'mousedown',
onTripleClick,
tableObserver.listenerOptions,
);
tableObserver.listenersToRemove.add(() => {
tableElement.removeEventListener('mousedown', onTripleClick);
});

// Clear selection when clicking outside of dom.
const mouseDownCallback = (event: MouseEvent) => {
Expand All @@ -312,6 +332,9 @@ export function applyTableHandlers(
mouseDownCallback,
tableObserver.listenerOptions,
);
tableObserver.listenersToRemove.add(() => {
editorWindow.removeEventListener('mousedown', mouseDownCallback);
});

for (const [command, direction] of ARROW_KEY_COMMANDS_WITH_DIRECTION) {
tableObserver.listenersToRemove.add(
Expand Down

0 comments on commit 82e9477

Please sign in to comment.