Conversation
9f89ff5 to
e6fbe99
Compare
5e6067e to
5c8eb7a
Compare
…e nodes Insert a ProvisionalParagraphNode which becomes a regular node on edit. This node serves as an insertion point target replaced on selection in order to prevent Lexical's use of the fake block selection cursor.
This case is now handled by an auto-created ProvisionalParagraphNode
Previously, the sibling node end was selected, which was wrong if the
sibling node was the next sibling rather than the previous sibling.
Remove await nextFrame() and sequential update functions which are not a
great pattern and { discrete: true } should be used if necessary.
This should be handled by Lexical, otherwise we now get double deletion when Lexical Rich Text handles the DELETE_CHAR_COMMAND
5c8eb7a to
ff05d38
Compare
e6fbe99 to
69cfe58
Compare
packagethief
left a comment
There was a problem hiding this comment.
Looks good, @samuelpecher. I really like the "provisional paragraph" naming. Just one suggestion for a possibly-clarifying refactor.
|
|
||
| const previousOrNextSibling = previousSibling || lastNode && $getNextSiblingOrParentSibling(lastNode)?.[0] | ||
|
|
||
| return [ previousOrNextSibling, isPreviousSibling ] |
There was a problem hiding this comment.
I think we should probably rename this function now that it returns more detail, namely the direction. Renaming to something like findAdjacentNodeWithDirection would clarify. If you wanted to be more explicit, returning an Object would let you name the values:
#findAdjacentNodeWithDirection(nodes) {
const firstNode = nodes[0]
const lastNode = nodes[nodes.length - 1]
const previousSibling = firstNode?.getPreviousSibling()
const nextSibling = lastNode && $getNextSiblingOrParentSibling(lastNode)?.[0]
if (previousSibling) {
return { node: previousSibling, direction: "previous" }
} else if (nextSibling) {
return { node: nextSibling, direction: "next" }
} else {
return { node: null, direction: null }
}
}But actually, maybe there's an opportunity to be more targeted with naming and purpose, since we only use these functions for one thing. Purpose-named functions are better than loosely connected variables. With something like this, we describe the intent, not the mechanism:
deleteSelectedNodes() {
this.editor.update(() => {
if (!this.#selection.hasNodeSelection) return
const nodesToRemove = $getSelection().getNodes()
if (nodesToRemove.length === 0) return
const focusTarget = this.#findFocusTargetAfterDeletion(nodesToRemove)
this.#deleteNodes(nodesToRemove)
this.#selectAfterDeletion(focusTarget)
this.editor.focus()
})
}
#findFocusTargetAfterDeletion(nodes) {
const firstNode = nodes[0]
const lastNode = nodes[nodes.length - 1]
const previousSibling = firstNode?.getPreviousSibling()
const nextSibling = lastNode && $getNextSiblingOrParentSibling(lastNode)?.[0]
if (previousSibling) {
return { node: previousSibling, direction: "previous" }
} else if (nextSibling) {
return { node: nextSibling, direction: "next" }
} else {
return { node: null, direction: null }
}
}
#selectAfterDeletion({ node, direction }) {
const root = $getRoot()
if (root.getChildrenSize() === 0) {
const newParagraph = $createParagraphNode()
root.append(newParagraph)
newParagraph.selectStart()
} else if (node) {
if ($isTextNode(node) || $isParagraphNode(node)) {
direction === "previous" ? node.selectEnd() : node.selectStart()
} else {
direction === "previous" ? node.selectNext(0, 0) : node.select()
}
}
}What do you think? If you think it's a bit too much, renaming the function to include the directionality bit would go a long way 😉
| } | ||
| } | ||
|
|
||
| function $removeUneededProvisionalParagraphs(rootNode) { |
There was a problem hiding this comment.
Typo: "uneeded" -> "unneeded"
Introduce a ProvisionalParagraphNode and accompanying extension to solve the issue of Lexical selection between DecoratorNodes and the editor boundaries. These nodes serve as selection targets for the cursor without rendering output. Previously Lexical would introduce a fake cursor which appeared foreign and behaved sub-optimally, particularly in image galleries (to be introduced in #716).
ProvisionalParagraphs are created when aDecoratorNodeor non-textElementNodeare neighbors or at the top/bottom of the editor.ProvisionalParagraphis added to an empty editorParagraphNodeon editI've also defined the transforms directly on the node, with Lexical's new
static transformAPI, which keeps the code neater.This cleans-up some edge cases:
<p>around nodes to ensure selection is fluid