[lexical-extension] Feature: Implement DecoratorTextExtension applying format to DecoratorTextNode#8114
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.
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.
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.
There was a problem hiding this comment.
it does look like there's a problem with it worth fixing or filing an issue for
I attempted to run the command on node v24.11.1. After switching to v22.22.0, pnpm run build-types executed successfully
etrepum
left a comment
There was a problem hiding this comment.
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.
I simplified the implementation a little and fixed fundamental errors that caused the tests to fail |
| await toggleBold(page); | ||
| await assertHTML( | ||
| page, | ||
| // After formatting the text, the selection will be reset from the decorator node, | ||
| // so it will retain its previous format when toggleBold is triggered again |
There was a problem hiding this comment.
I guess this is related to the TextPointType update in selection.formatText https://github.com/facebook/lexical/blob/main/packages/lexical/src/LexicalSelection.ts#L1319-L1325
Screen.Recording.2026-02-07.at.02.03.48.mov
cdee519 to
45aacdf
Compare
packages/lexical-playground/src/nodes/DateTimeNode/DateTimeNode.tsx
Outdated
Show resolved
Hide resolved
packages/lexical-playground/src/nodes/DateTimeNode/DateTimeNode.tsx
Outdated
Show resolved
Hide resolved
packages/lexical-playground/src/nodes/DateTimeNode/DateTimeNode.tsx
Outdated
Show resolved
Hide resolved
etrepum
left a comment
There was a problem hiding this comment.
Looks good, just some small cleanups to do!
| if ($isNodeSelection(selection)) { | ||
| selection.getNodes().forEach((node) => { | ||
| if ($isDecoratorTextNode(node)) { | ||
| node.toggleFormat(formatType); | ||
| } | ||
| }); | ||
| } else if ($isRangeSelection(selection)) { | ||
| const nodes = selection.getNodes(); | ||
|
|
||
| for (const node of nodes) { | ||
| if ($isDecoratorTextNode(node)) { | ||
| node.toggleFormat(formatType); | ||
| } else { | ||
| const decoratorText = $findMatchingParent( | ||
| node, | ||
| $isDecoratorTextNode, | ||
| ); | ||
| if (decoratorText !== null) { | ||
| decoratorText.toggleFormat(formatType); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
$isDecoratorTextNode will never return true for an ElementNode so there's no need to check parents
| if ($isNodeSelection(selection)) { | |
| selection.getNodes().forEach((node) => { | |
| if ($isDecoratorTextNode(node)) { | |
| node.toggleFormat(formatType); | |
| } | |
| }); | |
| } else if ($isRangeSelection(selection)) { | |
| const nodes = selection.getNodes(); | |
| for (const node of nodes) { | |
| if ($isDecoratorTextNode(node)) { | |
| node.toggleFormat(formatType); | |
| } else { | |
| const decoratorText = $findMatchingParent( | |
| node, | |
| $isDecoratorTextNode, | |
| ); | |
| if (decoratorText !== null) { | |
| decoratorText.toggleFormat(formatType); | |
| } | |
| } | |
| } | |
| if ($isNodeSelection(selection) || $isRangeSelection(selection)) { | |
| for (const node of selection.getNodes()) { | |
| if ($isDecoratorTextNode(node)) { | |
| node.toggleFormat(formatType); | |
| } | |
| } | |
| } |
| .filter(([flag]) => format & Number(flag)) | ||
| .map(([, className]) => className), |
There was a problem hiding this comment.
These can array returning methods be fused into a flatMap, although it would probably make more sense to have formatClassMap just be an array of pairs in the first place rather than converting it to one on demand every time.
| .filter(([flag]) => format & Number(flag)) | |
| .map(([, className]) => className), | |
| .flatMap(([flag, className]) => format & Number(flag) ? [className] : []) |
or
// This would be top-level in the module
const FORMAT_CLASSES = [
[IS_BOLD, 'bold'],
[IS_HIGHLIGHT, 'highlight'],
[IS_ITALIC, 'italic'],
[IS_STRIKETHROUGH, 'strikethrough'],
[IS_UNDERLINE, 'underline'],
] as const;
// …
const classNames = ['dateTimePill'];
for (const [flag, className] of FORMAT_CLASSES) {
if (format & flag) {
classNames.push(className);
}
}There was a problem hiding this comment.
although it would probably make more sense to have formatClassMap just be an array of pairs in the first place rather than converting it to one on demand every time
Agreed, done
7bbe225 to
f146bf9
Compare
…g format to DecoratorTextNode (facebook#8114)
Description
Added a new extension
DecoratorTextExtensionwhich provides a newDecoratorTextNodeclass and the ability to apply text formatting to it.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.This 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