From 5a16b7add8ea1e8f2fab3656a6cdba30185bfe76 Mon Sep 17 00:00:00 2001 From: Robert Vitonsky Date: Sat, 10 Oct 2026 19:51:07 +0300 Subject: [PATCH 1/5] fix: prevent lists merging --- .../__tests__/spec/list.browser.test.ts | 46 +++++++++++++++ .../remark/remarkPreserveBlankLines.ts | 57 ++++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts index c63bc3851..852e54d6c 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts @@ -110,3 +110,49 @@ test('item nesting can be decreased and empty list will be removed', async () => .poll(() => onChanged) .toHaveBeenLastCalledWith('- [ ] 1\n- [ ] 2\n- [ ] 3\n- [ ] 4\n- [ ] 5\n'); }); + +export const unjoinedLists = ` +- foo +- bar +- baz + + +- [ ] 1 +- [ ] 2 +- [ ] 3 +`.trim(); + +describe('Regressions', () => { + test('a regular list and a checklist separated by blank lines remain separate', async () => { + await renderRichEditorInDOM({ value: unjoinedLists }); + + expect(page.getByRole('list').all()).toHaveLength(2); + }); + + test('nested items stay nested when a regular list is followed by a checklist', async () => { + const value = ` +- foo + - bar +- baz + +- [ ] 1 + - [ ] 2 + - [ ] 3 +- [ ] 4 +- [ ] 5 +`.trim(); + + await renderRichEditorInDOM({ value }); + + const editor = page.getByRole('textbox'); + expect(editor.element().querySelectorAll(':scope > ul')).toHaveLength(2); + expect(editor.getByRole('list').all()).toHaveLength(5); + expect(editor.getByRole('checkbox')).toHaveLength(5); + }); + + test('blank-separated sibling items of the same kind remain one list', async () => { + await renderRichEditorInDOM({ value: '- foo\n\n- bar' }); + + expect(page.getByRole('list').all()).toHaveLength(1); + }); +}); diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts index c6ddd8c0f..1687c98a4 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts @@ -1,4 +1,4 @@ -import { type Root, RootContent } from 'mdast'; +import { type List, type ListItem, type Root, RootContent } from 'mdast'; import { Plugin } from 'unified'; import { CONTINUE, SKIP, visit } from 'unist-util-visit'; @@ -8,6 +8,55 @@ const ignoredNodeTypes = new Set([ 'tableRow', ] satisfies RootContent['type'][]); +const isTaskListItem = (item: ListItem) => + item.checked !== null && item.checked !== undefined; + +// Lexical represents task lists and regular lists as different list types, but +// mdast can keep both kinds of item in one list when there is a blank line. +const splitListAtTaskBoundary = (list: List): List[] => { + const groups: { items: ListItem[]; startIndex: number }[] = []; + let items: ListItem[] = []; + let startIndex = 0; + + for (let i = 0; i < list.children.length; i++) { + const item = list.children[i]; + const previousItem = items.at(-1); + const hasBlankLineBefore = + previousItem?.position && + item.position && + item.position.start.line > previousItem.position.end.line + 1; + const changesTaskStatus = + previousItem && isTaskListItem(previousItem) !== isTaskListItem(item); + + if (hasBlankLineBefore && changesTaskStatus) { + groups.push({ items, startIndex }); + items = []; + startIndex = i; + } + + items.push(item); + } + + if (items.length > 0) groups.push({ items, startIndex }); + if (groups.length <= 1) return [list]; + + return groups.map(({ items: groupItems, startIndex: groupStartIndex }) => { + const firstItem = groupItems[0]; + const lastItem = groupItems[groupItems.length - 1]; + const position = + list.position && firstItem.position && lastItem.position + ? { start: firstItem.position.start, end: lastItem.position.end } + : list.position; + + return { + ...list, + ...(list.ordered ? { start: (list.start ?? 1) + groupStartIndex } : {}), + children: groupItems, + position, + }; + }); +}; + export const fillGapsWithParagraphs = (tree: Root) => { const skipNodes = new Set(); @@ -27,7 +76,11 @@ export const fillGapsWithParagraphs = (tree: Root) => { const next = node.children[i + 1]; // Collect its own children - newChildren.push(current); + newChildren.push( + ...(current.type === 'list' + ? splitListAtTaskBoundary(current) + : [current]), + ); // Add empty lines to preserve if (next && current.position && next.position) { From 0d20eaccce8a8f119140c6514edb4c000a9543fe Mon Sep 17 00:00:00 2001 From: Robert Vitonsky Date: Sat, 10 Oct 2026 22:22:18 +0300 Subject: [PATCH 2/5] test: add cases --- .../__tests__/spec/list.browser.test.ts | 63 ++++++++++++++----- 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts index 852e54d6c..55c24c622 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts @@ -111,22 +111,55 @@ test('item nesting can be decreased and empty list will be removed', async () => .toHaveBeenLastCalledWith('- [ ] 1\n- [ ] 2\n- [ ] 3\n- [ ] 4\n- [ ] 5\n'); }); -export const unjoinedLists = ` -- foo -- bar -- baz - - -- [ ] 1 -- [ ] 2 -- [ ] 3 -`.trim(); - describe('Regressions', () => { - test('a regular list and a checklist separated by blank lines remain separate', async () => { - await renderRichEditorInDOM({ value: unjoinedLists }); - - expect(page.getByRole('list').all()).toHaveLength(2); + describe('Different lists separated by blank lines remain separate', () => { + [ + { + name: 'regular list and checklist', + expectedLists: 2, + content: [ + '- foo', + '- bar', + '- baz', + '', + '- [ ] 1', + '- [ ] 2', + '- [ ] 3', + ].join('\n'), + }, + { + name: 'regular list and ordered list', + expectedLists: 2, + content: ['- foo', '- bar', '- baz', '', '1. 1', '2. 2', '3. 3'].join( + '\n', + ), + }, + { + name: 'regular list, ordered list, checkbox list', + expectedLists: 3, + content: [ + '- foo', + '- bar', + '- baz', + '', + '1. 1', + '2. 2', + '3. 3', + '', + '- [ ] 1', + '- [ ] 2', + '- [ ] 3', + ].join('\n'), + }, + ].forEach(({ name, content, expectedLists }) => + test(name, async () => { + await renderRichEditorInDOM({ + value: content, + }); + + expect(page.getByRole('list').all()).toHaveLength(expectedLists); + }), + ); }); test('nested items stay nested when a regular list is followed by a checklist', async () => { From 7f4bf26170a10116e1bf7834e402998dc3101b7d Mon Sep 17 00:00:00 2001 From: Robert Vitonsky Date: Sat, 10 Oct 2026 23:41:40 +0300 Subject: [PATCH 3/5] refactor: split list on lexical transformer side --- .../__tests__/spec/list.browser.test.ts | 32 +++++++++++ .../plugins/Markdown/markdownParser.ts | 37 +++++++----- .../remark/remarkPreserveBlankLines.ts | 57 +------------------ 3 files changed, 58 insertions(+), 68 deletions(-) diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts index 55c24c622..f13a54a40 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts @@ -188,4 +188,36 @@ describe('Regressions', () => { expect(page.getByRole('list').all()).toHaveLength(1); }); + + test('list kind changes split items even without a blank line', async () => { + await renderRichEditorInDOM({ + value: '- regular\n- [ ] task\n- regular again', + }); + + expect(page.getByRole('list').all()).toHaveLength(3); + expect(page.getByRole('checkbox')).toHaveLength(1); + }); + + test('checked and unchecked task items remain in the same checklist', async () => { + await renderRichEditorInDOM({ + value: '- [ ] first\n\n- [x] second\n - [ ] third', + }); + + expect(page.getByRole('list').all()).toHaveLength(1); + expect(page.getByRole('checkbox')).toHaveLength(3); + }); + + test('ordered list numbering is retained after a task item splits the list', async () => { + await renderRichEditorInDOM({ + value: '3. regular\n4. [ ] task\n5. regular again', + }); + + const editor = page.getByRole('textbox'); + const orderedLists = editor.element().querySelectorAll(':scope > ol'); + + expect(orderedLists).toHaveLength(2); + expect(orderedLists[0].getAttribute('start')).toBe('3'); + expect(orderedLists[1].getAttribute('start')).toBe('5'); + expect(editor.getByRole('checkbox')).toHaveLength(1); + }); }); diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts index 81568997a..9dc2b190a 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts @@ -153,21 +153,32 @@ export const $convertFromMarkdownString = (rawMarkdown: string) => { return [heading]; } case 'list': { - let listType: ListType = 'bullet'; - if ( - node.children.some( - (item) => item.checked !== undefined && item.checked !== null, - ) - ) { - listType = 'check'; - } else if (node.ordered || typeof node.start === 'number') { - listType = 'number'; + const defaultListType: ListType = + node.ordered || typeof node.start === 'number' ? 'number' : 'bullet'; + const convertedLists: LexicalNode[] = []; + + let currentList: ReturnType | undefined; + let currentListType: ListType | undefined; + for (let i = 0; i < node.children.length; i++) { + const item = node.children[i]; + const itemListType: ListType = + item.checked !== undefined && item.checked !== null + ? 'check' + : defaultListType; + + if (!currentList || currentListType !== itemListType) { + currentListType = itemListType; + currentList = $createListNode( + itemListType, + itemListType === 'number' ? (node.start ?? 1) + i : 1, + ); + convertedLists.push(currentList); + } + + currentList.append(...convertToMarkdownNode(item)); } - const list = $createListNode(listType); - list.append(...convertToMarkdownNodes(node.children)); - - return [list]; + return convertedLists; } case 'listItem': { const listItem = $createListItemNode(node.checked ?? undefined); diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts index 1687c98a4..c6ddd8c0f 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts @@ -1,4 +1,4 @@ -import { type List, type ListItem, type Root, RootContent } from 'mdast'; +import { type Root, RootContent } from 'mdast'; import { Plugin } from 'unified'; import { CONTINUE, SKIP, visit } from 'unist-util-visit'; @@ -8,55 +8,6 @@ const ignoredNodeTypes = new Set([ 'tableRow', ] satisfies RootContent['type'][]); -const isTaskListItem = (item: ListItem) => - item.checked !== null && item.checked !== undefined; - -// Lexical represents task lists and regular lists as different list types, but -// mdast can keep both kinds of item in one list when there is a blank line. -const splitListAtTaskBoundary = (list: List): List[] => { - const groups: { items: ListItem[]; startIndex: number }[] = []; - let items: ListItem[] = []; - let startIndex = 0; - - for (let i = 0; i < list.children.length; i++) { - const item = list.children[i]; - const previousItem = items.at(-1); - const hasBlankLineBefore = - previousItem?.position && - item.position && - item.position.start.line > previousItem.position.end.line + 1; - const changesTaskStatus = - previousItem && isTaskListItem(previousItem) !== isTaskListItem(item); - - if (hasBlankLineBefore && changesTaskStatus) { - groups.push({ items, startIndex }); - items = []; - startIndex = i; - } - - items.push(item); - } - - if (items.length > 0) groups.push({ items, startIndex }); - if (groups.length <= 1) return [list]; - - return groups.map(({ items: groupItems, startIndex: groupStartIndex }) => { - const firstItem = groupItems[0]; - const lastItem = groupItems[groupItems.length - 1]; - const position = - list.position && firstItem.position && lastItem.position - ? { start: firstItem.position.start, end: lastItem.position.end } - : list.position; - - return { - ...list, - ...(list.ordered ? { start: (list.start ?? 1) + groupStartIndex } : {}), - children: groupItems, - position, - }; - }); -}; - export const fillGapsWithParagraphs = (tree: Root) => { const skipNodes = new Set(); @@ -76,11 +27,7 @@ export const fillGapsWithParagraphs = (tree: Root) => { const next = node.children[i + 1]; // Collect its own children - newChildren.push( - ...(current.type === 'list' - ? splitListAtTaskBoundary(current) - : [current]), - ); + newChildren.push(current); // Add empty lines to preserve if (next && current.position && next.position) { From 428b4a52acd57abc2233762095d1ca5d7be2d27a Mon Sep 17 00:00:00 2001 From: Robert Vitonsky Date: Sun, 11 Oct 2026 00:49:32 +0300 Subject: [PATCH 4/5] test: roundtrip cases --- .../__tests__/spec/list.browser.test.ts | 24 ++++++++++++++++--- .../__tests__/utils/renderEditorInDOM.tsx | 22 ++++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts index f13a54a40..010d29acf 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts @@ -1,5 +1,5 @@ import { act } from 'react'; -import { page, userEvent } from 'vitest/browser'; +import { page, userEvent, utils } from 'vitest/browser'; import { renderRichEditorInDOM } from '../utils/renderEditorInDOM'; import { selectContent } from '../utils/utils'; @@ -153,11 +153,29 @@ describe('Regressions', () => { }, ].forEach(({ name, content, expectedLists }) => test(name, async () => { - await renderRichEditorInDOM({ + const { render, getMarkdown, getEditor } = await renderRichEditorInDOM({ value: content, }); - expect(page.getByRole('list').all()).toHaveLength(expectedLists); + expect( + page.getByRole('list').all(), + 'There are expected number of lists', + ).toHaveLength(expectedLists); + + const rootNode = getEditor().getRootElement(); + const getHTML = () => utils.prettyDOM(rootNode); + + const htmlV1 = getHTML(); + const markdownV1 = getMarkdown(); + + // Cleanup and re-render + await render({ value: '' }); + await expect.poll(() => rootNode).toHaveTextContent(''); + + // Make sure the roundtrip are consistent + await render({ value: markdownV1 }); + expect(getHTML()).toBe(htmlV1); + expect(getMarkdown()).toBe(markdownV1); }), ); }); diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/utils/renderEditorInDOM.tsx b/packages/app/src/features/NoteEditor/RichEditor/__tests__/utils/renderEditorInDOM.tsx index 36c2f7842..9619aea3b 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/utils/renderEditorInDOM.tsx +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/utils/renderEditorInDOM.tsx @@ -22,6 +22,8 @@ import { RichEditorContentProps } from '@features/NoteEditor/RichEditor/RichEdit import { ThemeProvider } from '@features/ThemeProvider'; import { createTestStore } from '@tests/utils/redux'; +import { $convertToMarkdownString } from '../../plugins/Markdown/markdownParser'; + // Mock useUrlOpener to avoid importing monaco-editor-core in tests, // which causes Vite module resolution errors during test setup vi.mock('@hooks/useUrlOpener', () => ({ @@ -73,6 +75,8 @@ export type RichEditorTestAPI = { format: (format: TextFormat) => Promise; command: (format: CommandsPayload) => Promise; getEditor(): LexicalEditor; + getMarkdown(): string; + render: (props: RichEditorContentProps) => Promise; }; export const renderRichEditorInDOM = async ({ @@ -114,7 +118,14 @@ export const renderRichEditorInDOM = async ({ document.body.appendChild(container); const root = createRoot(container); - act(() => root.render(renderEditor({ ...props, editorRef }))); + + const render = (props: RichEditorContentProps) => { + return act(() => + root.render(renderEditor({ ...props, editorRef })), + ) as unknown as Promise; + }; + + render(props); const destroy = () => { act(() => { @@ -163,5 +174,14 @@ export const renderRichEditorInDOM = async ({ if (!editor) throw new Error('Error instance is not set'); return editor; }, + + getMarkdown() { + const editor = editorRef.current; + if (!editor) throw new Error('Error instance is not set'); + + return editor.read(() => $convertToMarkdownString()); + }, + + render, }; }; From 895e583c3322fc0176c5d4c0dc6c27510f35742f Mon Sep 17 00:00:00 2001 From: Robert Vitonsky Date: Sun, 11 Oct 2026 23:26:15 +0300 Subject: [PATCH 5/5] fix: ensure separate list markers --- .../__tests__/spec/list.browser.test.ts | 42 ++++++++++ .../__tests__/list-boundaries.browser.test.ts | 81 ++++++++++++++++++ .../plugins/Markdown/markdownParser.ts | 39 +++++++++ .../remark/remarkPreserveBlankLines.test.ts | 13 +++ .../remark/remarkPreserveBlankLines.ts | 4 + .../Markdown/serializeMarkdownTree.test.ts | 83 ++++++++++++++++++- 6 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/__tests__/list-boundaries.browser.test.ts diff --git a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts index 010d29acf..0f2a54f60 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/__tests__/spec/list.browser.test.ts @@ -180,6 +180,48 @@ describe('Regressions', () => { ); }); + test('3 empty lines splits the lists', async () => { + const editor1 = await renderRichEditorInDOM({ + value: '', + }); + + const editor1Locator = page + .elementLocator(editor1.getEditor().getRootElement()!) + .first(); + + // Create 2 lists + await act(async () => { + await editor1Locator.click(); + }); + await editor1.insert({ type: 'list', data: { type: 'unordered' } }); + await act(async () => { + await userEvent.keyboard('foo'); + await userEvent.keyboard('{Enter}'.repeat(3)); + }); + + await editor1.insert({ type: 'list', data: { type: 'unordered' } }); + await act(async () => { + await userEvent.keyboard('bar'); + }); + + expect(editor1Locator.getByRole('list').all()).toHaveLength(2); + expect(editor1Locator.getByRole('listitem').all()).toHaveLength(2); + + // Create another editor with serialized content + const editor2 = await renderRichEditorInDOM({ + value: editor1.getMarkdown(), + }); + + const editor2Locator = page.elementLocator(editor2.getEditor().getRootElement()!); + expect(editor2Locator.getByRole('list').all()).toHaveLength(2); + expect(editor2Locator.getByRole('listitem').all()).toHaveLength(2); + + // Content must be equal + expect(editor1Locator.element()).not.toBe(editor2Locator.element()); + expect(utils.prettyDOM(editor2Locator)).toBe(utils.prettyDOM(editor1Locator)); + expect(editor2.getMarkdown()).toBe(editor1.getMarkdown()); + }); + test('nested items stay nested when a regular list is followed by a checklist', async () => { const value = ` - foo diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/__tests__/list-boundaries.browser.test.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/__tests__/list-boundaries.browser.test.ts new file mode 100644 index 000000000..c0b660408 --- /dev/null +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/__tests__/list-boundaries.browser.test.ts @@ -0,0 +1,81 @@ +import { page } from 'vitest/browser'; +import { renderRichEditorInDOM } from '@features/NoteEditor/RichEditor/__tests__/utils/renderEditorInDOM'; + +test('same-kind lists separated by a large gap retain their boundaries and empty items', async () => { + const source = [ + '- first', + ...Array(10).fill('-'), + ...Array(100).fill(''), + '* second', + ...Array(10).fill('*'), + ].join('\n'); + const { getEditor, getMarkdown, render } = await renderRichEditorInDOM({ + value: source, + }); + + const editor = page.getByRole('textbox'); + const getListItemCounts = () => + Array.from( + getEditor().getRootElement()?.querySelectorAll(':scope > ul') ?? [], + ).map((list) => list.querySelectorAll(':scope > li').length); + const getGapParagraphCount = () => + getEditor().getRootElement()?.querySelectorAll(':scope > p').length ?? 0; + + expect(getListItemCounts()).toEqual([11, 11]); + expect(editor.getByRole('listitem')).toHaveLength(22); + expect(getGapParagraphCount()).toBe(98); + expect(editor.getByRole('listitem').nth(1)).toHaveTextContent(''); + expect(editor.getByRole('listitem').nth(12)).toHaveTextContent(''); + + const serialized = getMarkdown(); + expect(serialized).toContain('- first'); + expect(serialized).toContain('* second'); + + await render({ value: serialized }); + + expect(getListItemCounts()).toEqual([11, 11]); + expect(page.getByRole('listitem')).toHaveLength(22); + expect(getGapParagraphCount()).toBe(98); + expect(getMarkdown()).toBe(serialized); +}); + +test('a single empty editor paragraph keeps same-kind lists separate after round-trip', async () => { + const { getEditor, getMarkdown, render } = await renderRichEditorInDOM({ + value: '- first\n\n\n\n* second', + }); + const root = () => getEditor().getRootElement()!; + const getTopLevelLists = () => root().querySelectorAll(':scope > ul'); + const getTopLevelParagraphs = () => root().querySelectorAll(':scope > p'); + + expect(getTopLevelLists()).toHaveLength(2); + expect(getTopLevelParagraphs()).toHaveLength(1); + + const serialized = getMarkdown(); + expect(serialized).toContain('- first'); + expect(serialized).toContain('* second'); + + await render({ value: serialized }); + + expect(getTopLevelLists()).toHaveLength(2); + expect(getTopLevelParagraphs()).toHaveLength(1); + expect(getMarkdown()).toBe(serialized); +}); + +test('a list with many empty items remains one list', async () => { + const source = ['- first', ...Array(100).fill('-'), '- last'].join('\n'); + const { getEditor, getMarkdown, render } = await renderRichEditorInDOM({ + value: source, + }); + + const getListItemCount = () => + getEditor().getRootElement()?.querySelectorAll(':scope > ul > li').length; + expect(getEditor().getRootElement()?.querySelectorAll(':scope > ul')).toHaveLength(1); + expect(getListItemCount()).toBe(102); + + const serialized = getMarkdown(); + await render({ value: serialized }); + + expect(getEditor().getRootElement()?.querySelectorAll(':scope > ul')).toHaveLength(1); + expect(getListItemCount()).toBe(102); + expect(getMarkdown()).toBe(serialized); +}); diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts index 9dc2b190a..e42d9ac7b 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/markdownParser.ts @@ -36,13 +36,52 @@ import { $createRawNode } from './nodes/RawNode'; import { liftFormattingNodes } from './remark/remarkLiftFormatting'; import { fillGapsWithParagraphs } from './remark/remarkPreserveBlankLines'; +const emittedListMarkers = new WeakMap>(); + export const markdownProcessor = unified() .use(remarkParse) .use(remarkGfm) .use(remarkStringify, { bullet: '-', + bulletOther: '*', + bulletOrdered: '.', listItemIndent: 'one', handlers: { + list(node, parent, state, info) { + let listMarkers = emittedListMarkers.get(state); + if (!listMarkers) { + listMarkers = new WeakMap(); + emittedListMarkers.set(state, listMarkers); + } + + if (parent) { + const currentIndex = state.indexStack[state.indexStack.length - 1]; + + for (let i = (currentIndex ?? -1) - 1; i >= 0; i--) { + const sibling = parent.children[i]; + + if ( + sibling.type === 'paragraph' && + sibling.children.length === 0 + ) { + continue; + } + + if (sibling.type === 'list' && sibling.ordered === node.ordered) { + const previousMarker = listMarkers.get(sibling); + if (previousMarker) state.bulletLastUsed = previousMarker; + } + + break; + } + } + + const markdown = defaultHandlers.list(node, parent, state, info); + const marker = state.bulletLastUsed; + if (marker) listMarkers.set(node, marker); + + return markdown; + }, paragraph(node, parent, state, info) { // Empty line will be added after that node anyway // So empty paragraph converts to 1 empty line diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.test.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.test.ts index 2465ab56c..441c686a2 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.test.ts @@ -1,3 +1,4 @@ +import { List } from 'mdast'; import remarkParse from 'remark-parse'; import { unified } from 'unified'; @@ -29,6 +30,18 @@ test('Must add 8 paragraphs for gap in 10 lines', () => { expect(fillGapsWithParagraphs(tree).children).toHaveLength(2 + 8); }); +test('Must not insert paragraphs between list items for large source gaps', () => { + const tree = markdownProcessor.parse('- First\n' + '\n'.repeat(10) + '- Second'); + + const list = fillGapsWithParagraphs(tree).children[0] as List; + expect(list.type).toBe('list'); + + expect(list.children).toHaveLength(2); + expect(list.children).toStrictEqual( + Array(2).fill(expect.objectContaining({ type: 'listItem' })), + ); +}); + // TODO: implement lines preservation test.fails('Preserve lines for document with no text', () => { const tree = markdownProcessor.parse('\n'.repeat(10)); diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts index c6ddd8c0f..4b0be8e0a 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/remark/remarkPreserveBlankLines.ts @@ -21,6 +21,10 @@ export const fillGapsWithParagraphs = (tree: Root) => { // Skip already handled nodes if (skipNodes.has(node)) return SKIP; + // A list may only contain list items. Preserve item-to-item source gaps + // as list looseness, rather than inserting paragraphs that become items. + if (node.type === 'list') return CONTINUE; + const newChildren: RootContent[] = []; for (let i = 0; i < node.children.length; i++) { const current = node.children[i]; diff --git a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/serializeMarkdownTree.test.ts b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/serializeMarkdownTree.test.ts index 387483c03..d09da488f 100644 --- a/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/serializeMarkdownTree.test.ts +++ b/packages/app/src/features/NoteEditor/RichEditor/plugins/Markdown/serializeMarkdownTree.test.ts @@ -1,7 +1,25 @@ import { Paragraph } from 'mdast'; import { u } from 'unist-builder'; -import { serializeMarkdownTree } from './markdownParser'; +import { parseMarkdownToAST, serializeMarkdownTree } from './markdownParser'; + +const createBulletList = (value: string) => + u('list', { + ordered: false, + start: null, + spread: false, + children: [ + u('listItem', { + checked: null, + spread: false, + children: [ + u('paragraph', { + children: [u('text', { value })], + }), + ], + }), + ], + }); describe('AST serialization', () => { test('2 paragraphs have 1 empty line', () => { @@ -47,6 +65,69 @@ describe('AST serialization', () => { ).toBe(`Hello${'\n'.repeat(5)}\nWorld\n`); }); + test('Same-kind lists separated by empty paragraphs use distinct markers', () => { + const output = serializeMarkdownTree({ + type: 'root', + children: [ + createBulletList('First'), + u('paragraph', { children: [] }), + createBulletList('Second'), + u('paragraph', { children: [] }), + u('paragraph', { children: [] }), + createBulletList('Third'), + ], + }); + + const listItemMarkers = output + .split('\n') + .filter( + (line) => + line.endsWith('First') || + line.endsWith('Second') || + line.endsWith('Third'), + ) + .map((line) => line[0]); + + expect(listItemMarkers).toEqual(['-', '*', '-']); + expect( + parseMarkdownToAST(output).children.filter((node) => node.type === 'list'), + ).toHaveLength(3); + }); + + test('Ordered lists separated by empty paragraphs use distinct delimiters', () => { + const createOrderedList = (start: number, value: string) => + u('list', { + ordered: true, + start, + spread: false, + children: [ + u('listItem', { + checked: null, + spread: false, + children: [ + u('paragraph', { + children: [u('text', { value })], + }), + ], + }), + ], + }); + const output = serializeMarkdownTree({ + type: 'root', + children: [ + createOrderedList(3, 'First'), + u('paragraph', { children: [] }), + createOrderedList(4, 'Second'), + ], + }); + + expect(output).toContain('3. First'); + expect(output).toContain('4) Second'); + expect( + parseMarkdownToAST(output).children.filter((node) => node.type === 'list'), + ).toHaveLength(2); + }); + test('Quote with 3 empty paragraphs must contain 5 empty lines', () => { expect( serializeMarkdownTree({