Editor bugfixes #479
Editor bugfixes #479
Conversation
zoltanhosszu
force-pushed
the
bugfix+editor-fixes
branch
4 times, most recently
from
e1f9f84 to
c2196d4
Compare
samuelpecher
left a comment
samuelpecher
left a comment
There was a problem hiding this comment.
Some nice QoL improvements. Great find on Undo/Redo. Maybe we can get the node generation a little tighter and delegate node unwrapping to Lexical?
| if (!html) html = "<p></p>" | ||
| const nodes = $generateNodesFromDOM(this.editor, parseHtml(`<div>${html}</div>`)) | ||
| const defaultHtml = "<p></p>" | ||
| if (!html) html = defaultHtml | ||
| let nodes = $generateNodesFromDOM(this.editor, parseHtml(`<div>${html}</div>`)) | ||
|
|
||
| if (nodes.length === 0) { | ||
| nodes = $generateNodesFromDOM(this.editor, parseHtml(defaultHtml)) | ||
| } | ||
|
|
There was a problem hiding this comment.
From the caller in set value=on L30 and its caller in #loadInitialValue on L267, there's varying levels where a default is applied. #parseHtmlIntoLexicalNodes isn't even called if html === "".
I'd be up for avoiding a double call to $generateNodesFromDOM if we can.
There was a problem hiding this comment.
Thanks @samuelpecher ! Would this change make more sense?
There was a problem hiding this comment.
Scratch that, I found a better solution. We needed to check every toolbar action if it was performed on the root element, and handle it just by inserting the required element. I think this is a much better solution.
There was a problem hiding this comment.
@samuelpecher went back to forcing a <p> on empty state. How's this? :)
zoltanhosszu
force-pushed
the
bugfix+editor-fixes
branch
3 times, most recently
from
8ab76cf to
33b90d7
Compare
zoltanhosszu
force-pushed
the
bugfix+editor-fixes
branch
from
33b90d7 to
911d51d
Compare
If the value contained a <pre>, the editor's set value() ended up placing the cursor within the <pre> tag. This caused a selected button state in the toolbar for <>. A sleep function was added to a test to wait for async editor value updates.
Unwrapping a blockquote with simple TextNode (e.g. <blockquote>Hello</blockquote>), caused a Lexical error 99: "Only element or decorator nodes can be inserted in to the root node" This change wraps all TextNodes in a paragraph when unwrapping a blockquote.
Some initial editor values can result in an editor having no nodes inside. This can cause issues for trying to use the toolbar actions, which result in a Lexical error 51, causing a crash. For these cases, we add an empty paragraph node as a default value.
The issue was that we were registering these commands with the toolbar hotkeys, which added a duplication for the built-in undo/redo action
zoltanhosszu
force-pushed
the
bugfix+editor-fixes
branch
from
911d51d to
a10e96e
Compare
Editor bugfixes
This PR fixes a few small issues:
More details in each commit.