[lexical-react] Fix multiple node selection deletion

[ivailop7]

Selecting multiple nodes using Cmd + Shift + Click, which results in a NodeSelection with multiple nodes currently doesn't delete all of the selected nodes.

Before:

before_node_selection_fix.mov

After:

after_node_selection_fix.mov

Request for comment: The $onDelete method is super repetitive and identical for the exception of the 'isNodeOfType' check. Since most of these are in the playground, could do an util function, but an alternative solution is to modify this on the 'rich-text' plugin level deleteHandler and remove all of the $onDelete methods and overrides. Thoughts?

@zurfyx @etrepum @potatowagon

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2024 5:20pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 22, 2024 5:20pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.38 KB (0%)
lexical - esm	29.24 KB (0%)
@lexical/rich-text - cjs	37.74 KB (0%)
@lexical/rich-text - esm	31.05 KB (0%)
@lexical/plain-text - cjs	36.41 KB (0%)
@lexical/plain-text - esm	28.42 KB (0%)
@lexical/react - cjs	39.64 KB (+0.04% 🔺)
@lexical/react - esm	32.51 KB (0%)

[zurfyx]

Request for comment: The $onDelete method is super repetitive and identical for the exception of the 'isNodeOfType' check. Since most of these are in the playground, could do an util function, but an alternative solution is to modify this on the 'rich-text' plugin level deleteHandler and remove all of the $onDelete methods and overrides. Thoughts?

Agree! IMO I'd make it into the rich/plaintext plugins as a command to mimic what we already do with RangeSelection.

REMOVE_NODE_COMMAND would be the equivalent of DELETE_CHARACTER_COMMAND. We implement REMOVE_NODE_COMMAND either in the selection module or rich/plain plugin and allow for plugins to override it if necessary.

Unfortunately, this non backward compatible change can accidentally break some products so we might need a version bump at the very least or rethink how we ship this. FWIW, when selected nodes > 1 this is always the desired behavior since you're not expected to interact with the node without resetting the selection.

[ivailop7]

Request for comment: The $onDelete method is super repetitive and identical for the exception of the 'isNodeOfType' check. Since most of these are in the playground, could do an util function, but an alternative solution is to modify this on the 'rich-text' plugin level deleteHandler and remove all of the $onDelete methods and overrides. Thoughts?

Agree! IMO I'd make it into the rich/plaintext plugins as a command to mimic what we already do with RangeSelection.

REMOVE_NODE_COMMAND would be the equivalent of DELETE_CHARACTER_COMMAND. We implement REMOVE_NODE_COMMAND either in the selection module or rich/plain plugin and allow for plugins to override it if necessary.

Unfortunately, this non backward compatible change can accidentally break some products so we might need a version bump at the very least or rethink how we ship this. FWIW, when selected nodes > 1 this is always the desired behavior since you're not expected to interact with the node without resetting the selection.

OK, will merge this as is then and think on the REMOVE_NODE_COMMAND intro