fix(editor): make sure checkbox lists are unique (#2007)

This fixes a checkbox persistence bug where toggling one checkbox would affect others with identical text. To make this work, a new unique `data-task-id` attribute was added to each task list item for reliable identification.

Fixes #293, #563

🐰 With nanoid's magic and IDs so fine,
Each checkbox now knows which line is mine,
No more lost state when the page reloads—
Tasks persist through every browser node!
This commit is contained in:
kolaente
2025-12-19 17:12:31 +01:00
committed by GitHub
parent 84791aa996
commit ef1efbc29a
6 changed files with 308 additions and 12 deletions

View File

@@ -91,6 +91,7 @@
"klona": "2.0.6",
"lowlight": "3.3.0",
"marked": "17.0.1",
"nanoid": "^5.1.6",
"pinia": "3.0.4",
"register-service-worker": "1.7.2",
"sortablejs": "1.15.6",

View File

@@ -136,6 +136,9 @@ importers:
marked:
specifier: 17.0.1
version: 17.0.1
nanoid:
specifier: ^5.1.6
version: 5.1.6
pinia:
specifier: 3.0.4
version: 3.0.4(typescript@5.9.3)(vue@3.5.25(typescript@5.9.3))
@@ -4669,8 +4672,8 @@ packages:
engines: {node: ^10 || ^12 || ^13.7 || ^14 || >=15.0.1}
hasBin: true
nanoid@5.1.5:
resolution: {integrity: sha512-Ir/+ZpE9fDsNH0hQ3C68uyThDXzYcim2EqcZ8zn8Chtt1iylPT9xXJB0kPCnqzgcEGikO9RxSrh63MsmVCU7Fw==}
nanoid@5.1.6:
resolution: {integrity: sha512-c7+7RQ+dMB5dPwwCp4ee1/iV/q2P6aK1mTZcfr1BTuVlyW9hJYiMPybJCcnBlQtuSmTIWNeazm/zqNoZSSElBg==}
engines: {node: ^18 || >=20}
hasBin: true
@@ -9284,7 +9287,7 @@ snapshots:
'@vue/devtools-kit': 8.0.5
'@vue/devtools-shared': 8.0.5
mitt: 3.0.1
nanoid: 5.1.5
nanoid: 5.1.6
pathe: 2.0.3
vite-hot-client: 2.1.0(vite@7.2.7(@types/node@22.19.3)(jiti@2.4.2)(sass-embedded@1.96.0)(sass@1.96.0)(terser@5.31.6)(yaml@2.5.0))
vue: 3.5.25(typescript@5.9.3)
@@ -11381,7 +11384,7 @@ snapshots:
nanoid@3.3.11: {}
nanoid@5.1.5: {}
nanoid@5.1.6: {}
natural-compare@1.4.0: {}

View File

@@ -160,7 +160,8 @@ import Underline from '@tiptap/extension-underline'
import {Placeholder} from '@tiptap/extensions'
import Mention from '@tiptap/extension-mention'
import {TaskItem, TaskList} from '@tiptap/extension-list'
import {TaskList} from '@tiptap/extension-list'
import {TaskItemWithId} from './taskItemWithId'
import HardBreak from '@tiptap/extension-hard-break'
import {Node} from '@tiptap/pm/model'
@@ -468,30 +469,48 @@ const extensions : Extensions = [
CustomImage,
TaskList,
TaskItem.configure({
TaskItemWithId.configure({
nested: true,
onReadOnlyChecked: (node: Node, checked: boolean): boolean => {
if (!props.isEditEnabled) {
return false
}
// The following is a workaround for this bug:
// https://github.com/ueberdosis/tiptap/issues/4521
// https://github.com/ueberdosis/tiptap/issues/3676
// Use taskId attribute to reliably find the correct node
// This fixes GitHub issues #293 and #563
const targetTaskId = node.attrs.taskId
if (!targetTaskId) {
// Fallback to original behavior if no ID (shouldn't happen)
console.warn('TaskItem missing taskId, falling back to node comparison')
editor.value!.state.doc.descendants((subnode, pos) => {
if (subnode === node) {
const {tr} = editor.value!.state
tr.setNodeMarkup(pos, undefined, {
...node.attrs,
checked,
})
editor.value!.view.dispatch(tr)
bubbleSave()
}
})
return true
}
// Find node by taskId for reliable matching
editor.value!.state.doc.descendants((subnode, pos) => {
if (subnode === node) {
if (subnode.type.name === 'taskItem' && subnode.attrs.taskId === targetTaskId) {
const {tr} = editor.value!.state
tr.setNodeMarkup(pos, undefined, {
...node.attrs,
...subnode.attrs,
checked,
})
editor.value!.view.dispatch(tr)
bubbleSave()
return false // Stop iteration once found
}
})
return true
},
}),

View File

@@ -0,0 +1,137 @@
import { describe, it, expect } from 'vitest'
import { Editor } from '@tiptap/core'
import StarterKit from '@tiptap/starter-kit'
import { TaskList } from '@tiptap/extension-list'
import { TaskItemWithId } from './taskItemWithId'
describe('TaskItemWithId Extension', () => {
const createEditor = (content: string = '') => {
return new Editor({
extensions: [
StarterKit,
TaskList,
TaskItemWithId.configure({ nested: true }),
],
content,
})
}
it('should generate unique IDs for new task items', () => {
const editor = createEditor()
editor.commands.setContent('<ul data-type="taskList"><li data-type="taskItem" data-checked="false"><p>Item 1</p></li></ul>')
const html = editor.getHTML()
expect(html).toContain('data-task-id=')
editor.destroy()
})
it('should preserve existing IDs when parsing HTML', () => {
const existingId = 'test-id-123'
const editor = createEditor()
editor.commands.setContent(`<ul data-type="taskList"><li data-type="taskItem" data-checked="false" data-task-id="${existingId}"><p>Item 1</p></li></ul>`)
const html = editor.getHTML()
expect(html).toContain(`data-task-id="${existingId}"`)
editor.destroy()
})
it('should generate different IDs for different items', () => {
const editor = createEditor()
editor.commands.setContent('<ul data-type="taskList"><li data-type="taskItem" data-checked="false"><p>Item 1</p></li><li data-type="taskItem" data-checked="false"><p>Item 2</p></li><li data-type="taskItem" data-checked="false"><p>Item 3</p></li></ul>')
const html = editor.getHTML()
const idMatches = html.match(/data-task-id="([^"]+)"/g)
expect(idMatches).toHaveLength(3)
// Extract IDs and verify they're unique
const ids = idMatches!.map(match => match.match(/data-task-id="([^"]+)"/)?.[1])
const uniqueIds = new Set(ids)
expect(uniqueIds.size).toBe(3)
editor.destroy()
})
it('should regenerate duplicate IDs to be unique', () => {
const editor = createEditor()
const duplicateId = 'duplicate-id-abc'
// Inject HTML with two task items sharing the same data-task-id
editor.commands.setContent(`<ul data-type="taskList"><li data-type="taskItem" data-checked="false" data-task-id="${duplicateId}"><p>Item 1</p></li><li data-type="taskItem" data-checked="false" data-task-id="${duplicateId}"><p>Item 2</p></li></ul>`)
const html = editor.getHTML()
const idMatches = html.match(/data-task-id="([^"]+)"/g)
// Assert there are two data-task-id attributes
expect(idMatches).toHaveLength(2)
// Extract their values and assert the Set of IDs has size 2
const ids = idMatches!.map(match => match.match(/data-task-id="([^"]+)"/)?.[1])
const uniqueIds = new Set(ids)
expect(uniqueIds.size).toBe(2)
editor.destroy()
})
it('should generate different IDs for different items with the same name', () => {
const editor = createEditor()
editor.commands.setContent('<ul data-type="taskList"><li data-type="taskItem" data-checked="false"><p>Item 1</p></li><li data-type="taskItem" data-checked="false"><p>Item 1</p></li><li data-type="taskItem" data-checked="false"><p>Item 2</p></li></ul>')
const html = editor.getHTML()
const idMatches = html.match(/data-task-id="([^"]+)"/g)
expect(idMatches).toHaveLength(3)
// Extract IDs and verify they're unique
const ids = idMatches!.map(match => match.match(/data-task-id="([^"]+)"/)?.[1])
const uniqueIds = new Set(ids)
expect(uniqueIds.size).toBe(3)
editor.destroy()
})
it('should preserve IDs through getHTML/setContent round-trip', () => {
const editor = createEditor()
editor.commands.setContent('<ul data-type="taskList"><li data-type="taskItem" data-checked="false"><p>Test</p></li></ul>')
const html1 = editor.getHTML()
const idMatch1 = html1.match(/data-task-id="([^"]+)"/)
const originalId = idMatch1?.[1]
// Simulate round-trip
editor.commands.setContent(html1)
const html2 = editor.getHTML()
const idMatch2 = html2.match(/data-task-id="([^"]+)"/)
const preservedId = idMatch2?.[1]
expect(preservedId).toBe(originalId)
editor.destroy()
})
it('should handle items with identical text correctly', () => {
const editor = createEditor()
editor.commands.setContent('<ul data-type="taskList"><li data-type="taskItem" data-checked="false"><p>Duplicate</p></li><li data-type="taskItem" data-checked="false"><p>Duplicate</p></li><li data-type="taskItem" data-checked="false"><p>Duplicate</p></li></ul>')
const html = editor.getHTML()
const idMatches = html.match(/data-task-id="([^"]+)"/g)
expect(idMatches).toHaveLength(3)
// Even with identical text, IDs should be unique
const ids = idMatches!.map(match => match.match(/data-task-id="([^"]+)"/)?.[1])
const uniqueIds = new Set(ids)
expect(uniqueIds.size).toBe(3)
editor.destroy()
})
})

View File

@@ -0,0 +1,82 @@
import { TaskItem } from '@tiptap/extension-list'
import { nanoid } from 'nanoid'
import { Plugin, PluginKey } from '@tiptap/pm/state'
import type { Node } from '@tiptap/pm/model'
const uniqueIdPluginKey = new PluginKey('taskItemUniqueId')
/**
* Custom TaskItem extension that adds a unique ID to each task item.
* This fixes the checkbox persistence bug (GitHub #293, #563) by allowing
* reliable identification of which checkbox was toggled.
*/
export const TaskItemWithId = TaskItem.extend({
addAttributes() {
return {
...this.parent?.(),
taskId: {
default: null,
parseHTML: (element: HTMLElement) => {
// Preserve existing ID or generate new one
return element.getAttribute('data-task-id') || nanoid(8)
},
renderHTML: (attributes) => {
return {
'data-task-id': attributes.taskId || nanoid(8),
}
},
},
}
},
addProseMirrorPlugins() {
const parentPlugins = this.parent?.() || []
return [
...parentPlugins,
new Plugin({
key: uniqueIdPluginKey,
appendTransaction: (transactions, oldState, newState) => {
// Only process if document changed
if (!transactions.some(tr => tr.docChanged)) {
return null
}
const seenIds = new Set<string>()
const duplicates: { pos: number; node: Node }[] = []
// Find all task items and check for duplicate IDs
newState.doc.descendants((node, pos) => {
if (node.type.name === 'taskItem') {
const taskId = node.attrs.taskId
if (!taskId || seenIds.has(taskId)) {
// Missing or duplicate ID - needs regeneration
duplicates.push({ pos, node })
} else {
seenIds.add(taskId)
}
}
})
// If no duplicates, no transaction needed
if (duplicates.length === 0) {
return null
}
// Create transaction to fix duplicates
const tr = newState.tr
for (const { pos, node } of duplicates) {
tr.setNodeMarkup(pos, undefined, {
...node.attrs,
taskId: nanoid(8),
})
}
return tr
},
}),
]
},
})

View File

@@ -969,6 +969,60 @@ test.describe('Task', () => {
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').first()).toBeChecked()
})
test('Loads old checklist data without task IDs and generates unique IDs (backwards compatibility)', async ({authenticatedPage: page}) => {
// Old checklist HTML format without data-task-id attributes
// This simulates data saved before the checkbox persistence fix
const tasks = await TaskFactory.create(1, {
id: 1,
description: `
<ul data-type="taskList">
<li data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label>
<div><p>Item One</p></div>
</li>
<li data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label>
<div><p>Item Two</p></div>
</li>
<li data-checked="false" data-type="taskItem"><label><input type="checkbox"><span></span></label>
<div><p>Item Three</p></div>
</li>
</ul>`,
})
await page.goto(`/tasks/${tasks[0].id}`)
// Verify all checkboxes are rendered
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]')).toHaveCount(3)
await expect(page.locator('.task-view .checklist-summary')).toContainText('0 of 3 tasks')
// Check that unique IDs were generated for each task item
const taskIds = await page.evaluate(() => {
const items = document.querySelectorAll('.tiptap__editor [data-task-id]')
return Array.from(items).map(el => el.getAttribute('data-task-id'))
})
expect(taskIds).toHaveLength(3)
// All IDs should be unique
const uniqueIds = new Set(taskIds)
expect(uniqueIds.size).toBe(3)
// Toggle only the second checkbox
await page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(1).click()
await expect(page.locator('.task-view .details.content.description h3 span.is-small.has-text-success')).toContainText('Saved!')
await expect(page.locator('.task-view .checklist-summary')).toContainText('1 of 3 tasks')
// Verify only the second checkbox is checked, not the others
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(0)).not.toBeChecked()
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(1)).toBeChecked()
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(2)).not.toBeChecked()
// Reload and verify persistence
await page.reload()
await expect(page.locator('.task-view .checklist-summary')).toContainText('1 of 3 tasks')
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(0)).not.toBeChecked()
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(1)).toBeChecked()
await expect(page.locator('.tiptap__editor ul > li input[type=checkbox]').nth(2)).not.toBeChecked()
})
test('Should use the editor to render description', async ({authenticatedPage: page}) => {
const tasks = await TaskFactory.create(1, {
id: 1,