diff --git a/frontend/package.json b/frontend/package.json index 00da9d16a..946802b39 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -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", diff --git a/frontend/pnpm-lock.yaml b/frontend/pnpm-lock.yaml index 961c6b40c..838e2e346 100644 --- a/frontend/pnpm-lock.yaml +++ b/frontend/pnpm-lock.yaml @@ -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: {} diff --git a/frontend/src/components/input/editor/TipTap.vue b/frontend/src/components/input/editor/TipTap.vue index ce14c65a0..1078a8500 100644 --- a/frontend/src/components/input/editor/TipTap.vue +++ b/frontend/src/components/input/editor/TipTap.vue @@ -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 }, }), diff --git a/frontend/src/components/input/editor/taskItemWithId.test.ts b/frontend/src/components/input/editor/taskItemWithId.test.ts new file mode 100644 index 000000000..f7fce9c90 --- /dev/null +++ b/frontend/src/components/input/editor/taskItemWithId.test.ts @@ -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('') + + 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(``) + + 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('') + + 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(``) + + 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('') + + 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('') + + 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('') + + 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() + }) +}) diff --git a/frontend/src/components/input/editor/taskItemWithId.ts b/frontend/src/components/input/editor/taskItemWithId.ts new file mode 100644 index 000000000..f674d0f9a --- /dev/null +++ b/frontend/src/components/input/editor/taskItemWithId.ts @@ -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() + 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 + }, + }), + ] + }, +}) diff --git a/frontend/tests/e2e/task/task.spec.ts b/frontend/tests/e2e/task/task.spec.ts index 1b9bcb61f..01dae1a5d 100644 --- a/frontend/tests/e2e/task/task.spec.ts +++ b/frontend/tests/e2e/task/task.spec.ts @@ -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: ` +`, + }) + 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,