[lexical-react][lexical-dev-tools-core] Feature: Allow TreeView custom print output #6180

irahopkinson · 2024-05-28T00:21:45Z

Description

Previously, the TreeView output couldn't be customized.
This PR allows you to customize existing output and support custom node output.
- added optional customPrintNode function property to TreeView.
  - If the function is not defined then the built-in output is used in the TreeView.
  - If the function returns nothing then the built-in output is used.
  - If the function returns text then that text is used in the output for that node.
- moved output of mark IDs to printNode so it can be customized (it didn't seem to belong where it was anyway).
- made appropriate changes to flow.
- Note I didn't know how to update the API documentation as requested in https://github.com/facebook/lexical/blob/main/CONTRIBUTING.md but it looks like it is auto-generated anyway.

Test plan

Before

Previously in the Playground the TreeView output couldn't be customized:

After

To test, I changed packages\lexical-playground\src\plugins\TreeViewPlugin\index.tsx to:

/**
 * 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.
 *
 */

import {$isMarkNode} from '@lexical/mark';
import {useLexicalComposerContext} from '@lexical/react/LexicalComposerContext';
import {TreeView} from '@lexical/react/LexicalTreeView';
import {LexicalNode} from 'lexical';
import * as React from 'react';

function $customPrintNode(node: LexicalNode) {
  if ($isMarkNode(node)) {
    return `uids: [ ${node.getIDs().join(', ')} ]`;
  }
  return '';
}

export default function TreeViewPlugin(): JSX.Element {
  const [editor] = useLexicalComposerContext();
  return (
    <TreeView
      viewClassName="tree-view-output"
      treeTypeButtonClassName="debug-treetype-button"
      timeTravelPanelClassName="debug-timetravel-panel"
      timeTravelButtonClassName="debug-timetravel-button"
      timeTravelPanelSliderClassName="debug-timetravel-panel-slider"
      timeTravelPanelButtonClassName="debug-timetravel-panel-button"
      customPrintNode={$customPrintNode}
      editor={editor}
    />
  );
}

And then the Playground displays:

Note in the TreeView the mark IDs array label is now "uids" (instead of "id").

vercel · 2024-05-28T00:21:49Z

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	May 29, 2024 0:54am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 29, 2024 0:54am

github-actions · 2024-05-28T00:23:56Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
packages/lexical/dist/Lexical.js	23.95 KB (0%)	480 ms (0%)	210 ms (+159.97% 🔺)	689 ms
packages/lexical-rich-text/dist/LexicalRichText.js	34.63 KB (0%)	693 ms (0%)	703 ms (+172.31% 🔺)	1.4 s
packages/lexical-plain-text/dist/LexicalPlainText.js	34.61 KB (0%)	693 ms (0%)	578 ms (+80% 🔺)	1.3 s

potatowagon · 2024-05-28T07:18:56Z

packages/lexical-devtools-core/src/generateContent.ts

@@ -245,9 +249,17 @@ function normalize(text: string, obfuscateText: boolean = false) {
  return textToPrint;
 }

-// TODO Pass via props to allow customizability


this comment may still be valid, perhaps we shouldnt delete it

This PR allows full customizability for the TreeView print output for any existing node or any custom node and it it does it by passing a function in through the props. Perhaps I'm missing something but what part of that comment still needs implementing?

potatowagon · 2024-05-28T07:26:25Z

packages/lexical-devtools-core/src/generateContent.ts

+  const customPrint = customPrintNode
+    ? customPrintNode(node, obfuscateText)
+    : undefined;
+  if (customPrint) {
+    return customPrint;


Suggested change

const customPrint = customPrintNode

? customPrintNode(node, obfuscateText)

: undefined;

if (customPrint) {

return customPrint;

if (customPrintNode !== undefined) {

return customPrintNode(node, obfuscateText);

}

We only want the custom print output if customPrintNode(...) produces some text, i.e. if it returns an empty string because it doesn't handle a node then the existing code below would run and we would get the built-in print output. Your suggested change would not do that but return no output. That's not what we want here.

if (customPrint) isnt clear because customPrint isnt a boolean. preferably be more explicit about the comparison, such as

if (customPrint !== undefined && customPrint.length > 0) { return customPrint; }

That makes sense, done.

potatowagon · 2024-05-28T07:32:25Z

what are some example usecases of custom print node?

potatowagon · 2024-05-28T07:51:13Z

packages/lexical-devtools-core/src/generateContent.ts

+    : undefined;
+  if (customPrint) {
+    return customPrint;
+  } else if ($isTextNode(node)) {


since the if block above returns we dont need to wrap this in an else

Suggested change

} else if ($isTextNode(node)) {

if ($isTextNode(node)) {

I was just following the existing pattern as all the following else if blocks all return. Strictly they could all be changed to if blocks but I think that would make the code harder to read. I'm happy to change them all to if blocks if you prefer that?

the custom print case is a seperate case from the default print so its clearer to have it in its own if block rather then lump it tgt with the default print conditions

That makes sense also, done.

irahopkinson

Thanks for your speedy response.

irahopkinson · 2024-05-28T10:17:08Z

packages/lexical-devtools-core/src/generateContent.ts

@@ -245,9 +249,17 @@ function normalize(text: string, obfuscateText: boolean = false) {
  return textToPrint;
 }

-// TODO Pass via props to allow customizability


This PR allows full customizability for the TreeView print output for any existing node or any custom node and it it does it by passing a function in through the props. Perhaps I'm missing something but what part of that comment still needs implementing?

irahopkinson · 2024-05-28T10:22:00Z

packages/lexical-devtools-core/src/generateContent.ts

+  const customPrint = customPrintNode
+    ? customPrintNode(node, obfuscateText)
+    : undefined;
+  if (customPrint) {
+    return customPrint;


We only want the custom print output if customPrintNode(...) produces some text, i.e. if it returns an empty string because it doesn't handle a node then the existing code below would run and we would get the built-in print output. Your suggested change would not do that but return no output. That's not what we want here.

irahopkinson · 2024-05-28T10:25:17Z

packages/lexical-devtools-core/src/generateContent.ts

+    : undefined;
+  if (customPrint) {
+    return customPrint;
+  } else if ($isTextNode(node)) {


I was just following the existing pattern as all the following else if blocks all return. Strictly they could all be changed to if blocks but I think that would make the code harder to read. I'm happy to change them all to if blocks if you prefer that?

irahopkinson · 2024-05-28T10:34:24Z

what are some example usecases of custom print node?

The test code in the PR description above is one example where we override the existing output for a MarkNode. So existing output can be changed when debugging, e.g. I could also include an attribute's value in the output.

Also you can add handling of TreeView output for custom nodes, e.g. if I created a Custom MarkNode that grouped IDs according to type then I could display the output of those grouped IDs when debugging.

irahopkinson

I also rebased since generateContent.ts file changed on the main branch but I made my changes in a new commit for you to review.

StyleT · 2024-05-28T21:48:16Z

Hi!

Thanks for contributing! I think that overall strategy here is to eventually kill TreeView (see V2 plan here #5675 (comment))... So I'm not sure it's a good idea to expand it now.

@potatowagon wdyt?

irahopkinson · 2024-05-28T22:00:33Z

@StyleT yep totally get that and have seen that plan (and I'm excited for those changes ASAP) but there is no timing on that plan. So even if this change is only useful for a few months that would be helpful for me and hopefully others. Also given that the customPrintNode property is optional I wouldn't think adding it would hurt anything. However, I'm happy to let this PR go if it doesn't fit.

StyleT · 2024-05-28T23:03:43Z

@StyleT yep totally get that and have seen that plan (and I'm excited for those changes ASAP) but there is no timing on that plan. So even if this change is only useful for a few months that would be helpful for me and hopefully others. Also given that the customPrintNode property is optional I wouldn't think adding it would hurt anything. However, I'm happy to let this PR go if it doesn't fit.

I'm not strictly against it for sure! Let's hear other opinions here

potatowagon · 2024-05-29T00:40:57Z

packages/lexical-devtools-core/src/generateContent.ts

@@ -32,6 +32,11 @@ import {

 import {LexicalCommandLog} from './useLexicalCommandsLog';

+export type CustomPrintNode = (


nit: preferably rename CustomPrintNode to CustomPrintNodeFn to not confuse this with other lexical nodes such as TextNode

potatowagon · 2024-05-29T00:42:48Z

the re

Hi!

Thanks for contributing! I think that overall strategy here is to eventually kill TreeView (see V2 plan here #5675 (comment))... So I'm not sure it's a good idea to expand it now.

@potatowagon wdyt?

the example usecases listed by @irahopkinson makes sense to me so im good to have this merge, potentially helps unblock debugging of custom nodes

- added optional `customPrintNode` function property. If the function returns nothing then the built-in output is used. So you can customize existing output and support custom node output. - moved output of mark IDs to `printNode` so it can be customized (it didn't seem to belong where it was anyway). - made appropriate changes to flow.

potatowagon

👍

irahopkinson requested review from zurfyx, fantactuka, acywatson, Fetz and ivailop7 as code owners May 28, 2024 00:21

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 May 28, 2024

vercel bot deployed to Preview – lexical May 28, 2024 00:22 View deployment

vercel bot deployed to Preview – lexical-playground May 28, 2024 00:22 View deployment

potatowagon reviewed May 28, 2024

View reviewed changes

irahopkinson commented May 28, 2024

View reviewed changes

irahopkinson force-pushed the feature/devtools-custom-printNode branch from 7c18796 to 9ff18e0 Compare May 28, 2024 21:16

vercel bot deployed to Preview – lexical May 28, 2024 21:17 View deployment

vercel bot deployed to Preview – lexical-playground May 28, 2024 21:17 View deployment

irahopkinson force-pushed the feature/devtools-custom-printNode branch from 9ff18e0 to 0aa4ec5 Compare May 28, 2024 21:22

vercel bot deployed to Preview – lexical May 28, 2024 21:23 View deployment

vercel bot deployed to Preview – lexical-playground May 28, 2024 21:23 View deployment

irahopkinson commented May 28, 2024

View reviewed changes

potatowagon reviewed May 29, 2024

View reviewed changes

irahopkinson added 3 commits May 29, 2024 12:53

- code review & rebase changes

9d1974c

- code review changes

1fb4dd6

irahopkinson force-pushed the feature/devtools-custom-printNode branch from 0aa4ec5 to 1fb4dd6 Compare May 29, 2024 00:53

vercel bot deployed to Preview – lexical May 29, 2024 00:54 View deployment

vercel bot deployed to Preview – lexical-playground May 29, 2024 00:54 View deployment

potatowagon approved these changes May 29, 2024

View reviewed changes

Sahejkm mentioned this pull request May 29, 2024

[Lexical ] run tests on approval if there is no extended-tests label manually added #6198

Merged

potatowagon added this pull request to the merge queue May 30, 2024

Merged via the queue into facebook:main with commit 5b7e309 May 30, 2024
5 of 6 checks passed

irahopkinson deleted the feature/devtools-custom-printNode branch May 30, 2024 09:56

2wheeh mentioned this pull request May 31, 2024

Feature: add a printNode function to the base node classes and use that in the lexical tree view #6226

Open

irahopkinson mentioned this pull request Jun 25, 2024

[lexical-react][lexical-dev-tools-core] Bug Fix: TreeView selection indicator #6336

Merged

This was referenced Jun 26, 2024

[Snyk] Upgrade @lexical/react from 0.15.0 to 0.16.0 phazingazrael/Terra-Logger#30

Open

[Snyk] Upgrade lexical from 0.15.0 to 0.16.0 phazingazrael/Terra-Logger#31

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lexical-react][lexical-dev-tools-core] Feature: Allow TreeView custom print output #6180

[lexical-react][lexical-dev-tools-core] Feature: Allow TreeView custom print output #6180

irahopkinson commented May 28, 2024 •

edited

Loading

vercel bot commented May 28, 2024 •

edited

Loading

github-actions bot commented May 28, 2024 •

edited

Loading

potatowagon May 28, 2024

irahopkinson May 28, 2024

potatowagon May 28, 2024 •

edited

Loading

irahopkinson May 28, 2024

potatowagon May 28, 2024

irahopkinson May 28, 2024

potatowagon commented May 28, 2024

potatowagon May 28, 2024

irahopkinson May 28, 2024

potatowagon May 28, 2024

irahopkinson May 28, 2024

irahopkinson left a comment

irahopkinson May 28, 2024

irahopkinson May 28, 2024

irahopkinson May 28, 2024

irahopkinson commented May 28, 2024 •

edited

Loading

irahopkinson left a comment •

edited

Loading

StyleT commented May 28, 2024

irahopkinson commented May 28, 2024

StyleT commented May 28, 2024

potatowagon May 29, 2024

irahopkinson May 29, 2024

potatowagon commented May 29, 2024 •

edited

Loading

potatowagon left a comment

		@@ -32,6 +32,11 @@ import {

		import {LexicalCommandLog} from './useLexicalCommandsLog';

		export type CustomPrintNode = (

[lexical-react][lexical-dev-tools-core] Feature: Allow TreeView custom print output #6180

[lexical-react][lexical-dev-tools-core] Feature: Allow TreeView custom print output #6180

Conversation

irahopkinson commented May 28, 2024 • edited Loading

Description

Test plan

Before

After

vercel bot commented May 28, 2024 • edited Loading

github-actions bot commented May 28, 2024 • edited Loading

size-limit report 📦

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potatowagon May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potatowagon commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

irahopkinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

irahopkinson commented May 28, 2024 • edited Loading

irahopkinson left a comment • edited Loading

Choose a reason for hiding this comment

StyleT commented May 28, 2024

irahopkinson commented May 28, 2024

StyleT commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potatowagon commented May 29, 2024 • edited Loading

potatowagon left a comment

Choose a reason for hiding this comment

irahopkinson commented May 28, 2024 •

edited

Loading

vercel bot commented May 28, 2024 •

edited

Loading

github-actions bot commented May 28, 2024 •

edited

Loading

potatowagon May 28, 2024 •

edited

Loading

irahopkinson commented May 28, 2024 •

edited

Loading

irahopkinson left a comment •

edited

Loading

potatowagon commented May 29, 2024 •

edited

Loading