Fix #5620 Persist format at paragraph level for new line #5822

Sahejkm · 2024-04-04T07:46:58Z

The selection looses the format (bold,italic,etc) on tap of a new line
Update the paragraph node to persist the format which could be used to update the selection format on new paragraph creation
This fixes How to keep formatting between lines? #5620
Before

Screen.Recording.2024-04-08.at.9.32.51.AM.mov

After:

Screen.Recording.2024-04-08.at.9.34.16.AM.mov

This reverts commit d49c122.

vercel · 2024-04-04T07:47:05Z

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

github-actions · 2024-04-04T07:48:50Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	26.95 KB (+0.67% 🔺)	539 ms (+0.67% 🔺)	177 ms (+19.53% 🔺)	716 ms
packages/lexical-rich-text/dist/LexicalRichText.js	39.36 KB (+0.57% 🔺)	788 ms (+0.57% 🔺)	605 ms (+41.43% 🔺)	1.4 s
packages/lexical-plain-text/dist/LexicalPlainText.js	39.33 KB (+0.57% 🔺)	787 ms (+0.57% 🔺)	431 ms (-0.97% 🔽)	1.3 s

zurfyx

Great work, that's a hard one, some feedback:

Serialization

To support import/export we will need to add this new property onto the SerializedNode (defined above the class).

I don't think we need to handle HTML.

Updating after it's changed

The ParagraphNode currently doesn't update at all. The value is only set when creating the node. We will need to update that.

I believe a reconciler function nearby the direction logic should do the track to track the first node format.

Intentionally marked

Initially I thought we may need something like intentionally marked for selection. My hunch is that with the provided feedback I think we're good here. The selection toggle will flip when this happens and none of the other logic should override it.

But for some reason it currently doesn't work. We will need to investigate that.

(videos shows me trying to remove bold at the start of line, which currently is impossible)

Screen.Recording.2024-04-04.at.6.39.23.PM.mov

Similarly, what we should do is that when you change formatting on an empty paragraph node, we update the ParagraphNode styling.

Future work: new line after

It appears that there's a bug when the selection format carries over when deleting content.

Screen.Recording.2024-04-04.at.5.41.28.PM.mov

zurfyx · 2024-04-04T09:34:43Z

packages/lexical/src/LexicalSelection.ts

@@ -220,7 +221,12 @@ function $transferStartingElementPointToTextPoint(
  const target = $isRootNode(element)
    ? $createParagraphNode().append(textNode)
    : textNode;
-  textNode.setFormat(format);
+  const isParapgraphNode = $isParagraphNode(element);


Suggested change

const isParapgraphNode = $isParagraphNode(element);

const isParagraphNode = $isParagraphNode(element);

zurfyx · 2024-04-04T09:48:13Z

packages/lexical/src/nodes/LexicalParagraphNode.ts

    const newElement = $createParagraphNode();
+    if (rangeSelection != null) {
+      const isBackward = rangeSelection.isBackward();


Might be interesting to take this up separately #5825

zurfyx · 2024-04-04T09:48:41Z

packages/lexical/src/nodes/LexicalParagraphNode.ts

    const newElement = $createParagraphNode();
+    if (rangeSelection != null) {


Can it be null? The signature says otherwise

zurfyx · 2024-04-04T10:23:13Z

packages/lexical/src/nodes/LexicalParagraphNode.ts

+        ? this.$getParagaphNodeFromLastSelection(lastNode)
+        : null;
+      if (lastParagraphNode != null) {
+        newElement.setTextFormat(lastParagraphNode.getTextFormat());


I don't think we want this. Let's say that I have a paragraph marked as bold but none of the elements have bold now. I don't want to carry over the bold. In this case, I think we probably still want the selection if relevant

zurfyx · 2024-04-04T10:24:03Z

packages/lexical/src/nodes/LexicalParagraphNode.ts

+        ? rangeSelection.anchor
+        : rangeSelection.focus;
+      const lastNode = endPoint.getNode();
+      const lastParagraphNode = $isElementNode(lastNode)


The paragraph you're looking for is this, because we're inserting new after this paragraph

zurfyx · 2024-04-04T10:32:54Z

packages/lexical/src/LexicalSelection.ts

+  const isParapgraphNode = $isParagraphNode(element);
+  textNode.setFormat(
+    format === 0 && isParapgraphNode && element.getChildren().length === 0
+      ? element.getTextFormat()
+      : format,
+  );


Should we move this logic into the selectionChange function? I was thinking that, just like when you put your caret inside a bold text the selection is bold, the same should be for an empty paragraph.

In this case, this logic should probably be the DFS backward we discussed. We might need to adjust the current heuristic. Or just handle the empty paragraph node case for now.

zurfyx

LGTM, thanks! Can you add a couple more tests please?

Toggle format with CMD + B at the start of a paragraph of different format, toggled format persists
Removing all nodes of a paragraph with format and typing (I know that behavior doesn't resemble GDocs/Word but let's at least make sure it doesn't regress).

Can we follow up on the table formatting (in a separate PR)? Particularly persisting the format on creation and adding/removing rows and columns.

packages/lexical-playground/__tests__/e2e/Selection.spec.mjs

nickschinestzki · 2024-04-09T13:44:20Z

Does this fix also applies for styling? Like text color, font family, font size, etc?

Sahejkm added 9 commits April 3, 2024 13:47

from 5707

d49c122

.

6f3780a

Revert "from 5707"

59ba7d6

This reverts commit d49c122.

.

2fb1a2c

.

c3b13c9

.

9ae2516

fix tests

c277a29

.

ef27438

.

7b735de

Sahejkm requested review from zurfyx, fantactuka, acywatson, Fetz and ivailop7 as code owners April 4, 2024 07:46

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 4, 2024

.

44cb1ac

vercel bot deployed to Preview – lexical April 4, 2024 08:01 View deployment

vercel bot deployed to Preview – lexical-playground April 4, 2024 08:03 View deployment

fix test

b49fa29

vercel bot deployed to Preview – lexical April 4, 2024 08:10 View deployment

vercel bot deployed to Preview – lexical-playground April 4, 2024 08:12 View deployment

Sahejkm changed the title ~~#5620 Persist format at paragraph level for new line~~ Fix #5620 Persist format at paragraph level for new line Apr 4, 2024

zurfyx reviewed Apr 4, 2024

View reviewed changes

.

4732fd6

vercel bot deployed to Preview – lexical April 4, 2024 16:06 View deployment

vercel bot deployed to Preview – lexical-playground April 4, 2024 16:07 View deployment

.

1313104

vercel bot deployed to Preview – lexical April 4, 2024 16:09 View deployment

vercel bot deployed to Preview – lexical-playground April 5, 2024 10:30 View deployment

Refactor - rename variable name

2c50a3d

vercel bot deployed to Preview – lexical April 8, 2024 05:35 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 05:38 View deployment

Refactor - remove double setTextformat

4d896d8

vercel bot deployed to Preview – lexical April 8, 2024 05:39 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 05:41 View deployment

Add e2e tests

07048d3

vercel bot deployed to Preview – lexical April 8, 2024 05:54 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 05:56 View deployment

zurfyx reviewed Apr 8, 2024

View reviewed changes

packages/lexical-playground/__tests__/e2e/Selection.spec.mjs Outdated Show resolved Hide resolved

Skip Test for Plain Text mode

2388548

vercel bot deployed to Preview – lexical April 8, 2024 08:51 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 08:52 View deployment

add test toggle format at start of the paragraph persists the new format

865c65a

vercel bot deployed to Preview – lexical April 8, 2024 09:46 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 09:47 View deployment

formatting is persisted after deleting all nodes from the paragraph node

a04e03b

zurfyx approved these changes Apr 8, 2024

View reviewed changes

vercel bot deployed to Preview – lexical April 8, 2024 10:09 View deployment

vercel bot deployed to Preview – lexical-playground April 8, 2024 10:11 View deployment

Sahejkm merged commit e2f2e3d into main Apr 8, 2024
45 checks passed

acywatson mentioned this pull request Apr 16, 2024

v0.14.4 #5900

Merged

nickschinestzki mentioned this pull request Apr 24, 2024

How to keep styles between lines? #5958

Closed

dmitrymarokhonov mentioned this pull request May 19, 2024

[Snyk] Upgrade @lexical/react from 0.10.0 to 0.14.5 Opetushallitus/kouta-ui#574

Open

ivailop7 mentioned this pull request Jul 21, 2024

[lexical] [lexical-selection] Preserve paragraph styles between lines #6437

Merged

levensta mentioned this pull request Sep 20, 2024

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

Closed

ivailop7 mentioned this pull request Nov 10, 2024

Retain format on programatic updates #5707

Closed

zurfyx mentioned this pull request Dec 3, 2024

ElementNode format #5708

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #5620 Persist format at paragraph level for new line #5822

Fix #5620 Persist format at paragraph level for new line #5822

Sahejkm commented Apr 4, 2024 •

edited

Loading

vercel bot commented Apr 4, 2024 •

edited

Loading

github-actions bot commented Apr 4, 2024 •

edited

Loading

zurfyx left a comment

zurfyx Apr 4, 2024

zurfyx Apr 4, 2024

zurfyx Apr 4, 2024

zurfyx Apr 4, 2024

zurfyx Apr 4, 2024

zurfyx Apr 4, 2024

zurfyx left a comment

nickschinestzki commented Apr 9, 2024

	const isParapgraphNode = $isParagraphNode(element);
	const isParagraphNode = $isParagraphNode(element);

		const newElement = $createParagraphNode();
		if (rangeSelection != null) {

Fix #5620 Persist format at paragraph level for new line #5822

Fix #5620 Persist format at paragraph level for new line #5822

Conversation

Sahejkm commented Apr 4, 2024 • edited Loading

vercel bot commented Apr 4, 2024 • edited Loading

github-actions bot commented Apr 4, 2024 • edited Loading

size-limit report 📦

zurfyx left a comment

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx Apr 4, 2024

Choose a reason for hiding this comment

zurfyx left a comment

Choose a reason for hiding this comment

nickschinestzki commented Apr 9, 2024

Sahejkm commented Apr 4, 2024 •

edited

Loading

vercel bot commented Apr 4, 2024 •

edited

Loading

github-actions bot commented Apr 4, 2024 •

edited

Loading