From af57830f3c06fccb7ac1f1bb070e05771d0ff59d Mon Sep 17 00:00:00 2001 From: Manav Agarwal <dpsman13016@gmail.com> Date: Mon, 13 Nov 2023 13:13:26 +0000 Subject: [PATCH] Fix [ ] in plain text editor incorrectly expanded to * [ ] in RTE passed bullet attribute in task_list and added tests Changelog: changed --- .../content_editor/extensions/task_list.js | 7 ++ .../services/markdown_sourcemap.js | 6 +- .../output_example_snapshots/html.yml | 10 +- .../prosemirror_json.yml | 15 ++- .../services/markdown_serializer_spec.js | 28 +++++ .../services/markdown_sourcemap_spec.js | 109 +++++++++++++----- 6 files changed, 135 insertions(+), 40 deletions(-) diff --git a/app/assets/javascripts/content_editor/extensions/task_list.js b/app/assets/javascripts/content_editor/extensions/task_list.js index 01e5bddb97a27..5ef9cf42f931f 100644 --- a/app/assets/javascripts/content_editor/extensions/task_list.js +++ b/app/assets/javascripts/content_editor/extensions/task_list.js @@ -27,6 +27,13 @@ export default TaskList.extend({ default: false, parseHTML: (element) => /^[0-9]+\)/.test(getMarkdownSource(element)), }, + bullet: { + default: '*', + parseHTML(element) { + const bullet = getMarkdownSource(element)?.charAt(0); + return '*+-'.includes(bullet) ? bullet : '*'; + }, + }, }; }, diff --git a/app/assets/javascripts/content_editor/services/markdown_sourcemap.js b/app/assets/javascripts/content_editor/services/markdown_sourcemap.js index 11a11ed43bd09..a4abb8dcf38f3 100644 --- a/app/assets/javascripts/content_editor/services/markdown_sourcemap.js +++ b/app/assets/javascripts/content_editor/services/markdown_sourcemap.js @@ -16,8 +16,8 @@ const getRangeFromSourcePos = (sourcePos) => { const [endRow, endCol] = end.split(':'); return { - start: { row: Number(startRow) - 1, col: Number(startCol) - 1 }, - end: { row: Number(endRow) - 1, col: Number(endCol) - 1 }, + start: { row: Math.max(0, Number(startRow) - 1), col: Math.max(0, Number(startCol) - 1) }, + end: { row: Math.max(0, Number(endRow) - 1), col: Math.max(0, Number(endCol) - 1) }, }; }; @@ -33,8 +33,6 @@ export const getMarkdownSource = (element) => { for (let i = range.start.row; i <= range.end.row; i += 1) { if (i === range.start.row) { elSource += source[i].substring(range.start.col); - } else if (i === range.end.row) { - elSource += `\n${source[i]?.substring(0, range.start.col)}`; } else { elSource += `\n${source[i]}` || ''; } diff --git a/glfm_specification/output_example_snapshots/html.yml b/glfm_specification/output_example_snapshots/html.yml index ab98ee36a1d2e..ca51c26a760bf 100644 --- a/glfm_specification/output_example_snapshots/html.yml +++ b/glfm_specification/output_example_snapshots/html.yml @@ -7639,7 +7639,7 @@ <task-button></task-button><input type="checkbox" class="task-list-item-checkbox" disabled> incomplete</li> </ul> wysiwyg: |- - <ul dir="auto" start="1" parens="false" data-type="taskList"><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">incomplete</p></div></li></ul> + <ul dir="auto" start="1" parens="false" bullet="*" data-type="taskList"><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">incomplete</p></div></li></ul> 07_01_00__gitlab_official_specification_markdown__task_list_items__002: canonical: | <ul> @@ -7655,7 +7655,7 @@ <task-button></task-button><input type="checkbox" class="task-list-item-checkbox" checked disabled> completed</li> </ul> wysiwyg: |- - <ul dir="auto" start="1" parens="false" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">completed</p></div></li></ul> + <ul dir="auto" start="1" parens="false" bullet="*" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">completed</p></div></li></ul> 07_01_00__gitlab_official_specification_markdown__task_list_items__003: canonical: | <ul> @@ -7986,7 +7986,7 @@ wysiwyg: |- <ul dir="auto" bullet="*"><li dir="auto"><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/1.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/1.mp3" class="with-attachment-icon">Sample Audio</a></span></p></li><li dir="auto"><p dir="auto"><span class="media-container video-container"><video src="https://gitlab.com/2.mp4" controls="true" data-setup="{}" data-title="Sample Video"></video><a href="https://gitlab.com/2.mp4" class="with-attachment-icon">Sample Video</a></span></p></li></ul> <ol dir="auto" parens="false"><li dir="auto"><p dir="auto"><span class="media-container video-container"><video src="https://gitlab.com/1.mp4" controls="true" data-setup="{}" data-title="Sample Video"></video><a href="https://gitlab.com/1.mp4" class="with-attachment-icon">Sample Video</a></span></p></li><li dir="auto"><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/2.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/2.mp3" class="with-attachment-icon">Sample Audio</a></span></p></li></ol> - <ul dir="auto" start="1" parens="false" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/1.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/1.mp3" class="with-attachment-icon">Sample Audio</a></span></p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/2.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/2.mp3" class="with-attachment-icon">Sample Audio</a></span></p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container video-container"><video src="https://gitlab.com/3.mp4" controls="true" data-setup="{}" data-title="Sample Video"></video><a href="https://gitlab.com/3.mp4" class="with-attachment-icon">Sample Video</a></span></p></div></li></ul> + <ul dir="auto" start="1" parens="false" bullet="*" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/1.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/1.mp3" class="with-attachment-icon">Sample Audio</a></span></p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container audio-container"><audio src="https://gitlab.com/2.mp3" controls="true" data-setup="{}" data-title="Sample Audio"></audio><a href="https://gitlab.com/2.mp3" class="with-attachment-icon">Sample Audio</a></span></p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto"><span class="media-container video-container"><video src="https://gitlab.com/3.mp4" controls="true" data-setup="{}" data-title="Sample Video"></video><a href="https://gitlab.com/3.mp4" class="with-attachment-icon">Sample Video</a></span></p></div></li></ul> 08_04_10__gitlab_internal_extension_markdown__migrated_golden_master_examples__blockquote__001: canonical: | TODO: Write canonical HTML for this example @@ -8477,7 +8477,7 @@ <task-button></task-button><input type="checkbox" class="task-list-item-checkbox" disabled> example</li> </ol> wysiwyg: |- - <ol dir="auto" start="1" parens="false" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">hello</p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">world</p></div></li><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">example</p></div></li></ol> + <ol dir="auto" start="1" parens="false" bullet="*" data-type="taskList"><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">hello</p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">world</p></div></li><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">example</p></div></li></ol> 08_04_46__gitlab_internal_extension_markdown__migrated_golden_master_examples__reference_for_project_wiki__001: canonical: | TODO: Write canonical HTML for this example @@ -8828,7 +8828,7 @@ <task-button></task-button><input type="checkbox" class="task-list-item-checkbox" checked disabled> bar</li> </ul> wysiwyg: |- - <ul dir="auto" start="1" parens="false" data-type="taskList"><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">foo</p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">bar</p></div></li></ul> + <ul dir="auto" start="1" parens="false" bullet="*" data-type="taskList"><li dir="auto" data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label><div><p dir="auto">foo</p></div></li><li dir="auto" data-checked="true" data-type="taskItem"><label><input type="checkbox" checked="checked"><span></span></label><div><p dir="auto">bar</p></div></li></ul> 09_04_00__gfm_undocumented_extensions_and_more_robust_test__task_lists__002: canonical: | <ul> diff --git a/glfm_specification/output_example_snapshots/prosemirror_json.yml b/glfm_specification/output_example_snapshots/prosemirror_json.yml index 00c805b6928c4..95e7003a202af 100644 --- a/glfm_specification/output_example_snapshots/prosemirror_json.yml +++ b/glfm_specification/output_example_snapshots/prosemirror_json.yml @@ -20555,7 +20555,8 @@ "attrs": { "numeric": false, "start": 1, - "parens": false + "parens": false, + "bullet": "*" }, "content": [ { @@ -20588,7 +20589,8 @@ "attrs": { "numeric": false, "start": 1, - "parens": false + "parens": false, + "bullet": "*" }, "content": [ { @@ -21261,7 +21263,8 @@ "attrs": { "numeric": false, "start": 1, - "parens": false + "parens": false, + "bullet": "*" }, "content": [ { @@ -22996,7 +22999,8 @@ "attrs": { "numeric": true, "start": 1, - "parens": false + "parens": false, + "bullet": "*" }, "content": [ { @@ -23902,7 +23906,8 @@ "attrs": { "numeric": false, "start": 1, - "parens": false + "parens": false, + "bullet": "*" }, "content": [ { diff --git a/spec/frontend/content_editor/services/markdown_serializer_spec.js b/spec/frontend/content_editor/services/markdown_serializer_spec.js index 548c6030ed729..b506572c183f4 100644 --- a/spec/frontend/content_editor/services/markdown_serializer_spec.js +++ b/spec/frontend/content_editor/services/markdown_serializer_spec.js @@ -607,6 +607,34 @@ this is not really json:table but just trying out whether this case works or not ); }); + it('correctly serializes bullet task list with different bullet styles', () => { + expect( + serialize( + taskList( + { bullet: '+' }, + taskItem({ checked: true }, paragraph('list item 1')), + taskItem(paragraph('list item 2')), + taskItem( + paragraph('list item 3'), + taskList( + { bullet: '-' }, + taskItem({ checked: true }, paragraph('sub-list item 1')), + taskItem(paragraph('sub-list item 2')), + ), + ), + ), + ), + ).toBe( + ` ++ [x] list item 1 ++ [ ] list item 2 ++ [ ] list item 3 + - [x] sub-list item 1 + - [ ] sub-list item 2 + `.trim(), + ); + }); + it('correctly serializes a numeric list', () => { expect( serialize( diff --git a/spec/frontend/content_editor/services/markdown_sourcemap_spec.js b/spec/frontend/content_editor/services/markdown_sourcemap_spec.js index 2efc73ddef867..4428fa682e76f 100644 --- a/spec/frontend/content_editor/services/markdown_sourcemap_spec.js +++ b/spec/frontend/content_editor/services/markdown_sourcemap_spec.js @@ -1,6 +1,8 @@ import { Extension } from '@tiptap/core'; import BulletList from '~/content_editor/extensions/bullet_list'; import ListItem from '~/content_editor/extensions/list_item'; +import TaskList from '~/content_editor/extensions/task_list'; +import TaskItem from '~/content_editor/extensions/task_item'; import Paragraph from '~/content_editor/extensions/paragraph'; import markdownDeserializer from '~/content_editor/services/gl_api_markdown_deserializer'; import { getMarkdownSource, getFullSource } from '~/content_editor/services/markdown_sourcemap'; @@ -18,6 +20,20 @@ const BULLET_LIST_HTML = `<ul data-sourcepos="1:1-3:24" dir="auto"> </li> </ul>`; +const BULLET_TASK_LIST_MARKDOWN = `- [ ] list item 1 ++ [x] checked list item 2 + + [ ] embedded list item 1 + - [x] checked embedded list item 2`; +const BULLET_TASK_LIST_HTML = `<ul data-sourcepos="1:1-4:36" class="task-list" dir="auto"> + <li data-sourcepos="1:1-1:17" class="task-list-item"><input type="checkbox" class="task-list-item-checkbox"> list item 1</li> + <li data-sourcepos="2:1-4:36" class="task-list-item"><input type="checkbox" class="task-list-item-checkbox" checked> checked list item 2 + <ul data-sourcepos="3:3-4:36" class="task-list"> + <li data-sourcepos="3:3-3:28" class="task-list-item"><input type="checkbox" class="task-list-item-checkbox"> embedded list item 1</li> + <li data-sourcepos="4:3-4:36" class="task-list-item"><input type="checkbox" class="task-list-item-checkbox" checked> checked embedded list item 2</li> + </ul> + </li> +</ul>`; + const SourcemapExtension = Extension.create({ // lets add `source` attribute to every element using `getMarkdownSource` addGlobalAttributes() { @@ -38,19 +54,68 @@ const SourcemapExtension = Extension.create({ }); const tiptapEditor = createTestEditor({ - extensions: [BulletList, ListItem, SourcemapExtension], + extensions: [BulletList, ListItem, TaskList, TaskItem, SourcemapExtension], }); const { - builders: { doc, bulletList, listItem, paragraph }, + builders: { doc, bulletList, listItem, taskList, taskItem, paragraph }, } = createDocBuilder({ tiptapEditor, names: { bulletList: { nodeType: BulletList.name }, listItem: { nodeType: ListItem.name }, + taskList: { nodeType: TaskList.name }, + taskItem: { nodeType: TaskItem.name }, }, }); +const bulletListDoc = () => + doc( + bulletList( + { bullet: '+', source: '+ list item 1\n+ list item 2\n - embedded list item 3' }, + listItem({ source: '+ list item 1' }, paragraph('list item 1')), + listItem( + { source: '+ list item 2\n - embedded list item 3' }, + paragraph('list item 2'), + bulletList( + { bullet: '-', source: '- embedded list item 3' }, + listItem({ source: '- embedded list item 3' }, paragraph('embedded list item 3')), + ), + ), + ), + ); + +const bulletTaskListDoc = () => + doc( + taskList( + { + bullet: '-', + source: + '- [ ] list item 1\n+ [x] checked list item 2\n + [ ] embedded list item 1\n - [x] checked embedded list item 2', + }, + taskItem({ source: '- [ ] list item 1' }, paragraph('list item 1')), + taskItem( + { + source: + '+ [x] checked list item 2\n + [ ] embedded list item 1\n - [x] checked embedded list item 2', + checked: true, + }, + paragraph('checked list item 2'), + taskList( + { + bullet: '+', + source: '+ [ ] embedded list item 1\n - [x] checked embedded list item 2', + }, + taskItem({ source: '+ [ ] embedded list item 1' }, paragraph('embedded list item 1')), + taskItem( + { source: '- [x] checked embedded list item 2', checked: true }, + paragraph('checked embedded list item 2'), + ), + ), + ), + ), + ); + describe('content_editor/services/markdown_sourcemap', () => { describe('getFullSource', () => { it.each` @@ -72,29 +137,21 @@ describe('content_editor/services/markdown_sourcemap', () => { }); }); - it('gets markdown source for a rendered HTML element', async () => { - const { document } = await markdownDeserializer({ - render: () => BULLET_LIST_HTML, - }).deserialize({ - schema: tiptapEditor.schema, - markdown: BULLET_LIST_MARKDOWN, - }); - - const expected = doc( - bulletList( - { bullet: '+', source: '+ list item 1\n+ list item 2' }, - listItem({ source: '+ list item 1' }, paragraph('list item 1')), - listItem( - { source: '+ list item 2' }, - paragraph('list item 2'), - bulletList( - { bullet: '-', source: '- embedded list item 3' }, - listItem({ source: '- embedded list item 3' }, paragraph('embedded list item 3')), - ), - ), - ), - ); + it.each` + description | sourceMarkdown | sourceHTML | expectedDoc + ${'bullet list'} | ${BULLET_LIST_MARKDOWN} | ${BULLET_LIST_HTML} | ${bulletListDoc} + ${'bullet task list'} | ${BULLET_TASK_LIST_MARKDOWN} | ${BULLET_TASK_LIST_HTML} | ${bulletTaskListDoc} + `( + 'gets markdown source for a rendered $description', + async ({ sourceMarkdown, sourceHTML, expectedDoc }) => { + const { document } = await markdownDeserializer({ + render: () => sourceHTML, + }).deserialize({ + schema: tiptapEditor.schema, + markdown: sourceMarkdown, + }); - expect(document.toJSON()).toEqual(expected.toJSON()); - }); + expect(document.toJSON()).toEqual(expectedDoc().toJSON()); + }, + ); }); -- GitLab