-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[lexical-react] Feature: Implement DecoratorTextNode and TextWithFormattedContents component #8114
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| * @flow strict | ||
| */ | ||
|
|
||
| /** | ||
| * LexicalDecoratorTextNode | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was generated by the update-flowconfig command, but I didn't understand whether I needed to add the types manually. The build-types command returned an error. Perhaps it would be worth adding a brief explanation to CONTRIBUTING.md of how to correctly add new exportable things to packages 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pnpm run update-flowconfig only creates the files, it doesn't try and write the flow types for you.
pnpm run build-types is unrelated to flow, it only runs typescript. it does look like there's a problem with it worth fixing or filing an issue for, it tries to type check the build products in a way that doesn't make sense, so pnpm build-types will fail if you have run pnpm run build without pnpm run clean or equivalent.
etrepum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't taken a very close look but this does seem like a lot of code for this feature, without any tests, and it appears to be failing existing tests. I suspect that it could be simpler.
Description
In this PR, I add
DecoratorTextNode— a new node extended fromDecoratorNode. The implementation of the node is simple and is actually the sameDecoratorNode, but with formatting methods exactly the same as those ofTextNode. I also added the HOC componentTextWithFormattedContents, which handles formatting.The implementation is very similar to the existing pairs in this package:
DecoratorBlockNodeandBlockWithAlignableContentsThis feature can be useful for decorator nodes that need to be rendered in the middle of text and behave like text when the user wants to convert a part in a some format. Support for specific formats and how they affect the visual component of the decorator is left up to the user. As an example, I made how this might work for DateTimeNode in lexical-playground
Test plan
Before
Screen.Recording.2026-02-04.at.05.42.50.mov
After
Screen.Recording.2026-02-04.at.05.40.45.mov