From ac211f01219e1ec0b8206e6194840c22cdfa788e Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Wed, 5 Jul 2023 22:15:07 -0700 Subject: [PATCH 1/7] Fix bug with legacy task list position calculation --- src/drafts/MarkdownViewer/_useListInteraction.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/drafts/MarkdownViewer/_useListInteraction.ts b/src/drafts/MarkdownViewer/_useListInteraction.ts index 1ce972970a1..8e47464b65c 100644 --- a/src/drafts/MarkdownViewer/_useListInteraction.ts +++ b/src/drafts/MarkdownViewer/_useListInteraction.ts @@ -4,6 +4,8 @@ import {ListItem, listItemToString, parseListItem} from '../MarkdownEditor/_useL type TaskListItem = ListItem & {taskBox: '[ ]' | '[x]'} +const isCodeBlockDelimiter = (line: string) => line.trimStart().startsWith('```') + const isTaskListItem = (item: ListItem | null): item is TaskListItem => typeof item?.taskBox === 'string' const toggleTaskListItem = (item: TaskListItem): TaskListItem => ({ @@ -43,11 +45,17 @@ export const useListInteraction = ({ const onToggleItem = useCallback( (toggledItemIndex: number) => () => { const lines = markdownRef.current.split('\n') + let inCodeBlock = false for (let lineIndex = 0, taskIndex = 0; lineIndex < lines.length; lineIndex++) { + if (isCodeBlockDelimiter(lines[lineIndex])) { + inCodeBlock = !inCodeBlock + continue + } + const parsedLine = parseListItem(lines[lineIndex]) - if (!isTaskListItem(parsedLine)) continue + if (!isTaskListItem(parsedLine) || inCodeBlock) continue if (taskIndex === toggledItemIndex) { const updatedLine = listItemToString(toggleTaskListItem(parsedLine)) From b456e32ae4a80125bffef9c4cca0cef8a4b866e5 Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Thu, 6 Jul 2023 09:45:44 -0700 Subject: [PATCH 2/7] Add test for hierarchy task list combination --- .../MarkdownViewer/MarkdownViewer.test.tsx | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx index 14d4e393a79..298c4eb5825 100644 --- a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx +++ b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx @@ -27,6 +27,30 @@ text before list - [x] item 1 - [ ] item 2 +text after list` + const hierarchyBeforeTaskListNoItemsChecked = ` +text before list + +\`\`\`[tasklist] +- [ ] item A +- [ ] item B +\`\`\` + +- [ ] item 1 +- [ ] item 2 + +text after list` + const hierarchyBeforeTaskListOneItemChecked = ` +text before list + +\`\`\`[tasklist] +- [ ] item A +- [ ] item B +\`\`\` + +- [x] item 1 +- [ ] item 2 + text after list` it('enables checklists by default', () => { @@ -75,6 +99,21 @@ text after list` await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(firstItemCheckedMarkdown)) }) + it('calls `onChange` with the updated Markdown when a task is checked and hierarchy is present', async () => { + const onChangeMock = jest.fn() + const {getAllByRole} = render( + + ) + const items = getAllByRole('checkbox') + fireEvent.change(items[0]) + await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(hierarchyBeforeTaskListOneItemChecked)) + }) + it('calls `onChange` with the updated Markdown when a task is unchecked', async () => { const onChangeMock = jest.fn() const {getAllByRole} = render( From 8e5df65c6e896487745ea655fbb40660621cd3e4 Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Thu, 6 Jul 2023 10:30:06 -0700 Subject: [PATCH 3/7] Make check for code fences more robust per spec --- .../MarkdownViewer/MarkdownViewer.test.tsx | 43 ++++++++++++++++++- .../MarkdownViewer/_useListInteraction.ts | 27 +++++++++--- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx index 298c4eb5825..c6db485a296 100644 --- a/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx +++ b/src/drafts/MarkdownViewer/MarkdownViewer.test.tsx @@ -51,6 +51,32 @@ text before list - [x] item 1 - [ ] item 2 +text after list` + const hierarchyBeforeTaskListNoItemsCheckedTildes = ` +text before list + +~~~[tasklist] +- [ ] item A +- [ ] item B +\`\`\` +~~~~~~ + +- [ ] item 1 +- [ ] item 2 + +text after list` + const hierarchyBeforeTaskListOneItemCheckedTildes = ` +text before list + +~~~[tasklist] +- [ ] item A +- [ ] item B +\`\`\` +~~~~~~ + +- [x] item 1 +- [ ] item 2 + text after list` it('enables checklists by default', () => { @@ -107,13 +133,28 @@ text after list` markdownValue={hierarchyBeforeTaskListNoItemsChecked} onChange={onChangeMock} disabled - /> + />, ) const items = getAllByRole('checkbox') fireEvent.change(items[0]) await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(hierarchyBeforeTaskListOneItemChecked)) }) + it('calls `onChange` with the updated Markdown when a task is checked and hierarchy is present with tildes', async () => { + const onChangeMock = jest.fn() + const {getAllByRole} = render( + , + ) + const items = getAllByRole('checkbox') + fireEvent.change(items[0]) + await waitFor(() => expect(onChangeMock).toHaveBeenCalledWith(hierarchyBeforeTaskListOneItemCheckedTildes)) + }) + it('calls `onChange` with the updated Markdown when a task is unchecked', async () => { const onChangeMock = jest.fn() const {getAllByRole} = render( diff --git a/src/drafts/MarkdownViewer/_useListInteraction.ts b/src/drafts/MarkdownViewer/_useListInteraction.ts index 8e47464b65c..4cbb28807a1 100644 --- a/src/drafts/MarkdownViewer/_useListInteraction.ts +++ b/src/drafts/MarkdownViewer/_useListInteraction.ts @@ -4,7 +4,16 @@ import {ListItem, listItemToString, parseListItem} from '../MarkdownEditor/_useL type TaskListItem = ListItem & {taskBox: '[ ]' | '[x]'} -const isCodeBlockDelimiter = (line: string) => line.trimStart().startsWith('```') +// Make check for code fences more robust per spec: https://github.github.com/gfm/#fenced-code-blocks +const parseCodeFenceBegin = (line: string) => { + const match = line.match(/^ {0,3}(`{3,}|~{3,})[^`]*$/) + return match ? match[1] : null +} + +const isCodeFenceEnd = (line: string, fence: string) => { + const match = line.match(new RegExp(`^ {0,3}${fence}${fence[0]}* *$`)) + return match !== null +} const isTaskListItem = (item: ListItem | null): item is TaskListItem => typeof item?.taskBox === 'string' @@ -45,17 +54,23 @@ export const useListInteraction = ({ const onToggleItem = useCallback( (toggledItemIndex: number) => () => { const lines = markdownRef.current.split('\n') - let inCodeBlock = false + let currentCodeFence: string | null = null for (let lineIndex = 0, taskIndex = 0; lineIndex < lines.length; lineIndex++) { - if (isCodeBlockDelimiter(lines[lineIndex])) { - inCodeBlock = !inCodeBlock + const line = lines[lineIndex] + + if (!currentCodeFence) { + currentCodeFence = parseCodeFenceBegin(line) + } else if (isCodeFenceEnd(line, currentCodeFence)) { + currentCodeFence = null continue } - const parsedLine = parseListItem(lines[lineIndex]) + if (currentCodeFence) continue + + const parsedLine = parseListItem(line) - if (!isTaskListItem(parsedLine) || inCodeBlock) continue + if (!isTaskListItem(parsedLine)) continue if (taskIndex === toggledItemIndex) { const updatedLine = listItemToString(toggleTaskListItem(parsedLine)) From c2e70df311154e10a9ddfe41f97c469fe5317714 Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Mon, 10 Jul 2023 17:29:32 +0000 Subject: [PATCH 4/7] Update to use regex test --- src/drafts/MarkdownViewer/_useListInteraction.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/drafts/MarkdownViewer/_useListInteraction.ts b/src/drafts/MarkdownViewer/_useListInteraction.ts index 4cbb28807a1..ce1dad4ace0 100644 --- a/src/drafts/MarkdownViewer/_useListInteraction.ts +++ b/src/drafts/MarkdownViewer/_useListInteraction.ts @@ -11,8 +11,8 @@ const parseCodeFenceBegin = (line: string) => { } const isCodeFenceEnd = (line: string, fence: string) => { - const match = line.match(new RegExp(`^ {0,3}${fence}${fence[0]}* *$`)) - return match !== null + const regex = new RegExp(`^ {0,3}${fence}${fence[0]}* *$`) + return regex.test(line) } const isTaskListItem = (item: ListItem | null): item is TaskListItem => typeof item?.taskBox === 'string' From 22cc801fb43b256b9c8679109677b30b91b1f87b Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Mon, 10 Jul 2023 17:37:58 +0000 Subject: [PATCH 5/7] Add changeset --- .changeset/smart-points-exercise.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/smart-points-exercise.md diff --git a/.changeset/smart-points-exercise.md b/.changeset/smart-points-exercise.md new file mode 100644 index 00000000000..67adbdb7df7 --- /dev/null +++ b/.changeset/smart-points-exercise.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Fix legacy task list position calculation From 89809a39dc4787792af5fff978419c85e5f8e536 Mon Sep 17 00:00:00 2001 From: Nicolle Romero Date: Mon, 10 Jul 2023 17:43:25 +0000 Subject: [PATCH 6/7] Update changeset --- .changeset/smart-points-exercise.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/smart-points-exercise.md b/.changeset/smart-points-exercise.md index 67adbdb7df7..e9e1516d204 100644 --- a/.changeset/smart-points-exercise.md +++ b/.changeset/smart-points-exercise.md @@ -2,4 +2,4 @@ '@primer/react': patch --- -Fix legacy task list position calculation +Address scenario in useListInteraction where the position calculation can be incorrect when tasklists appear above legacy task lists From 26b488f464a44aebf964cfb875efe84a95ea876b Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 11 Jul 2023 10:59:12 +0200 Subject: [PATCH 7/7] add component name in changelog --- .changeset/smart-points-exercise.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/smart-points-exercise.md b/.changeset/smart-points-exercise.md index e9e1516d204..944dee128b2 100644 --- a/.changeset/smart-points-exercise.md +++ b/.changeset/smart-points-exercise.md @@ -2,4 +2,6 @@ '@primer/react': patch --- -Address scenario in useListInteraction where the position calculation can be incorrect when tasklists appear above legacy task lists +MarkdownViewer: Address scenario in useListInteraction where the position calculation can be incorrect when tasklists appear above legacy task lists + +