[lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html #6376

jnnwnk · 2024-07-07T13:23:28Z

Description

Current behaviour

On copy past of text with content type text/html (e.g. from google docs), line breaks inside of paragraphs are ignored. The HTML looks like this (I removed styling attributes for better overview):

<html>
<body>
    <!--StartFragment-->
    <meta charset="utf-8">
    <b>
    	<br />
        <p dir="ltr">Test1</span><span><br /></span><span>Test2</span></p>
        <p dir="ltr">Test3</span></p>
    </b><br class="Apple-interchange-newline">
    <!--EndFragment-->
</body>
</html>

The first line break (before p tag) is recognized correctly.
The second line break (which is embedded between Test1 and Test2 in the first paragraph) gets lost.

Expected / Fixed behaviour

The line break is inserted correctly, no matter where it was placed.

Closes #5592

Test plan

Before

lexical-line-break-before-fix_XsA1rhKO.mp4

After

lexical-line-break-after-fix_gAjhcqd8.mp4

vercel · 2024-07-07T13:23:33Z

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	Jul 7, 2024 2:25pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 7, 2024 2:25pm

facebook-github-bot · 2024-07-07T13:23:33Z

Hi @jnnwnk!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

github-actions · 2024-07-07T13:25:09Z

size-limit report 📦

Path	Size
lexical - cjs	28.47 KB (0%)
lexical - esm	28.28 KB (0%)
@lexical/rich-text - cjs	36.86 KB (0%)
@lexical/rich-text - esm	28.08 KB (0%)
@lexical/plain-text - cjs	35.49 KB (0%)
@lexical/plain-text - esm	25.3 KB (0%)
@lexical/react - cjs	38.82 KB (0%)
@lexical/react - esm	29.27 KB (0%)

facebook-github-bot · 2024-07-07T13:28:41Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

jnnwnk · 2024-07-07T13:32:25Z

Hi guys, this is my first contribution to an open source project of this size.
Please let me know if I need to add anything further or what needs to be done so that the change can be merged.

This reverts commit 8db4d50.

potatowagon · 2024-07-09T03:00:56Z

tested with these cases

<ol>
  <li>
    1<br />3
  </li>
</ol>

 1<br />3

<pre>1<br />3</pre>

they are working

potatowagon · 2024-07-09T03:31:13Z

packages/lexical/src/nodes/LexicalLineBreakNode.ts

  const parentElement = node.parentElement;
-  if (parentElement !== null) {
+  if (parentElement !== null && parentElement.tagName === 'P') {


maybe we could generalise this further to not convert all only child line breaks in block nodes, cause eg.

1 <div> <br /> </div> 3

becomes

1 3

when it should be

1 3

doing on all block nodes would need more testing, ill look into it in a seperate PR.

meanwhile this change is an upgrade for lexical so i would be happy to merge once all the tests pass :)

@potatowagon Are there any tests I need to fix/add?

The checks look good for me, except for After Approval which was skipped for any reason? If there is anything I can do to make this mergable, let me know :)

nothing needed, thanks for the fix!

…content type text/html (facebook#6376) Co-authored-by: Janna Wieneke <[email protected]>

Create line break even if only child

8db4d50

vercel bot deployed to Preview – lexical July 7, 2024 13:24 View deployment

vercel bot deployed to Preview – lexical-playground July 7, 2024 13:24 View deployment

jnnwnk changed the title ~~[lexical] Bug Fix: Create line break on paste of content type text/html~~ [lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html Jul 7, 2024

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 Jul 7, 2024

jnnwnk marked this pull request as ready for review July 7, 2024 13:29

jnnwnk requested review from zurfyx, fantactuka, acywatson, Fetz, ivailop7, Sahejkm and potatowagon as code owners July 7, 2024 13:29

Janna Wieneke added 2 commits July 7, 2024 16:23

Revert "Create line break even if only child"

c1fc003

This reverts commit 8db4d50.

Check if br is only child in paragraph on insert of line breaks

0cc1026

vercel bot deployed to Preview – lexical July 7, 2024 14:24 View deployment

vercel bot deployed to Preview – lexical-playground July 7, 2024 14:25 View deployment

potatowagon approved these changes Jul 9, 2024

View reviewed changes

potatowagon reviewed Jul 9, 2024

View reviewed changes

potatowagon added the extended-tests Run extended e2e tests on a PR label Jul 9, 2024

potatowagon added this pull request to the merge queue Jul 9, 2024

Merged via the queue into facebook:main with commit 3ac2e91 Jul 9, 2024
77 checks passed

2wheeh pushed a commit to 2wheeh/lexical that referenced this pull request Jul 17, 2024

[lexical][lexical-playground] Bug Fix: Create line break on paste of …

4b15060

…content type text/html (facebook#6376) Co-authored-by: Janna Wieneke <[email protected]>

phazingazrael mentioned this pull request Aug 23, 2024

[Snyk] Upgrade @lexical/react from 0.15.0 to 0.17.0 phazingazrael/Terra-Logger#34

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html #6376

[lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html #6376

jnnwnk commented Jul 7, 2024 •

edited

Loading

vercel bot commented Jul 7, 2024 •

edited

Loading

facebook-github-bot commented Jul 7, 2024

github-actions bot commented Jul 7, 2024 •

edited

Loading

facebook-github-bot commented Jul 7, 2024

jnnwnk commented Jul 7, 2024

potatowagon commented Jul 9, 2024 •

edited

Loading

potatowagon Jul 9, 2024

jnnwnk Jul 9, 2024

potatowagon Jul 10, 2024

[lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html #6376

[lexical][lexical-playground] Bug Fix: Create line break on paste of content type text/html #6376

Conversation

jnnwnk commented Jul 7, 2024 • edited Loading

Description

Current behaviour

Expected / Fixed behaviour

Test plan

Before

After

vercel bot commented Jul 7, 2024 • edited Loading

facebook-github-bot commented Jul 7, 2024

Action Required

Process

github-actions bot commented Jul 7, 2024 • edited Loading

size-limit report 📦

facebook-github-bot commented Jul 7, 2024

jnnwnk commented Jul 7, 2024

potatowagon commented Jul 9, 2024 • edited Loading

potatowagon Jul 9, 2024

Choose a reason for hiding this comment

jnnwnk Jul 9, 2024

Choose a reason for hiding this comment

potatowagon Jul 10, 2024

Choose a reason for hiding this comment

jnnwnk commented Jul 7, 2024 •

edited

Loading

vercel bot commented Jul 7, 2024 •

edited

Loading

github-actions bot commented Jul 7, 2024 •

edited

Loading

potatowagon commented Jul 9, 2024 •

edited

Loading