Skip to content

Commit d012f0a

Browse files
authored
Fix list markdown behaviors (#906)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes a couple bugs with lists. Indentation levels for nested lists on edit and lists rendering paragraph tags on new lines when they shouldnt. #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings ### AI disclosure: - [x] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Tests were fully AI generated.
2 parents 744ebda + ee575c0 commit d012f0a

9 files changed

Lines changed: 116 additions & 4 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Fixed lists rendering `p` html tags on new lines.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Fixed nested lists having wrong indentation levels when editing.

src/app/plugins/markdown/bidirectional.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,26 @@ describe('bidirectional round-trip', () => {
103103
expect(result).toContain('2. Second');
104104
});
105105

106+
it('round-trips nested sublists with four-space indent', () => {
107+
const markdown = '1. parent\n - child';
108+
const html = markdownToHtml(markdown);
109+
expect(html).toMatch(/<li>[\s\S]*<ul/i);
110+
const injected = injectDataMd(html);
111+
const result = htmlToMarkdown(injected);
112+
expect(result).toContain('1. parent');
113+
expect(result).toContain(' - child');
114+
});
115+
116+
it('round-trips nested sublists written with two-space indent', () => {
117+
const markdown = '1. parent\n - child';
118+
const html = markdownToHtml(markdown);
119+
expect(html).toMatch(/<li>[\s\S]*<ul/i);
120+
const injected = injectDataMd(html);
121+
const result = htmlToMarkdown(injected);
122+
expect(result).toContain('1. parent');
123+
expect(result).toContain('- child');
124+
});
125+
106126
it('round-trips spoiler syntax', () => {
107127
const markdown = '||hidden message||';
108128
const html = markdownToHtml(markdown);

src/app/plugins/markdown/expandBlockNewlines.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,24 @@ describe('expandBlockBoundariesAfterSingleNewlines', () => {
1515
it('still expands when a blockquote ends', () => {
1616
expect(expandBlockBoundariesAfterSingleNewlines('> quote\nplain')).toBe('> quote\n\nplain');
1717
});
18+
19+
it('does not expand between consecutive ordered list items', () => {
20+
expect(expandBlockBoundariesAfterSingleNewlines('1. one\n2. two')).toBe('1. one\n2. two');
21+
});
22+
23+
it('does not expand before a 2-space nested sublist', () => {
24+
expect(expandBlockBoundariesAfterSingleNewlines('1. test\n - sub')).toBe('1. test\n - sub');
25+
});
26+
27+
it('does not expand before a 4-space nested sublist', () => {
28+
expect(expandBlockBoundariesAfterSingleNewlines('1. test\n - sub')).toBe(
29+
'1. test\n - sub'
30+
);
31+
});
32+
33+
it('still expands before the first top-level list item after prose', () => {
34+
expect(expandBlockBoundariesAfterSingleNewlines('intro\n- item')).toBe('intro\n\n- item');
35+
});
1836
});
1937

2038
describe('consecutive blockquotes', () => {

src/app/plugins/markdown/expandBlockNewlines.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,41 @@ function nextLineIsBlockStarter(md: string, newlineIdx: number): boolean {
135135
return looksLikeBlockStart(effective);
136136
}
137137

138+
function leadingSpaces(line: string): number {
139+
let k = 0;
140+
while (k < line.length && line[k] === ' ') k++;
141+
return k;
142+
}
143+
144+
function lineLooksLikeListItem(line: string): boolean {
145+
const effective = effectiveContentAfterEscapes(line);
146+
if (effective === null) return false;
147+
return /^\d{1,9}\.\s/.test(effective) || /^[-*+]\s/.test(effective);
148+
}
149+
150+
function nextLineIsNestedListItem(md: string, newlineIdx: number): boolean {
151+
const prevLine = lineAtNewline(md, newlineIdx);
152+
const nextLine = lineAfterNewline(md, newlineIdx);
153+
if (!lineLooksLikeListItem(nextLine)) return false;
154+
return leadingSpaces(nextLine) > leadingSpaces(prevLine);
155+
}
156+
157+
function nextLineIsSiblingListItem(md: string, newlineIdx: number): boolean {
158+
const prevLine = lineAtNewline(md, newlineIdx);
159+
const nextLine = lineAfterNewline(md, newlineIdx);
160+
if (!lineLooksLikeListItem(prevLine) || !lineLooksLikeListItem(nextLine)) return false;
161+
return leadingSpaces(prevLine) === leadingSpaces(nextLine);
162+
}
163+
138164
function shouldExpandSingleNewline(md: string, newlineIdx: number): boolean {
139165
// Consecutive `>` lines belong to one blockquote, keep the single `\n` between them.
140166
if (prevLineIsBlockquote(md, newlineIdx) && nextLineContinuesBlockquote(md, newlineIdx)) {
141167
return false;
142168
}
169+
// Keep single `\n` between list items (siblings) and before nested sublists (2- or 4-space indent).
170+
if (nextLineIsSiblingListItem(md, newlineIdx) || nextLineIsNestedListItem(md, newlineIdx)) {
171+
return false;
172+
}
143173
if (nextLineIsBlockStarter(md, newlineIdx)) return true;
144174
// CommonMark lazy continuation keeps non-`>` lines inside blockquotes, close on single `\n`.
145175
if (prevLineIsBlockquote(md, newlineIdx) && !nextLineContinuesBlockquote(md, newlineIdx)) {

src/app/plugins/markdown/htmlToMarkdown.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import {
44
MX_EMOTICON_MD_SEP,
55
MX_EMOTICON_MD_START,
66
} from './extensions/matrix-emoticon';
7+
import { toMatrixCustomHTML, trimCustomHtml } from '$components/editor/output';
78
import { plainToEditorInput } from '$components/editor/input';
89
import { BlockType } from '$components/editor/types';
10+
import { injectDataMd } from './injectDataMd';
911
import { htmlToMarkdown } from './htmlToMarkdown';
12+
import { markdownToHtml } from './markdownToHtml';
1013

1114
describe('htmlToMarkdown', () => {
1215
it('converts headings', () => {
@@ -125,6 +128,20 @@ describe('htmlToMarkdown', () => {
125128
expect(result).toContain('Item 1');
126129
});
127130

131+
it('indents nested sublists with four spaces per level', () => {
132+
const html = '<ol><li><p>parent</p><ul><li><p>child</p></li></ul></li></ol>';
133+
expect(htmlToMarkdown(html)).toContain('1. parent');
134+
expect(htmlToMarkdown(html)).toContain(' - child');
135+
});
136+
137+
it('edit-load and save round-trip keeps nested sublist', () => {
138+
const md = '1. test\n - sub\n';
139+
const loaded = htmlToMarkdown(injectDataMd(markdownToHtml(md)));
140+
expect(loaded).toContain(' - sub');
141+
const html = trimCustomHtml(toMatrixCustomHTML(plainToEditorInput(loaded), {}));
142+
expect(html).toMatch(/<li>[\s\S]*<ul/i);
143+
});
144+
128145
it('preserves data-md attributes for round-trip', () => {
129146
const result = htmlToMarkdown('<strong data-md="**">bold</strong>');
130147
expect(result).toContain('**bold**');

src/app/plugins/markdown/htmlToMarkdown.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { isAllowedHtmlTag } from './allowedHtmlTags';
1111
import { formatMfmColorDataMd } from './extensions/matrix-mfm-color';
1212
import { isMatrixHexColor } from '$utils/matrixHtml';
1313

14+
/** CommonMark list nesting indent (four spaces per level). */
15+
const LIST_MARKDOWN_INDENT = ' ';
16+
1417
/**
1518
* Converts Matrix-compatible HTML back to markdown for round-trip editing.
1619
* Preserves original markdown syntax via data-md attributes and converts
@@ -419,7 +422,7 @@ function processUnorderedList(
419422
insideCode: boolean = false
420423
): string {
421424
const mdSequence = node.attribs['data-md'] || '-';
422-
const indent = ' '.repeat(depth);
425+
const indent = LIST_MARKDOWN_INDENT.repeat(depth);
423426
const items = node.children
424427
.filter((c): c is Element => isTag(c) && c.name === 'li')
425428
.map((li) => {
@@ -439,7 +442,7 @@ function processOrderedList(node: Element, depth: number = 0, insideCode: boolea
439442
? mdSequence
440443
: `${mdSequence}.`;
441444

442-
const indent = ' '.repeat(depth);
445+
const indent = LIST_MARKDOWN_INDENT.repeat(depth);
443446
const items = node.children
444447
.filter((c): c is Element => isTag(c) && c.name === 'li')
445448
.map((li, index) => {

src/app/plugins/react-custom-html-parser.test.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,4 +446,15 @@ describe('react custom html parser', () => {
446446
expect(screen.queryByText('&lt;test&gt;')).not.toBeInTheDocument();
447447
}
448448
);
449+
450+
it('unwraps paragraph tags inside list items instead of rendering block breaks', () => {
451+
const { container } = renderMessage(
452+
'<ol><li><p>one</p></li><li><p>two</p></li></ol><ul><li><p>bullet</p></li></ul>'
453+
);
454+
455+
expect(container.querySelector('p')).toBeNull();
456+
expect(container.textContent).toContain('one');
457+
expect(container.textContent).toContain('two');
458+
expect(container.textContent).toContain('bullet');
459+
});
449460
});

src/app/plugins/react-custom-html-parser.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ function KatexRenderer({
111111

112112
useEffect(() => {
113113
let mounted = true;
114-
Promise.all([import('katex'), import('katex/dist/katex.min.css')]).then(([katex]) => {
114+
void Promise.all([import('katex'), import('katex/dist/katex.min.css')]).then(([katex]) => {
115115
if (mounted) {
116116
setHtml(katex.default.renderToString(math, { throwOnError: false, displayMode }));
117117
}
@@ -430,7 +430,7 @@ export function CodeBlock({
430430
const [copied, setCopied] = useTimeoutToggle();
431431

432432
const handleCopy = () => {
433-
copyToClipboard(extractTextFromChildren(children));
433+
void copyToClipboard(extractTextFromChildren(children));
434434
setCopied();
435435
};
436436

@@ -631,6 +631,9 @@ export const getReactCustomHtmlParser = (
631631
}
632632

633633
if (name === 'p') {
634+
if (parent instanceof Element && parent.name === 'li') {
635+
return <>{renderChildren()}</>;
636+
}
634637
return (
635638
<Text {...props} className={classNames(css.Paragraph, css.MarginSpaced)} size="Inherit">
636639
{renderChildren()}

0 commit comments

Comments
 (0)