Skip to content

Commit b72fe3e

Browse files
mtsgrdmtsgrd
committed
Fix text editor paragraph handling regressions
This change removes the plaintext plugin and updates all existing components to properly handle ParagraphNode elements in addition to LineBreakNode elements. This fixes several regressions including: - Incorrect rewrapping when editing in the middle of auto-wrapped paragraphs - Blank lines being collapsed during initialization
1 parent 5aa0404 commit b72fe3e

14 files changed

+946
-229
lines changed

‎packages/ui/src/lib/richText/RichTextEditor.svelte‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { WRAP_ALL_COMMAND } from '$lib/richText/commands';
44
import { standardConfig } from '$lib/richText/config/config';
55
import { standardTheme } from '$lib/richText/config/theme';
6-
import { INLINE_CODE_TRANSFORMER, PARAGRAPH_TRANSFORMER } from '$lib/richText/customTransforers';
6+
import { INLINE_CODE_TRANSFORMER } from '$lib/richText/customTransforers';
77
import { getCurrentText } from '$lib/richText/getText';
88
// import CodeBlockTypeAhead from '$lib/richText/plugins/CodeBlockTypeAhead.svelte';
99
import EmojiPlugin from '$lib/richText/plugins/Emoji.svelte';
@@ -321,7 +321,7 @@
321321
<PlainTextPlugin />
322322
<PlainTextIndentPlugin />
323323
<!-- <CodeBlockTypeAhead /> -->
324-
<MarkdownShortcutPlugin transformers={[INLINE_CODE_TRANSFORMER, PARAGRAPH_TRANSFORMER]} />
324+
<MarkdownShortcutPlugin transformers={[INLINE_CODE_TRANSFORMER]} />
325325
{:else}
326326
<AutoLinkPlugin />
327327
<CheckListPlugin />

‎packages/ui/src/lib/richText/customTransforers.ts‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,5 @@
11
import { createInlineCodeNode } from '$lib/richText/node/inlineCode';
2-
import { $isParagraphNode, ParagraphNode } from 'lexical';
3-
import type { ElementTransformer, Transformer } from '@lexical/markdown';
4-
5-
/**
6-
* A transformer used for exporting to markdown, where each paragraph
7-
* becomes its own line separated by `\n`.
8-
*
9-
* NOTE: This transformer is export-only and should never match during typing.
10-
* The regExp is set to never match to prevent interference with user input.
11-
*/
12-
export const PARAGRAPH_TRANSFORMER: ElementTransformer = {
13-
dependencies: [ParagraphNode],
14-
export: (node, traverseChildren) => {
15-
if ($isParagraphNode(node)) {
16-
const nextSibling = node.getNextSibling();
17-
const text = traverseChildren(node);
18-
// Each paragraph becomes a line, separated by a single newline
19-
if (nextSibling !== null) {
20-
return `${text}`;
21-
}
22-
return text;
23-
}
24-
return null;
25-
},
26-
regExp: /$.^/,
27-
replace: () => false,
28-
type: 'element'
29-
};
2+
import type { Transformer } from '@lexical/markdown';
303

314
export const INLINE_CODE_TRANSFORMER: Transformer = {
325
type: 'text-match',

‎packages/ui/src/lib/richText/linewrap.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,56 @@ describe('wrapline', () => {
9898
expect(newLine).toEqual('- hello ');
9999
expect(remainder).toEqual('world');
100100
});
101+
102+
test('commit message line under 72 chars should not wrap', () => {
103+
// Line is 68 chars, should not wrap
104+
const { newLine, newRemainder: remainder } = wrapLine({
105+
line: 'The introduction of InlineCodeNode revealed that the text editor was',
106+
maxLength: 72
107+
});
108+
expect(newLine).toEqual('The introduction of InlineCodeNode revealed that the text editor was');
109+
expect(remainder).toEqual('');
110+
});
111+
112+
test('commit message second line under 72 chars should not wrap', () => {
113+
// Line is 68 chars, should not wrap
114+
const { newLine, newRemainder: remainder } = wrapLine({
115+
line: 'operating in a simplified rich text mode rather than plaintext mode,',
116+
maxLength: 72
117+
});
118+
expect(newLine).toEqual('operating in a simplified rich text mode rather than plaintext mode,');
119+
expect(remainder).toEqual('');
120+
});
121+
122+
test('line that exceeds 72 chars by 1', () => {
123+
const { newLine, newRemainder: remainder } = wrapLine({
124+
line: 'since visually distinct backtick-quoted text requires rich text features.',
125+
maxLength: 72
126+
});
127+
// Line is 73 chars, should wrap
128+
expect(newLine.length).toBeLessThanOrEqual(72);
129+
expect(remainder).toBeTruthy();
130+
// Verify no characters are lost
131+
const reconstructed = newLine.trim() + ' ' + remainder;
132+
expect(reconstructed).toEqual(
133+
'since visually distinct backtick-quoted text requires rich text features.'
134+
);
135+
});
136+
137+
test('bullet line that exceeds 72 chars', () => {
138+
const { newLine, newRemainder: remainder } = wrapLine({
139+
line: '- Incorrect rewrapping when editing in the middle of auto-wrapped paragraphs',
140+
maxLength: 72,
141+
bullet: { prefix: '- ', indent: ' ' }
142+
});
143+
// Line is 76 chars, should wrap
144+
expect(newLine.length).toBeLessThanOrEqual(72);
145+
expect(remainder).toBeTruthy();
146+
// Verify no characters are lost (accounting for bullet formatting)
147+
const originalText =
148+
'Incorrect rewrapping when editing in the middle of auto-wrapped paragraphs';
149+
const newText = newLine.substring(2).trim(); // Remove '- ' prefix
150+
const reconstructed = newText + ' ' + remainder;
151+
expect(reconstructed).toEqual(originalText);
152+
});
101153
});

‎packages/ui/src/lib/richText/linewrap.ts‎

Lines changed: 72 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,28 @@ export function wrapLine({ line, maxLength, remainder = '', indent = '', bullet
3131
newLine: string;
3232
newRemainder: string;
3333
} {
34-
const parts = Array.from(line.substring(indent.length).match(/([ \t]+|\S+)/g) || []);
34+
// When we have a bullet, skip the bullet prefix to get the actual text parts
35+
const prefixLength = bullet ? bullet.prefix.length : indent.length;
36+
const parts = Array.from(line.substring(prefixLength).match(/([ \t]+|\S+)/g) || []);
3537
let acc = remainder.length > 0 ? indent + remainder + ' ' : bullet ? bullet.prefix : indent;
36-
for (const word of parts) {
37-
if (acc.length + word.length > maxLength) {
38-
const newLine = acc ? acc : word;
39-
const concatLine = remainder
40-
? indent + remainder + ' ' + line.substring(indent.length)
41-
: line;
42-
const newRemainder = concatLine.slice(newLine.length).trim();
4338

39+
for (let i = 0; i < parts.length; i++) {
40+
const word = parts[i];
41+
if (acc.length + word.length > maxLength) {
42+
// If acc is empty/just prefix, use the current word as newLine
43+
// and start remainder from next word
44+
if (!acc || acc === indent || acc === (bullet?.prefix ?? '')) {
45+
const remainingParts = parts.slice(i + 1);
46+
return {
47+
newLine: word,
48+
newRemainder: remainingParts.join('').trim()
49+
};
50+
}
51+
// Otherwise, acc becomes newLine and remainder starts from current word
52+
const remainingParts = parts.slice(i);
4453
return {
45-
newLine,
46-
newRemainder
54+
newLine: acc,
55+
newRemainder: remainingParts.join('').trim()
4756
};
4857
}
4958
acc += word;
@@ -85,7 +94,7 @@ export function parseBullet(text: string): Bullet | undefined {
8594
return { prefix, indent, number };
8695
}
8796

88-
export function wrapIfNecssary({ node, maxLength }: { node: TextNode; maxLength: number }) {
97+
export function wrapIfNecessary({ node, maxLength }: { node: TextNode; maxLength: number }) {
8998
const line = node.getTextContent();
9099
if (line.length <= maxLength) {
91100
return;
@@ -102,14 +111,14 @@ export function wrapIfNecssary({ node, maxLength }: { node: TextNode; maxLength:
102111
const paragraph = node.getParent();
103112

104113
if (!$isParagraphNode(paragraph)) {
105-
console.warn('[wrapIfNecssary] Node parent is not a paragraph:', paragraph?.getType());
114+
console.warn('[wrapIfNecessary] Node parent is not a paragraph:', paragraph?.getType());
106115
return;
107116
}
108117

109118
const selection = getSelection();
110119
const selectionOffset = isRangeSelection(selection) ? selection.focus.offset : 0;
111120

112-
// Wrap the current line
121+
// Wrap only the current line - don't collect other paragraphs
113122
const { newLine, newRemainder } = wrapLine({
114123
line,
115124
maxLength,
@@ -120,48 +129,23 @@ export function wrapIfNecssary({ node, maxLength }: { node: TextNode; maxLength:
120129
// Update current text node
121130
node.setTextContent(newLine);
122131

123-
// If there's a remainder, we need to create new paragraphs or reuse related ones
132+
// If there's a remainder, create new paragraphs for it
124133
if (newRemainder) {
125134
let remainder = newRemainder;
126135
let lastParagraph = paragraph;
127136

128-
// Get related paragraphs (paragraphs with same indentation following this one)
129-
const relatedParagraphs = getRelatedParagraphs(paragraph, indent);
130-
131-
// Process the remainder with related paragraphs
132-
for (const relatedPara of relatedParagraphs) {
133-
if (!remainder) break;
134-
135-
const relatedText = relatedPara.getTextContent();
136-
137-
// Combine remainder with related paragraph text
138-
const combinedText = remainder + ' ' + relatedText;
139-
const { newLine: wrappedLine, newRemainder: newRem } = wrapLine({
140-
line: combinedText,
141-
maxLength,
142-
indent
143-
});
144-
145-
// Update the related paragraph
146-
const textNode = relatedPara.getFirstChild();
147-
if (isTextNode(textNode)) {
148-
textNode.setTextContent(wrappedLine);
149-
}
150-
151-
remainder = newRem;
152-
lastParagraph = relatedPara;
153-
}
154-
155-
// Create new paragraphs for any remaining text
137+
// Create new paragraphs for the wrapped text
156138
while (remainder && remainder.length > 0) {
139+
// Prepend indent to the remainder before wrapping it
140+
const indentedLine = indent + remainder;
157141
const { newLine: finalLine, newRemainder: finalRem } = wrapLine({
158-
line: remainder,
142+
line: indentedLine,
159143
maxLength,
160144
indent
161145
});
162146

163147
const newParagraph = new ParagraphNode();
164-
const newTextNode = new TextNode(indent + finalLine);
148+
const newTextNode = new TextNode(finalLine);
165149
newParagraph.append(newTextNode);
166150
lastParagraph.insertAfter(newParagraph);
167151

@@ -170,85 +154,59 @@ export function wrapIfNecssary({ node, maxLength }: { node: TextNode; maxLength:
170154
}
171155

172156
// Try to maintain cursor position
173-
if (selectionOffset > newLine.length) {
174-
// Cursor was after the wrap point
175-
// Try to find which paragraph it should be in now
176-
let targetPara = paragraph;
177-
let accumulatedLength = newLine.length;
178-
179-
// Walk through paragraphs to find where cursor should land
180-
let nextPara = paragraph.getNextSibling();
181-
while (nextPara && $isParagraphNode(nextPara)) {
182-
const nextText = nextPara.getFirstChild();
183-
if (!isTextNode(nextText)) break;
184-
185-
const paraLength = nextText.getTextContentSize();
186-
187-
// Check if cursor should be in this paragraph
188-
if (selectionOffset <= accumulatedLength + paraLength) {
189-
const offset = Math.min(selectionOffset - accumulatedLength, paraLength);
190-
nextText.select(offset, offset);
191-
return;
192-
}
193-
194-
accumulatedLength += paraLength;
195-
targetPara = nextPara;
196-
nextPara = nextPara.getNextSibling();
197-
198-
// Stop at non-related paragraphs
199-
const text = targetPara.getTextContent();
200-
const paraIndent = parseIndent(text);
201-
if (paraIndent !== indent || parseBullet(text)) {
157+
// Calculate which paragraph the cursor should end up in
158+
let remainingOffset = selectionOffset;
159+
160+
// If cursor was in the first line
161+
if (remainingOffset <= newLine.length) {
162+
// Keep cursor in the current paragraph at the same position
163+
node.select(remainingOffset, remainingOffset);
164+
} else {
165+
// Cursor should be in one of the wrapped paragraphs
166+
remainingOffset -= newLine.length + 1; // Account for the line and space
167+
168+
// Walk through the created paragraphs to find where cursor belongs
169+
let currentPara: ParagraphNode | null = paragraph.getNextSibling() as ParagraphNode | null;
170+
let tempRemainder = newRemainder;
171+
172+
// Calculate all the wrapped lines to find cursor position
173+
while (tempRemainder && tempRemainder.length > 0) {
174+
const indentedLine = indent + tempRemainder;
175+
const { newLine: tempLine, newRemainder: tempRem } = wrapLine({
176+
line: indentedLine,
177+
maxLength,
178+
indent
179+
});
180+
181+
// tempLine now includes the indent, so just check against its length
182+
if (remainingOffset <= tempLine.length) {
183+
// Cursor belongs in this line
202184
break;
203185
}
186+
remainingOffset -= tempLine.length + 1; // +1 for space between lines
187+
tempRemainder = tempRem;
188+
currentPara = currentPara?.getNextSibling() as ParagraphNode | null;
204189
}
205190

206-
// If we didn't find a place, put cursor at end of last paragraph
207-
if (targetPara) {
208-
const lastText = targetPara.getFirstChild();
209-
if (isTextNode(lastText)) {
210-
lastText.selectEnd();
191+
// Set cursor in the appropriate paragraph
192+
if (currentPara && $isParagraphNode(currentPara)) {
193+
const textNode = currentPara.getFirstChild();
194+
if (isTextNode(textNode)) {
195+
textNode.select(Math.max(0, remainingOffset), Math.max(0, remainingOffset));
196+
}
197+
} else {
198+
// Fallback: put cursor at end of last created paragraph
199+
if (lastParagraph) {
200+
const textNode = lastParagraph.getFirstChild();
201+
if (isTextNode(textNode)) {
202+
textNode.selectEnd();
203+
}
211204
}
212205
}
213206
}
214207
}
215208
}
216209

217-
/**
218-
* Returns paragraphs that follow the given paragraph that are considered part of the same
219-
* logical paragraph. This enables us to re-wrap a paragraph when edited in the middle.
220-
*
221-
* In the multi-paragraph structure, "related paragraphs" are those with the same
222-
* indentation and no bullet points, representing continuation of the same text block.
223-
*/
224-
function getRelatedParagraphs(paragraph: ParagraphNode, indent: string): ParagraphNode[] {
225-
const collectedParagraphs: ParagraphNode[] = [];
226-
let next = paragraph.getNextSibling();
227-
228-
while (next && $isParagraphNode(next)) {
229-
const text = next.getTextContent();
230-
231-
// Empty paragraphs break the chain
232-
if (text.trimStart() === '') {
233-
break;
234-
}
235-
236-
// We don't consider altered indentations or new bullet points to be
237-
// part of the same logical paragraph.
238-
const bullet = parseBullet(text);
239-
const lineIndent = parseIndent(text);
240-
241-
if (indent !== lineIndent || bullet) {
242-
break;
243-
}
244-
245-
collectedParagraphs.push(next);
246-
next = next.getNextSibling();
247-
}
248-
249-
return collectedParagraphs;
250-
}
251-
252210
export function wrapAll(editor: LexicalEditor, maxLength: number) {
253211
editor.update(
254212
() => {
@@ -259,7 +217,7 @@ export function wrapAll(editor: LexicalEditor, maxLength: number) {
259217
if ($isParagraphNode(child)) {
260218
const textNode = child.getFirstChild();
261219
if (isTextNode(textNode)) {
262-
wrapIfNecssary({ node: textNode, maxLength });
220+
wrapIfNecessary({ node: textNode, maxLength });
263221
}
264222
}
265223
}

0 commit comments

Comments
 (0)