Embeds get re rendered on adding list items in editor about plate HOT 16 CLOSED

The workaround caused more issues than anticipated. issue is that any Transforms function that uses move, and has a wrapped list involved, would still cause the re-render. this includes things like mergeNodes or liftNodes.

At this point, I believe this issue should be solved from slate than trying to do a workaround that requires to re-write most of slate's Transforms functions.

from plate.

My bad, I actually uploaded the wrong gif 😂 it's bed time for me.

I will look into the sandbox tomorrow and see what I can do.

from plate.

Sounds good!

I will look into getting this implemented.

and rather, it's that Transforms.wrapNodes uses Transforms.moveNodes.
Though, Transforms.unwrapNodes doesn't.
But, what's consistent among the three of them is the Range.unref() function that is used at the end, and I am suspecting this is what's causing the re-render. I couldn't find documentation nor do I have the full comprehension on what it does, sadly.

• Transfrorms.wrapNodes uses it internally here and through Transforms.moveNodes here
• Transforms.unwrapNodes uses it internally here
• Transforms.moveNodes uses it internally here

This is all just speculation with no testing, I could be wrong about the unref() function.

from plate.

@zbeyens

This is happening because of the way the initial values were passed in the playground story.

const initialValue: Node[] = [
  ...initialValueForcedLayout,
  ...initialValueBasicMarks,
  ...initialValueHighlight,
  ...initialValueBasicElements,
  ...initialValueList,
  ...initialValueTables,
  ...initialValueLinks,
  ...initialValueMentions,
  ...initialValueImages,
  ...initialValueEmbeds,
  ...initialValueAutoformat,
  ...initialValueSoftBreak,
  ...initialValueExitBreak,
  ...initialValuePasteHtml,
];

the result of that is it will be multiple arrays that contain the nodes to render inside a children property, inside the main children property, rather than the nodes themselves flattened. Currently it's structured as how a list is wrapped.

Adding, modifying or removing a list's item would re-render the entire node, since they all belong to the same 'block'.
Transforms.wrapNodes seems to be the thing that is causing the the re-render to happen

Adding a list anywhere in initialValueEmbeds's data would actually make the embed re-render.

Passing the initial value's children fixes this issue, as the nodes no longer behave as if they are wrapped.

const initialValue: Node[] = [
  ...initialValueForcedLayout,
  ...initialValueBasicMarks,
  ...initialValueHighlight,
  ...initialValueBasicElements,
  ...initialValueList,
  ...initialValueTables,
  ...initialValueLinks,
  ...initialValueMentions,
  ...initialValueImages,
  ...initialValueEmbeds[0].children,
  ...initialValueAutoformat,
  ...initialValueSoftBreak,
  ...initialValueExitBreak,
  ...initialValuePasteHtml,
];

However, with that solution in mind, it will break exit-break plugin's behavior.

• Exiting headers with enter will not work
• Existing code blocks with mod+enter will not work either

Thankfully, in onKeyDownExitBreak.ts, there is a level parameter that when is changed to 0, it will fix the breaking issue.

  rules.forEach(
    ({
      hotkey,
      query: { start, end, ...query } = {},
      level = 0,
      before,
      defaultType = DEFAULT_ELEMENT,
    }) => {
      if (isHotkey(hotkey, event) && isNodeType(entry, query)) {
...

What do you propose to go about this issue?

from plate.

@riexn Thanks for the explanation!

I think we can just create a function to merge the initial values under the same parent.

Please use the SlateDocument type instead of Node[] for your initial value.

from plate.

@zbeyens Anytime!

That's a possible solution. However the issue comes with the type SlateDocument as well.
it currently has the current structure:

{
  children: [
    { type: "p", children: [{ text: "this is the first line" }] },
    { type: "p", children: [{ text: "this is the second line" }] },
    { type: "p", children: [{ text: "this is the third line" }] }
  ]
}

which is emulating a wrapped list, still.

while it should be:

[
  { type: "p", children: [{ text: "this is the first line" }] },
  { type: "p", children: [{ text: "this is the second line" }] },
  { type: "p", children: [{ text: "this is the third line" }] }
]

that way, the initial value will be "flat". This follows the same concept that is found in their examples here.

we could use SlateDocumentFragment as the initial value for slate as a workaround,

but the issue is that almost every functionality breaks since it expects things to be structured the way SlateDocument is.

The things that I've noticed to break were:

• when setting a new empty node as any of the headers, and there is another node after it, then the cursor will move to the next node.
• when hitting enter on any list item for the first time, while the cursor is at the end of the text, the cursor will stay at where it is
• when hitting enter in the middle of a list item, the list will keep adding items on top of the selected ones infinitely.
• when there is an item after the selected list, and the cursor is at the end of the text, hitting enter will behave normally, but if the new item was erased, then hitting enter on the item above it, while on its end, then it will go back to paragraph.
• in code blocks, if I attempt to exit break, then backspace right after it, then it will remove the entire block

All in all, I believe a good amount of the plugins will need to be adjusted to accommodate for the new structure.
Unless I might be missing something?

from plate.

Thanks again for the detailed answer!

The reason why I set a top-level children node is that slate was throwing errors when running getCommonAncestor. It happened when manipulating (nested) lists. The problem is that there is no ancestor at the top-level nodes...

I would say it's a bug that should be fixed in slate so we can go back to the flat structure.

On the other hand it's also weird that we're forced to use a flat structure to fix this issue.

from plate.

Here is the referred bug when using top-level nodes, pressing delete backward:

from plate.

interesting.
Could you send me the URL of the sandbox so I could have a look at it?

here is the result of it working from my side with the flat structure:

we'll ignore the issue with the paragraph's children being split into two for now

from plate.

You need to use 2 different lists to reproduce it 🙂 https://codesandbox.io/s/slate-plugins-reproductions-5keto?file=/index.tsx

Completing my previous answer, It would also mean that we would be "forbidden" to use any nested stateful nodes: in tables, split views, or any other complex nodes 😕

from plate.

Alright, so we both align with the same idea, we will be sticking with the nested nodes due to the reasons that you have mentioned.

With further testing, indeed, the issue is within Transforms.wrapNodes and Transfroms.unwrapNodes causing the entire parent node to re-render.

Here is a proposed solution:
We emulate how wrapNodes and unwrapNodes behave.
I've done a quick and dirty logic just to get the idea across, and it doesn't make the parent node re-render.

toggleList.ts

  // get the block and its path
  const [block, path] = getBlockAboveSelection(editor);
  // set the current node to the type of the list
  Transforms.setNodes(editor, { type: typeList });
  // define the list item path
  const indentedPath = [...path, 0];
  // create the new element in the list, with the children that were in it, to maintain the marks
  const listItem = {
    type: typeLi,
    children: [{ children: [{ type: 'p', children: block.children }] }],
  };
  Transforms.insertNodes(editor, listItem, { at: indentedPath });

And, in this case, the other list plugin handlers will need to be modified to fit this.
What do you think?

from plate.

Nice workaround, I agree with this!

I opened an issue in ianstormtaylor/slate#3748

It appears Transforms.moveNodes has the same problem as it's using wrapNodes ianstormtaylor/slate#3740

from plate.

@riexn

You can also see that the spellchecker (red dots) is resetting when re-rendering.

I just tried to copy the code of moveNodes without editor.apply({ type: 'move_node', path, newPath }); and it does not re-render, so the problem may be inside this operation?

from plate.

@zbeyens

Nice find!

I tried to dig into the operation and find what's causing the issue, but nothing.

At first, I thought it was this one:
slate/src/interfaces/editor.ts
but every operation uses it.

from plate.

Indeed!

from plate.

I'll close this as this is more related to slate, thanks again for the collaboration

from plate.