facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
19.98k stars 1.7k forks source link

Bug: Unexepected behavior when using append() on ListNode #6793

Closed basile-savouret closed 1 day ago

basile-savouret commented 3 weeks ago

I have copied the column layout and image nodes from the playground in my project and I implemented a backspace handler for the column layout, to be able to remove it. My solution is to append all the children of the layout item into his ancestor and then remove it. Here is the backspace handler i wrote:

const $onDelete = (isBackspace: boolean) => {
      const selection = $getSelection();
      if (
        $isRangeSelection(selection)
      ) {
        const container = $findMatchingParent(
          selection.anchor.getNode(),
          $isLayoutContainerNode,
        );
        const { layoutItemNode, previousSibling } = getLayoutItemInParentsAndFindPreviousSibling(selection.anchor.getNode())
        const previousContainer = layoutItemNode?.getPreviousSibling() ?? container?.getPreviousSibling()
        if (previousContainer && $isElementNode(previousContainer)) {

          if (layoutItemNode && !previousSibling && selection.anchor.offset === 0 && isBackspace) {
            previousContainer.append(...layoutItemNode.getChildren())
            layoutItemNode.remove(false)
            return false
          }

          if (layoutItemNode && layoutItemNode.getChildren().length === 1) {
            const firstChild = layoutItemNode.getFirstChild()
            if ($isParagraphNode(firstChild) && firstChild.isEmpty()) {
              previousContainer.append(...layoutItemNode.getChildren())
              layoutItemNode.remove(false)
            }
          }

        }

      }

      return false
    };

It works fine on every node:

https://github.com/user-attachments/assets/7ab7bc03-1cfe-4775-95fa-f1f6d84bc4c2

But it fails when the ancestor is a ListNode.

The editor stop working and i get those errors Expected node root to have a parent, updateEditor: selection has been lost because the previously selected nodes have been removed and selection wasn't moved to another node. Ensure selection changes after removing/replacing a selected node

When i disable the remove of the layout item and just keep the append of the children i notice the different behavior of the ListNode compared to the others:

Append on a quote node:

https://github.com/user-attachments/assets/3b82b74c-8661-4b0d-9d5f-ecca2e5d3a70

The children are moved into the node like i expect them to be. The selection follows them.

Append on a list node:

https://github.com/user-attachments/assets/0ffcc6a1-19c2-4c83-b271-0dc7b390ecf2

The children are duplicated into the node and the image is lost. The selection stays in the original children.

I believe this is what triggers my bug as when i try to remove the layout item the children are still in and the selection is lost afterwards.

Lexical version: 0.19.0

basile-savouret commented 3 weeks ago

After looking into the source code it seems like this behavior is comming from this line of code: https://github.com/facebook/lexical/blob/af344f3a1647351792519b9fef7d65b564353a66/packages/lexical-list/src/LexicalListNode.ts#L203

else if ($isElementNode(currentNode)) {
          const textNode = $createTextNode(currentNode.getTextContent());
          listItemNode.append(textNode);
}

where it creates a new textNode and only paste the text content inside it ignoring the image that is kept as a decorator.

I can try to resolve this and make a pull request but I don't have enough context and there might be a reason for this special behaviour on ListNode that i don't know of.

Shopiley commented 1 day ago

Hi @etrepum, can this issue be closed now?