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

Bug: Preserves format at paragraph level from the previous line #6654

Closed
levensta opened this issue Sep 20, 2024 · 6 comments
Closed

Bug: Preserves format at paragraph level from the previous line #6654

levensta opened this issue Sep 20, 2024 · 6 comments

Comments

@levensta
Copy link
Contributor

Lexical version: 0.17.1

Steps To Reproduce

  1. Write any line with a modified format, e.g. bold
  2. Press Enter

The current behavior

Formatting from the previous line is carried over to the new paragraph

The expected behavior

A new paragraph does not retain the formatting from the previous block

Impact of fix

I found a related PR #5822 with this problem. I see the earlier behavior was exactly what I needed, but then an issue popped up reporting that not saving the format is a problem.

My editor is not supposed to save the format when creating a new paragraph because they are different entities. Similar behavior can be found in Notion as one of the reference WYSIWYG editors. So for my users, saving the format in the next paragraph is an anti-pattern.

Looking at the changes in PR, I saw that the format is set right at the reconciliation step and there is no way to simply prevent this. Since this behavior is controversial, I would suggest making formatting and style preservation an optional parameter that can be overridden. But maybe I'm missing something and there really is an easy way to change this without changing the LexicalReconciler code

@etrepum
Copy link
Collaborator

etrepum commented Sep 20, 2024

I think you're misreading that code a bit. Pretty sure that the format/etc. comes from ParagraphNode.insertNewAfter which picks it up from the selection. If you modify the selection's format in INSERT_PARAGRAPH_COMMAND before the editor's listener runs then I think it should have the desired behavior (or you could use node replacement on ParagraphNode to override this method to ignore the selection's format, but you'd have to audit the rest of the code to make sure that won't have other undesired effects).

@levensta
Copy link
Contributor Author

I tried completely overriding the insertNewAfter method by copying the original code from the ParagraphNode and removing the format setting line from the selection, but that didn't produce any results

insertNewAfter(
  _rangeSelection: RangeSelection,
  restoreSelection: boolean
): ParagraphNode {
  const newElement = $createExtendedParagraphNode();
  // newElement.setTextFormat(rangeSelection.format);
  // const direction = this.getDirection();
  // newElement.setDirection(direction);
  this.insertAfter(newElement, restoreSelection);
  return newElement;
}

I also tried registering the INSERT_PARAGRAPH_COMMAND command listener and resetting the format of each node within the selection and that didn't work either

editor.registerCommand(
  INSERT_PARAGRAPH_COMMAND,
  () => {
    editor.update(() => {
      $getSelection()
        ?.getNodes()
        .filter($isParagraphNode)
        .forEach((paragraph) => paragraph.setTextFormat(0));
    });
    return false;
  },
  COMMAND_PRIORITY_HIGH
)

Correct me if I have misunderstood you

@etrepum
Copy link
Collaborator

etrepum commented Sep 20, 2024

The RangeSelection itself needs the format and style properties set as this is where the INSERT_NEW_PARAGRAPH implementation is getting the style for the new paragraph.

https://github.com/facebook/lexical/blob/main/packages/lexical/src/LexicalSelection.ts#L399-L400
https://github.com/facebook/lexical/blob/main/packages/lexical/src/nodes/LexicalParagraphNode.ts#L193-L194

@levensta
Copy link
Contributor Author

Wow, I thought the format field was read-only, but it looks like a really workable solution!

editor.registerCommand(
  INSERT_PARAGRAPH_COMMAND,
  () => {
    editor.update(() => {
      const selection = $getSelection();
      if ($isRangeSelection(selection)) {
        selection.format = 0;
      }
    });
    return false;
  },
  COMMAND_PRIORITY_HIGH
)

Maybe even the editor.update() call is not needed in this case

@levensta
Copy link
Contributor Author

Well, then the issue can be closed, thanks for the tip!

But now I was wondering, what does the reconcileParagraphFormat() function in LexicalReconciler do? https://github.com/facebook/lexical/blob/v0.17.1/packages/lexical/src/LexicalReconciler.ts#L368-L378

@etrepum
Copy link
Collaborator

etrepum commented Sep 20, 2024

All command listeners are dispatched from inside an editor.update already, so adding another one is not necessary.

reconcileParagraphFormat takes the format from the first child of the paragraph if it's a TextNode and applies it to the paragraph (via some awkward module level state variables so the control flow is hard to follow, but subTreeTextFormat is set in $reconcileChildren). In this case there are no children.

@etrepum etrepum closed this as completed Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants