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.

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

Feature: Add ability to disable text formatting and text-align in a performant and easy way #7298

Open
GermanJablo opened this issue Mar 5, 2025 · 4 comments
Labels
enhancement Improvement over existing feature

Comments

@GermanJablo
Copy link
Contributor

For context, a decision was previously made to disable pasting of styles such as color and background-color from the clipboard.

The reason for this is that the editor might end up with unwanted styles.

However this does not take into account:

  • TextNode formats, such as bold, italics, underline, etc.
  • ElementNode text-align (format)

In Payload, if those formats are not explicitly enabled in an editor, we use a transformListener on TextNode to revert the nodes with those formats.

This could be made more performant if instead of iterating over dirty nodes, Lexical introduced a property to disable those styles. The implementation I can think of would be to have that property intervene in the setFormat method of TextNode.

Note that I did not consider the option of overriding the command to add formats, as that does not prevent other forms of mutation such as paste from clipboard, browser extensions, other editor plugins, etc.

The problem has become worse now that I have tried to implement the same for text align.

NodeTransforms cannot be registered on ElementNode, and registering the same change on all nodes that might have text-align feels wrong.

I thought about using a transformation on RootNode and iterating over dirtyElements, but that object is created when the transformations finish and before the update listeners start.

Doing this in the updateListener would not be ideal either, as it would trigger an incorrect and unnecessary update.

Again, I think we should include an option in the editorConfig to disable text-align, as well as text formats.

The API I imagine is something like this:

const editorConfig = {
  disabledFormats: ["text-align", "bold", "italic"]
  // ...
}

If the idea makes sense to you, I can do the PR.

@GermanJablo GermanJablo added the enhancement Improvement over existing feature label Mar 5, 2025
@etrepum
Copy link
Collaborator

etrepum commented Mar 6, 2025

I think part of the solution to this problem (and some other architectural issues) is to stop working with styles as strings and instead use an object-based approach like React does to get rid of that awful CSS_TO_STYLES cache and make it easier to patch styles which is particularly important when you have plug-ins manipulating styles. The big decision there is probably to figure out how those styles are represented, either with their camel cased names like React does or with their hyphen cased names (e.g. 'textAlign' vs 'text-align'), or with an object that normalizes it either way (like a CSSStyleDeclaration, but probably ponyfilled for SSR purposes). For backwards compatibility we could have computed __style / __textStyle properties that are still strings, leave the existing getters/setters alone (as deprecated probably), and just use $functions for the new getters/setters since they would apply to all nodes anyway.

Applying classes should likely be done in a similar way, working with some sort of object (e.g. a Record<string, boolean> or Set<string>).

Either way if we use NodeState to do this style/class storage we can have the serialized representation stay a string while keeping the "live" version in its performant parsed representation.

Additionally we should have a lower-level interface to plug into the $applyAllTransforms loop where you can feasibly do something based on any dirty node or element (kind of like an update listener, but before reconciliation). Basically the kind of interface that you could use to implement $applyAllTransforms if it weren't already hard-wired into $beginUpdate. This is the sort of thing you'd want to be able to register a transform that applies to particular keys of dirty NodeState.

@GermanJablo
Copy link
Contributor Author

I like the idea, although that's a bigger task than I can volunteer for right now.
If that big change can be postponed and we can now fix this without changing the internal properties, I think I can make the time.
These seem like two problems that can be addressed separately, but I'll defer to whatever decision you guys make.

@etrepum
Copy link
Collaborator

etrepum commented Mar 6, 2025

The latter "document transform" hook would be sufficient for you to detect changes in __style and __textStyle for any dirty text/elements and do any patching you need.

@etrepum
Copy link
Collaborator

etrepum commented Mar 7, 2025

#7294 (WIP) is an example that starts to take most of these concepts and plugs it into an example, still needs some work of course and the hooks would have to be inserted in the core for importDOM/exportDOM to work better with this sort of thing but if we used such an approach at the lower level instead of style strings it would be a lot cleaner.

The rightmost button on the toolbar adds a shadow

See: https://github.com/etrepum/lexical/blob/nodestate-example/examples/node-state-style/src/styleState.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement over existing feature
Projects
None yet
Development

No branches or pull requests

2 participants