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.

Sign up for GitHub

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

Jump to bottom

Allow an empty paragraph child on converting from markdown #5517

Merged
merged 8 commits into from
Jan 26, 2024
Merged

Allow an empty paragraph child on converting from markdown #5517

merged 8 commits into from
Jan 26, 2024

Conversation

2wheeh
Copy link
Contributor

@2wheeh 2wheeh commented Jan 19, 2024 •
edited
Loading

fixes #5516

after:

after.mov

@vercel Vercel
Copy link

vercel bot commented Jan 19, 2024 •
edited
Loading

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

@facebook-github-bot 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 Jan 19, 2024
@2wheeh 2wheeh changed the title Allow to remove empty paragraph only for the nodes that can be empty Allow removing empty paragraph only for the nodes that can be empty Jan 19, 2024
@2wheeh 2wheeh marked this pull request as draft January 19, 2024 08:00
@2wheeh 2wheeh changed the title Allow removing empty paragraph only for the nodes that can be empty Allow the empty paragraph child on converting from markdown Jan 19, 2024
@2wheeh 2wheeh marked this pull request as ready for review January 19, 2024 08:29
@2wheeh 2wheeh changed the title Allow the empty paragraph child on converting from markdown Allow an empty paragraph child on converting from markdown Jan 19, 2024
zurfyx
zurfyx previously approved these changes Jan 19, 2024
View reviewed changes
Copy link
Member

@zurfyx zurfyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the proposal is reasonable, should we just compare whether clearing the last one leads to an empty state? Also, root canBeEmpty is just a product of inheriting ElementNode, the only case where this is valid is when starting the editor and the expectation is that soon after it will be populated with the bootstrapping content (the case where this is the most obvious is in collab)

packages/lexical-markdown/src/MarkdownImport.ts Outdated Show resolved Hide resolved
@2wheeh
Copy link
Contributor Author

2wheeh commented Jan 22, 2024

@zurfyx Thank you for kind explanation. I misunderstood some methods. I applied your suggestion: 036b878
Btw, while debugging this, I found some other cases that table node looses table row and cell node. I will open a new issue about them.

@2wheeh 2wheeh requested a review from zurfyx January 25, 2024 16:16
zurfyx
zurfyx approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@zurfyx zurfyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for iterating on this!

@zurfyx zurfyx merged commit 5ef89dd into facebook:main Jan 26, 2024
42 of 45 checks passed
@2wheeh 2wheeh deleted the insert-paragraph-on-empty-table-cell branch January 26, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zurfyx zurfyx zurfyx approved these changes

@fantactuka fantactuka Awaiting requested review from fantactuka fantactuka is a code owner

@acywatson acywatson Awaiting requested review from acywatson acywatson is a code owner

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: an empty table cell breaks the editor on converting from markdown string
3 participants
@2wheeh @zurfyx @facebook-github-bot