[0.12.0] Rework generateNodesFromDOM API #4253

acywatson · 2023-04-01T21:50:38Z

I will try to get around to writing up a more extensive explanation later. The main purposes of this are twofold:

Improve the out-of-the-box experience for HTML serialization and deserialization in the storage format use-case (as opposed to the transfer/copy-paste use case).

One of the big issues here is that we can't support interpretation of html attributes like background-color or text-align in the core without creating a poor user experience during HTML copy+paste. The idea here is to transfer those semantics via custom attributes on HTML, so we can support HTML-as-a-storage-format (deserializing into an editor with the same configuration) at a higher fidelity while preserving control during copy/paste.

Amend the public API to be less cluttered and provide a path for additional options

Ultimately, I'd like to experiment with bringing more of this logic out of the core library and into the editor configuration, possible with some defaults in the rich-text or utils modules. import/exportDOM makes sense on the nodes, but it is honestly pretty clunky to have to do that for just a simple use case like supporting a text style.

fixes #4243

Example:

const editor = createEditor();
const htmlString =  '<body><p lexical-data-format-align="center" ><span lexical-data-style="background: red">Hello</span></body>'  //$generateHtmlFromNodes(editor, selection | null);
const parser = new DOMParser();
const dom = parser.parseFromString(htmlString, textHtmlMimeType);
const nodes = $generateNodesFromDOM(editor, dom, { elementFormats: true, textStyles: true });
// ParagraphNode (format: CENTER)
//  - TextNode (style: background: red)
const nodesWithoutStyle = $generateNodesFromDOM(editor, dom, { elementFormats: false, textStyles: false });
// ParagraphNode (align: undefined)
//  - TextNode (style: undefined)

vercel · 2023-04-01T21:50:46Z

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 2, 2023 11:36pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 2, 2023 11:36pm

github-actions · 2023-04-01T21:53:04Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	27.02 KB (+0.49% 🔺)	541 ms (+0.49% 🔺)	222 ms (-42.3% 🔽)	762 ms
packages/lexical-rich-text/dist/LexicalRichText.js	37.96 KB (+0.63% 🔺)	760 ms (+0.63% 🔺)	230 ms (-19.92% 🔽)	989 ms
packages/lexical-plain-text/dist/LexicalPlainText.js	37.94 KB (+0.63% 🔺)	759 ms (+0.63% 🔺)	319 ms (-43.35% 🔽)	1.1 s

amyworrall · 2023-04-03T15:59:55Z

I like the concept here.

QQ, why is convertDOMElementLexicalData() only for element nodes?

fantactuka · 2023-04-03T18:39:23Z

Could you add an example of new API usage?

zurfyx

LGTM

Amend the public API to be less cluttered and provide a path for additional options

I'm unsure how this PR addresses this but I like the long-term vision of moving all the conversion functionality outside of the Core

zurfyx · 2023-04-03T21:31:15Z

packages/lexical/src/LexicalNode.ts

@@ -121,10 +121,27 @@ export type DOMConversion<T extends HTMLElement = HTMLElement> = {
  priority: 0 | 1 | 2 | 3 | 4;
 };

+export type DOMConversionOptions = {
+  textStyles?: true;
+  elementFormats?: true;


Does this mean it should be treated like an Element or something?

It controls whether or not you want Lexical to try interpret the element formats and map them to core element formats (indent, align, etc)

zurfyx · 2023-04-03T21:35:10Z

packages/lexical/src/LexicalUtils.ts

+  const lexicalDataFormatAlign = element.getAttribute(
+    'lexical-data-format-align',
+  );


One of the big issues here is that we can't support interpretation of html attributes like background-color or text-align in the core without creating a poor user experience during HTML copy+paste.

Why is it correct for Lexical to Lexical but not for external users? Did we do some research on how other editors address similar problems? Have we considered an in-between between EditorState and HTML to transfer data between lexical Editors?

Because the semantics of Lexical data storage and transfer shouldn't be coupled to HTML semantics (because they don't map 1:1). This is the root of this issue, imo.

Copy and Paste is one serialization use case, but it's not the only one. Another is simple storage/retrieval. If we rely on HTML semantics to transfer Lexical via imperfect mappings, we end up in a situation where external content can cause issues by inadvertently colliding with those semantics.

For example, an inline background-color style can be added on a span tag on the clipboard in google docs. If Lexical core interprets that as a TextNode style, then every editor will interpret it that way, even if the editor doesn't support those styles. This is why right now, for instance, you can change the alignment of some text in GDocs and then paste it into one of our internal editors and it will be stuck in that alignment.

Because the semantics of Lexical data storage and transfer shouldn't be coupled to HTML semantics

Makes sense, it sounds like we're reusing HTML to build this additional in-between fidelity layer.

you can change the alignment of some text in GDocs and then paste it into one of our internal editors and it will be stuck in that alignment.

What do you mean by stuck? You can't left-align it afterward?

What do you mean by stuck? You can't left-align it afterward?

Yea, if the editor doesn't support text-alignment (i.e. if it doesn't expose any UI to set the alignment back) then it's just stuck in the center or right alignment. Same with text-color or background-color if those were handled in the core the same way as alignment (they aren't right now).

Yea, if the editor doesn't support text-alignment (i.e. if it doesn't expose any UI to set the alignment back) then it's just stuck in the center or right alignment. Same with text-color or background-color if those were handled in the core the same way as alignment (they aren't right now).

Have we considered a feature system? #4254 (comment)

acywatson · 2023-04-03T22:13:12Z

LGTM

Amend the public API to be less cluttered and provide a path for additional options

I'm unsure how this PR addresses this but I like the long-term vision of moving all the conversion functionality outside of the Core

I was thinking because now generateNodesFromDOM will take an options object instead of just continuously adding more arguments.

acywatson · 2023-04-04T16:59:41Z

I like the concept here.

QQ, why is convertDOMElementLexicalData() only for element nodes?

Good question - it's because all ElementNode subclasses don't need this logic, so I couldn't very easily put it in the base class. Instead, I put it in a util to avoid repetition. Will look closer at this when I have time to take another pass.

acywatson · 2023-04-04T16:59:54Z

Could you add an example of new API usage?

yes, I will

thegreatercurve · 2023-06-19T11:54:37Z

@acywatson Mind resolving the merge conflicts?

rework generateNodesFromDOM API

d18f457

acywatson requested review from zurfyx, fantactuka, tylerjbainbridge and thegreatercurve as code owners April 1, 2023 21:50

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 1, 2023

acywatson changed the title ~~Rework generateNodesFromDOM API~~ [0.10] Rework generateNodesFromDOM API Apr 1, 2023

small fixes

0560f3f

acywatson changed the title ~~[0.10] Rework generateNodesFromDOM API~~ [RFC][0.10] Rework generateNodesFromDOM API Apr 2, 2023

vercel bot deployed to Preview – lexical April 2, 2023 23:35 View deployment

vercel bot deployed to Preview – lexical-playground April 2, 2023 23:37 View deployment

This was referenced Apr 3, 2023

heading generateHtmlFromNodes was not adding text-align #4250

Closed

Allow passing in a custom Set of tags to ignore in $generateNodesFromDOM #4178

Closed

zurfyx reviewed Apr 3, 2023

View reviewed changes

zurfyx approved these changes Apr 4, 2023

View reviewed changes

fantactuka approved these changes Apr 19, 2023

View reviewed changes

ivailop7 approved these changes Apr 19, 2023

View reviewed changes

thegreatercurve changed the title ~~[RFC][0.10] Rework generateNodesFromDOM API~~ [RFC][0.12.0] Rework generateNodesFromDOM API Jun 19, 2023

thegreatercurve changed the title ~~[RFC][0.12.0] Rework generateNodesFromDOM API~~ [0.12.0] Rework generateNodesFromDOM API Jun 19, 2023

potatowagon added the stale-pr PRs that are closed due to staleness. Please reopen the PR if there are new updates label Nov 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[0.12.0] Rework generateNodesFromDOM API #4253

[0.12.0] Rework generateNodesFromDOM API #4253

acywatson commented Apr 1, 2023 •

edited

Loading

vercel bot commented Apr 1, 2023 •

edited

Loading

github-actions bot commented Apr 1, 2023 •

edited

Loading

amyworrall commented Apr 3, 2023

fantactuka commented Apr 3, 2023

zurfyx left a comment

zurfyx Apr 3, 2023

acywatson Apr 3, 2023

zurfyx Apr 3, 2023

acywatson Apr 3, 2023

zurfyx Apr 3, 2023

acywatson Apr 3, 2023

zurfyx Apr 4, 2023

acywatson commented Apr 3, 2023

acywatson commented Apr 4, 2023

acywatson commented Apr 4, 2023

thegreatercurve commented Jun 19, 2023

[0.12.0] Rework generateNodesFromDOM API #4253

Are you sure you want to change the base?

[0.12.0] Rework generateNodesFromDOM API #4253

Conversation

acywatson commented Apr 1, 2023 • edited Loading

vercel bot commented Apr 1, 2023 • edited Loading

github-actions bot commented Apr 1, 2023 • edited Loading

size-limit report 📦

amyworrall commented Apr 3, 2023

fantactuka commented Apr 3, 2023

zurfyx left a comment

Choose a reason for hiding this comment

zurfyx Apr 3, 2023

Choose a reason for hiding this comment

acywatson Apr 3, 2023

Choose a reason for hiding this comment

zurfyx Apr 3, 2023

Choose a reason for hiding this comment

acywatson Apr 3, 2023

Choose a reason for hiding this comment

zurfyx Apr 3, 2023

Choose a reason for hiding this comment

acywatson Apr 3, 2023

Choose a reason for hiding this comment

zurfyx Apr 4, 2023

Choose a reason for hiding this comment

acywatson commented Apr 3, 2023

acywatson commented Apr 4, 2023

acywatson commented Apr 4, 2023

thegreatercurve commented Jun 19, 2023

acywatson commented Apr 1, 2023 •

edited

Loading

vercel bot commented Apr 1, 2023 •

edited

Loading

github-actions bot commented Apr 1, 2023 •

edited

Loading