[lexical-markdown][lexical-playground] Bug Fix: Keep indent when switching with markdown#7845
[lexical-markdown][lexical-playground] Bug Fix: Keep indent when switching with markdown#7845lytion wants to merge 18 commits intofacebook:mainfrom
Conversation
This transformer allows to keep indentation when switching with markdown
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Failing lint https://github.com/facebook/lexical/actions/runs/17919434734/job/50950181819?pr=7845 |
|
Now the prettier check is failing https://github.com/facebook/lexical/actions/runs/17919849090/job/50951631139 You can run the integrity checks locally with |
# Conflicts: # packages/lexical-markdown/src/MarkdownTransformers.ts
|
Is there some way to run tests in a linux environment or should I test it in a vm ? |
| return; | ||
| } | ||
| parentNode.setIndent(indents.length); | ||
| textNode.setTextContent(textNode.getTextContent().replace(/^\t+/, '')); |
There was a problem hiding this comment.
This seems like maybe there's a bug elsewhere because tabs are usually supposed to use TabNode
There was a problem hiding this comment.
I noticed that this PR was updated with main but this comment hasn't been addressed
There was a problem hiding this comment.
Sorry I missed the comment.
I think you're right, when clicking on "left indent" button in normal mode, there's no INDENT_CONTENT_COMMAND which I believe should happen and therefore should add a TabNode.
There was a problem hiding this comment.
I want to make sure we're on the same page.
From what I see:
Markdown use \t for indent and tab key.
CodeHighlight convert \t into TabNode when converting to markdown.
Nodes use __indent and TabNode.
Here the transformer set the correct number of indent and remove all \t found from the start of the line.
If I understand correctly, what you're talking about is to convert all \t into TabNode even if CodeHighlight is disabled.
I can create a TAB transformer that does that, but maybe that's a separate issue
There was a problem hiding this comment.
Generally speaking there should never be '\t' or a '\n' in a TextNode, anything that creates a TextNode with those characters in the contents probably has a bug. The solution isn't a transformer, it's to fix the code that's creating those TextNodes. I don't think there is currently a helper to do this other than RangeSelection.insertRawText but one could be added to make this a little easier.
There was a problem hiding this comment.
This is a quick and dirty refactor but is this the direction you are thinking about ?
diff --git a/packages/lexical-markdown/src/MarkdownImport.ts b/packages/lexical-markdown/src/MarkdownImport.ts
index d64037d91..851707205 100644
--- a/packages/lexical-markdown/src/MarkdownImport.ts
+++ b/packages/lexical-markdown/src/MarkdownImport.ts
+// Copy of LexicalSelection insertRawText function
+function insertRawText(text: string): LexicalNode[] {
+ const parts = text.split(/(\r?\n|\t)/);
+ const nodes = [];
+ const length = parts.length;
+ for (let i = 0; i < length; i++) {
+ const part = parts[i];
+ if (part === '\n' || part === '\r\n') {
+ nodes.push($createLineBreakNode());
+ } else if (part === '\t') {
+ nodes.push($createTabNode());
+ } else {
+ nodes.push($createTextNode(part));
+ }
+}
+ return nodes;
+}
+
function $importBlocks(
lineText: string,
rootNode: ElementNode,
@@ -227,27 +248,39 @@ function $importBlocks(
textMatchTransformers: Array<TextMatchTransformer>,
shouldPreserveNewLines: boolean,
) {
- const textNode = $createTextNode(lineText);
+ const nodes = insertRawText(lineText);
const elementNode = $createParagraphNode();
- elementNode.append(textNode);
+ elementNode.append(...nodes);
rootNode.append(elementNode);
for (const {regExp, replace} of elementTransformers) {
- const match = lineText.match(regExp);
-
- if (match) {
- textNode.setTextContent(lineText.slice(match[0].length));
- if (replace(elementNode, [textNode], match, true) !== false) {
+ for (const node of nodes) {
+ if (!$isTextNode(node)) {
break;
}
+ const match = node.getTextContent().match(regExp);
+
+ if (match) {
+ node.setTextContent(lineText.slice(match[0].length));
+ if (replace(elementNode, [node], match, true) !== false) {
+ break;
+ }
+ }
}
}
- importTextTransformers(
- textNode,
- textFormatTransformersIndex,
- textMatchTransformers,
- );
+ for (const node of nodes) {
+ if (!$isTextNode(node)) {
+ break;
+ }
+ // TODO: Do this for TabNode too (maybe Line
+ importTextTransformers(
+ node,
+ textFormatTransformersIndex,
+ textMatchTransformers,
+ );
+ }
+
There was a problem hiding this comment.
Hard to say without a thorough audit of that code but probably something like that. It might be the case that it’s better to fix it up afterwards in the markdown case depending on how that code works.
Co-authored-by: Bob Ippolito <bob@redivi.com>
Description
Reported issue: #7820
To summarize, if you use indent button and then switch to markdown, indent will be lost.
I decided to fix this issue by adding a transformer by I think it could also be fixed by handling it directly in
createMarkdownExportandcreateMarkdownImportThis transformer simply add '\t' when exporting to markdown and remove those '\t' and set indent when importing from markdown. This only happen if '\t' is found at the beginning of the string and if the parent is a paragraph to avoid applying it to lists.
Closes #7820