Fix legacy task list position calculation (#3491)

* Fix bug with legacy task list position calculation

* Add test for hierarchy task list combination

* Make check for code fences more robust per spec

* Update to use regex test

* Add changeset

* Update changeset

* add component name in changelog

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

.changeset/smart-points-exercise.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+MarkdownViewer: Address scenario in useListInteraction where the position calculation can be incorrect when tasklists appear above legacy task lists
+
+<!-- Changed components: MarkdownViewer -->

src/drafts/MarkdownViewer/MarkdownViewer.test.tsx

@@ -27,6 +27,56 @@ 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`
+    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', () => {
@@ -75,6 +125,36 @@ 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(
+        <MarkdownViewer
+          dangerousRenderedHTML={htmlObject}
+          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(
+        <MarkdownViewer
+          dangerousRenderedHTML={htmlObject}
+          markdownValue={hierarchyBeforeTaskListNoItemsCheckedTildes}
+          onChange={onChangeMock}
+          disabled
+        />,
+      )
+      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(

src/drafts/MarkdownViewer/_useListInteraction.ts

@@ -4,6 +4,17 @@ import {ListItem, listItemToString, parseListItem} from '../MarkdownEditor/_useL
 
 type TaskListItem = ListItem & {taskBox: '[ ]' | '[x]'}
 
+// 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 regex = new RegExp(`^ {0,3}${fence}${fence[0]}* *$`)
+  return regex.test(line)
+}
+
 const isTaskListItem = (item: ListItem | null): item is TaskListItem => typeof item?.taskBox === 'string'
 
 const toggleTaskListItem = (item: TaskListItem): TaskListItem => ({
@@ -43,9 +54,21 @@ export const useListInteraction = ({
   const onToggleItem = useCallback(
     (toggledItemIndex: number) => () => {
       const lines = markdownRef.current.split('\n')
+      let currentCodeFence: string | null = null
 
       for (let lineIndex = 0, taskIndex = 0; lineIndex < lines.length; lineIndex++) {
-        const parsedLine = parseListItem(lines[lineIndex])
+        const line = lines[lineIndex]
+
+        if (!currentCodeFence) {
+          currentCodeFence = parseCodeFenceBegin(line)
+        } else if (isCodeFenceEnd(line, currentCodeFence)) {
+          currentCodeFence = null
+          continue
+        }
+
+        if (currentCodeFence) continue
+
+        const parsedLine = parseListItem(line)
 
         if (!isTaskListItem(parsedLine)) continue