-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[lexical][lexical-list][lexical-rich-text]: Fix: Preserve indentation when serializing to and from HTML #6693
Merged
+215
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GermanJablo
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm and
potatowagon
as code owners
October 3, 2024 00:04
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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
Oct 3, 2024
size-limit report 📦
|
This was referenced Oct 3, 2024
I think the failed tests are flaky |
etrepum
approved these changes
Oct 12, 2024
AlessioGr
pushed a commit
to payloadcms/payload
that referenced
this pull request
Nov 12, 2024
I'm needing facebook/lexical#6693 I'm going to keep the dependency bump and feature updates in separate PRs unless they're breaking changes.* **BREAKING:** This upgrades our lexical dependencies from 0.18.0 to 0.20.0. If you have lexical dependencies installed in your project, you will have to upgrade those. Additionally, the lexical team may introduce breaking changes in this upgrade. If you use lexical APIs directly, please consult their changelog for more information: https://github.com/facebook/lexical/releases
AlessioGr
pushed a commit
to payloadcms/payload
that referenced
this pull request
Nov 21, 2024
… Lexical <-> HTML (#9165) Fixes #5146 This had been solved in facebook/lexical#6693 but we are using another serialization. I open #9166 to discuss/track how we can improve this in the future
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
extended-tests
Run extended e2e tests on a PR
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR ensures that the indent property is preserved when importing and exporting HTML.
Closes #6082
Additionally, the following bugs are fixed:
ParagraphNode.exportDOM
the correct default value is used. It was set to an old value.ParagraphNode.exportDOM
the indent is exported using thepadding-inline-start
property as used in the editor, instead oftext-indent
. Apparently, the person who set that did so becausepadding-inline-start
has poor support in email clients. However,text-indent
is a first-line indent, different from the lexical indent which encompasses the entire block. If someone needs to export for use in email, they should probably create their own converter.Considerations
This PR does not include serialization of custom indent values defined in the editorConfig.theme. The problem is that the CSS variable
--lexical-indent-base-value
is used, but when importing html the variable is not accessible. It can be used in any context, even headless. This has already caused problems for other users:#4583
#5062
As I see it, the solution to this would be that the indent spacing is not configured with CSS under the editorConfig theme property, but using javaScript. That would be a breaking change.
Test plan
Tests are included for Paragraphs, Quotes and Headings.