-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(richtext-lexical): prevent extra paragraph when inserting blocks or uploadNodes. Add preemptive selection normalization #12077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…locks or uploadNodes. Add a preventative plugin to normalize the selection
// Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist | ||
$insertNodeToNearestRoot(blockNode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key point of the fix. We still insert before removing focusNode, but after getting focusNode, since $insertNodeToNearestRoot
can change the selection.
// First, delete currently selected node if it's an empty paragraph and if there are sufficient | ||
// paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user | ||
if ( | ||
$isParagraphNode(focusNode) && | ||
focusNode.getTextContentSize() === 0 && | ||
focusNode | ||
.getParentOrThrow() | ||
.getChildren() | ||
.filter((node) => $isParagraphNode(node)).length > 1 | ||
) { | ||
// Delete the node it it's an empty paragraph | ||
if ($isParagraphNode(focusNode) && !focusNode.__first) { | ||
focusNode.remove() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since PR #10530 this is no longer necessary
@@ -112,6 +107,7 @@ export const LexicalEditor: React.FC< | |||
} | |||
ErrorBoundary={LexicalErrorBoundary} | |||
/> | |||
<NormalizeSelectionPlugin /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was resolved with the changes above, so this isn't strictly necessary, but I decided to add it to make the editor more robust and prevent errors in the future. Additionally, I believe this will fix a bug in the email builder plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed on call and it looks good to me!
🚀 This is included in version v3.36.0 |
…or uploadNodes. Add preemptive selection normalization (#12077) Fixes #11628 PR #6389 caused bug #11628, which is a regression, as it had already been fixed in #4441 It is likely that some things have changed because [Lexical had recently made improvements](facebook/lexical#7046) to address selection normalization. Although it wasn't necessary to resolve the issue, I added a `NormalizeSelectionPlugin` to the editor, which makes selection handling in the editor more robust. I'm also adding a new collection to the Lexical test suite, intending it to be used by default for most tests going forward. I've left an explanatory comment on the dashboard. ___ Looking at #11628's video, it seems users also want to be able to prevent the first paragraph from being empty. This makes sense to me, so I think in another PR we could add a button at the top, just [like we did at the bottom of the editor](#10530).
…or uploadNodes. Add preemptive selection normalization (#12077) Fixes #11628 PR #6389 caused bug #11628, which is a regression, as it had already been fixed in #4441 It is likely that some things have changed because [Lexical had recently made improvements](facebook/lexical#7046) to address selection normalization. Although it wasn't necessary to resolve the issue, I added a `NormalizeSelectionPlugin` to the editor, which makes selection handling in the editor more robust. I'm also adding a new collection to the Lexical test suite, intending it to be used by default for most tests going forward. I've left an explanatory comment on the dashboard. ___ Looking at #11628's video, it seems users also want to be able to prevent the first paragraph from being empty. This makes sense to me, so I think in another PR we could add a button at the top, just [like we did at the bottom of the editor](#10530).
Fixes #11628
PR #6389 caused bug #11628, which is a regression, as it had already been fixed in #4441
It is likely that some things have changed because Lexical had recently made improvements to address selection normalization.
Although it wasn't necessary to resolve the issue, I added a
NormalizeSelectionPlugin
to the editor, which makes selection handling in the editor more robust.I'm also adding a new collection to the Lexical test suite, intending it to be used by default for most tests going forward. I've left an explanatory comment on the dashboard.
Looking at #11628's video, it seems users also want to be able to prevent the first paragraph from being empty. This makes sense to me, so I think in another PR we could add a button at the top, just like we did at the bottom of the editor.