Skip to content
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

feat(core): add experimental decorator element node and nested root node #5981

Closed
wants to merge 8 commits into from

Conversation

yf-yang
Copy link
Contributor

@yf-yang yf-yang commented Apr 28, 2024

DecoratorElementNode

This is a VERY EARLY draft of DecoratorElementNode (#5930).

I submit this PR because I want to:

  1. Confirm the API design.
  2. Allow the maintenance team and the community to review the codes as early as possible, because I am not an expert of reconciliation, your guidance/your bug report may help me work faster to deal with the reconciliation correctly.

Naive Demo

  • CardNode (Fixed children)
Screen.Recording.2024-04-28.at.18.12.43.mov
  • ReactListNode(Variable children)
Screen.Recording.2024-04-29.at.10.14.13.mov

Tasks to do before this PR is submitted

  • Confirm the API design is good enough (I don't like editor.setNestedRootElement(), could it be better?)
  • Fix the most conspicuous reconciliation bugs
  • Provide a demo including two fixed nested roots (CardNode)
  • Provide a demo of a node containing arbitrary number of child nested roots (ReactListNode)
  • Provide a demo that the nested root could be mounted and unmounted with a button (Add a show/hide button to CardNode)

Tasks I don't plan to do in this PR

Since I don't have so much spare time, I plan to deal with those issues later, maybe the community can offer some help.

  • Add enough test cases
  • Document the node well
  • Deal with IO and copy/paste
  • Deal with corner cases of the selection change (there are many many bugs with it)
  • Guarantee deeply nested decorator element nodes are working correctly
  • Make UI of the demo nodes beautiful

API Documentation Draft

DecoratorElementNode

To combine external framework and Lexical node trees, decorator element node could be an option. It offers a decorate() method to allow external framework to render the DOM, but the external can also have child Lexical nodes handled by Lexical.

export class CardNode extends EXPERIMENTAL_DecoratorElementNode<JSX.Element> {
  static getType(): string {
    return 'card';
  }

  static clone(node: CardNode): CardNode {
    return new CardNode(node.__key);
  }

  constructor(key?: NodeKey) {
    super(key);

    // ElementNode will automatically clone children
    // So we only need to set the children if the node is new
    if (key === undefined) {
      this.append($createNestedRootNode());
      this.append($createNestedRootNode());
    }
  }

  createDOM(): HTMLElement {
    return document.createElement('div');
  }

  updateDOM(): false {
    return false;
  }

  decorate(editor): JSX.Element {
    const onTitleRef = (element) => {
      editor.setNestedRootElement(this.getChildAtIndex(0).getKey(), element);
    }
    const onBodyRef = (element) => {
      editor.setNestedRootElement(this.getChildAtIndex(1).getKey(), element);
    }
   
    return (
      <div>
        <div ref={onTitleRef} />
        <div ref={onBodyRef} />
      </div>
    );
  }
}

Take the CardNode as an example:

  • DecoratorElementNode is a subclass of ElementNode, so it can have an array of LexicalNodes. However, for DecoratorElementNode, the only valid type of its children is NestedRootNode. To use other nodes, they should be NestedRootNode's children.
  • Like DecoratorNode, DecoratorElementNode provides decorate() method, where developers can implement a render function to be handled by external frameworks.
  • To allow Lexical handle external framework's DOMs and mount Lexical's dom to them, LexicalEditor provides an setNestedRootElement method. When a new HTMLElement is set, Lexical will render its children DOMs.

If a DecoratorElementNode has a fixed set of children, developers are responsible to record each child's index and meaning. If it has a variable array of children, then just make sure the onRef's order in decorate function maps to each child correctly.

Copy link

vercel bot commented Apr 28, 2024

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 May 9, 2024 9:10am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 9:10am

@GermanJablo
Copy link
Contributor

What are the problems you currently encounter with DecoratorNodes in your use case?

In my case they are:

  • different undo managers with the editor.
  • fine grained real time collaboration

I believe that these problems are solvable and that they should be addressed even if this PR goes ahead.
Perhaps by talking about these problems and yours we can think of other alternatives.

In the past I tried to do something similar to what you do in this PR (on a local branch), but I ran into a lot of complications. The 1 LexicalNode = 1 DOM node model is something that is deeply rooted from the roots of Lexical.

@yf-yang
Copy link
Contributor Author

yf-yang commented May 7, 2024

@GermanJablo #5930

I agree I have similar troubles, but the main idea behind is decorating element nodes (instead of manipulating simple DOM node).

By the way, when realizing this is just a simplified nested editor, it is not so difficult to debug. The key is treat NestedRootNode and RootNode in the same manner in the core lib.

@GermanJablo
Copy link
Contributor

Still, may I know how you would describe what specific problems this solves?

@yf-yang
Copy link
Contributor Author

yf-yang commented May 14, 2024

Decorate an ElementNode.

Consider things like that: https://github.com/facebook/lexical/blob/main/packages/lexical-playground/src/nodes/LayoutContainerNode.ts

You have to know vanilla JS well to make a complex element node, or you can do very little stuff. Alternatively, NestedEditor is needed and there exists multiple editor states.

@LvChengbin
Copy link

I'm all for adding support for the decorate method to ElementNode.

  1. We often rely on React component libraries like Material-UI to keep our element nodes, such as ParagraphNode and QuoteNode, looking consistent. But without the decorate method, we end up writing more CSS just to match the styles of components in the library.

  2. Trying to do things across nested editors comes with its fair share of headaches—think wonky history management, wonky selection handling, tangled data structures, plugin clashes, you name it.

@CanRau
Copy link
Contributor

CanRau commented Jun 28, 2024

Am also dreaming of something like an ElementNode with decorate capability 🥹
A separate DecoratorElementNode as proposed here would be very welcome to not risk breaking anything existing but allowing for this additional feature where desired 🙌🏼

Agree with most if not all reasons pointed out here, where the "simplest" is being able to just re-use existing ui components as @LvChengbin has mentioned

Also nested editor situation isn't (yet) as intuitive to use imho, especially, as already mentioned, when it comes to selections, history etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants