From 25d59665b0c5638ddb3ae3068e5205378b8f52ae Mon Sep 17 00:00:00 2001 From: blokboy Date: Mon, 13 Jul 2026 15:45:43 -0500 Subject: [PATCH 1/2] fix(open-knowledge): Backspace/Delete at a list-item boundary orphaning rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging list items via Backspace (start of item) or Delete (end of item) routed through StarterKit's ListKeymap sub-extension, which assumes the fragmented BulletList/OrderedList/TaskList/TaskItem schema this repo replaced with a unified listItem node. Two of its branches misfire against that schema: Backspace into a previous item that has a nested sublist lifts the merged item clean out of the list into a bare, unmarked paragraph, and Delete across a nested/top-level depth change re-nests the merged item at the wrong depth and drops its checked attr — both read by users as "orphan" list rows (inkeep/open-knowledge#609). ListItemNode now binds Backspace/Delete itself (priority 101, ahead of ListKeymap's default 100) using prosemirror-commands' joinTextblockBackward/ joinTextblockForward, which descend through container nodes to the actual textblocks and merge only those, falling back to joinBackward/joinForward when there's no adjacent item to join into. --- .../list-item-backspace-delete-merge.md | 5 + .../app/src/editor/list-keymap-merge.test.ts | 204 ++++++++++++++++++ packages/core/src/extensions/list.ts | 58 +++++ 3 files changed, 267 insertions(+) create mode 100644 .changeset/list-item-backspace-delete-merge.md create mode 100644 packages/app/src/editor/list-keymap-merge.test.ts diff --git a/.changeset/list-item-backspace-delete-merge.md b/.changeset/list-item-backspace-delete-merge.md new file mode 100644 index 000000000..50d3675e6 --- /dev/null +++ b/.changeset/list-item-backspace-delete-merge.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Fixes `Backspace`/`Delete` at a list-item boundary orphaning rows (inkeep/open-knowledge#609). Merging two list items — e.g. backspacing at the start of a checked task item into the item above, or deleting forward across a nested/top-level boundary — could leave the merged text as a bare paragraph with no bullet or checkbox, or silently re-nest it at the wrong depth. Backspace and Delete now merge the two items' text directly, keeping the surviving item's list marker and nesting intact. diff --git a/packages/app/src/editor/list-keymap-merge.test.ts b/packages/app/src/editor/list-keymap-merge.test.ts new file mode 100644 index 000000000..ff82d643c --- /dev/null +++ b/packages/app/src/editor/list-keymap-merge.test.ts @@ -0,0 +1,204 @@ +/** + * Backspace/Delete merge at a listItem boundary (#609 cause 2 — "orphan" + * rows from deleting, as distinct from cause 1's paste mis-placement fixed + * in handle-paste.list-placement.test.ts). + * + * StarterKit's ListKeymap sub-extension (@tiptap/extension-list, wired via + * shared.ts's `listKeymap` option) targets the fragmented BulletList/ + * OrderedList/TaskList/TaskItem schema. Against this repo's unified single + * `listItem` schema, two of its branches misbehave: + * + * - Backspace at the start of an item whose PRECEDING sibling has a nested + * sublist takes `liftListItem` instead of merging text, which lifts the + * merged item clean out of the list into a bare paragraph with no + * bullet/checkbox — the orphan row users see. + * - Delete at the end of the last item in a nested sublist, merging a + * shallower next top-level item, re-nests that item at the wrong depth + * and drops its `checked` attr, silently rewriting document structure. + * + * list.ts's ListItemNode now binds Backspace/Delete itself (priority 101, + * ahead of ListKeymap's default 100) using prosemirror-commands' + * `joinTextblockBackward`/`joinTextblockForward` — these descend through + * container nodes to the actual textblocks and merge only those, never + * re-parenting or dropping the surviving item's attrs, falling back to + * `joinBackward`/`joinForward` only when there's no adjacent list item to + * join into (first/last item in the list). + * + * Each test mounts a real TipTap editor over the core schema and dispatches + * a real `keydown` through `view.someProp('handleKeyDown', ...)` — the same + * path a live keypress takes — rather than calling a PM command directly, + * so the assertions cover the actual registered keymap-plugin race. + */ + +import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test'; +import { MarkdownManager, sharedExtensions } from '@inkeep/open-knowledge-core'; +import { Editor, type JSONContent } from '@tiptap/core'; +import { TextSelection } from '@tiptap/pm/state'; +import { installDomGlobals } from './walk-currency-test-harness'; + +const mdManager = new MarkdownManager({ extensions: sharedExtensions }); + +let restoreDomGlobals: (() => void) | null = null; +const editors: Editor[] = []; + +beforeAll(() => { + restoreDomGlobals = installDomGlobals(); +}); + +afterAll(() => { + restoreDomGlobals?.(); + restoreDomGlobals = null; +}); + +afterEach(() => { + while (editors.length > 0) editors.pop()?.destroy(); +}); + +function mountEditor(md: string): Editor { + const host = document.createElement('div'); + document.body.appendChild(host); + const editor = new Editor({ + element: host, + content: mdManager.parse(md) as JSONContent, + extensions: [...sharedExtensions], + }); + editors.push(editor); + return editor; +} + +/** Doc position at the start or end of the nth listItem's first paragraph. */ +function itemPos(editor: Editor, n: number, where: 'start' | 'end'): number { + let i = 0; + let found: number | null = null; + editor.state.doc.descendants((node, pos) => { + if (found !== null) return false; + if (node.type.name === 'listItem') { + if (i === n) { + const para = node.firstChild; + found = where === 'start' ? pos + 2 : pos + 2 + (para ? para.content.size : 0); + return false; + } + i++; + } + return true; + }); + if (found === null) throw new Error(`listItem ${n} not found`); + return found; +} + +function setCaret(editor: Editor, pos: number): void { + editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, pos))); +} + +function press(editor: Editor, key: string): unknown { + const event = new KeyboardEvent('keydown', { key, code: key, bubbles: true, cancelable: true }); + return editor.view.someProp('handleKeyDown', (f) => f(editor.view, event)); +} + +function serialize(editor: Editor): string { + return mdManager.serialize(editor.getJSON() as JSONContent); +} + +/** No bare/unmarked line sitting where a list item used to be. */ +function hasOrphanParagraph(md: string): boolean { + return /\n\n[a-z]/i.test(md); +} + +const TASK_SEED = ['- [ ] alpha', '- [ ] bravo', '- [x] charlie', '- [ ] delta', '- [ ] echo'].join( + '\n', +); + +describe('list keymap — Backspace/Delete merge at an item boundary', () => { + test('Backspace at item start merges text into the previous flat item, no orphan', () => { + const editor = mountEditor(TASK_SEED); + setCaret(editor, itemPos(editor, 2, 'start')); // start of "charlie" + expect(press(editor, 'Backspace')).toBe(true); + + const md = serialize(editor); + expect(md).toContain('- [ ] bravocharlie'); + expect(hasOrphanParagraph(md)).toBe(false); + }); + + test('Backspace merging into a previous item that has a nested sublist keeps the merged text inside the list (no bare orphan paragraph)', () => { + const editor = mountEditor('- [ ] top\n - [ ] child\n- [ ] next'); + setCaret(editor, itemPos(editor, 2, 'start')); // start of "next" (top-level, after "top"+"child") + expect(press(editor, 'Backspace')).toBe(true); + + const md = serialize(editor); + // "next" must land inside the list (merged onto the deepest preceding + // textblock), never as a bare paragraph outside any list item. + expect(hasOrphanParagraph(md)).toBe(false); + expect(md).toContain('childnext'); + }); + + test('Delete at the end of a nested item merges the next shallower item at the correct depth, checked attr intact', () => { + const editor = mountEditor('- [ ] a\n - [ ] b\n - [ ] c\n- [ ] d'); + setCaret(editor, itemPos(editor, 2, 'end')); // end of "c" (nested, sibling of b) + expect(press(editor, 'Delete')).toBe(true); + + const md = serialize(editor); + // "d" merges onto "c" at the nested depth — no bare re-nested plain + // bullet, no orphan line at the wrong indent. + expect(md).toContain(' - [ ] cd'); + expect(hasOrphanParagraph(md)).toBe(false); + }); + + test('a ranged selection spanning two items still deletes via the normal path (unaffected)', () => { + const editor = mountEditor(TASK_SEED); + const from = itemPos(editor, 1, 'start'); // start of "bravo" + const to = itemPos(editor, 2, 'start'); // start of "charlie" + editor.view.dispatch( + editor.state.tr.setSelection(TextSelection.create(editor.state.doc, from, to)), + ); + editor.view.dispatch(editor.state.tr.deleteSelection()); + + const md = serialize(editor); + expect(md).toContain('- [x] charlie'); + expect(md).not.toContain('bravo'); + expect(hasOrphanParagraph(md)).toBe(false); + }); +}); + +describe('list keymap — pre-existing behavior preserved', () => { + test('Backspace on the empty line after a list merges back in (no stray bullet)', () => { + const editor = mountEditor('- item one\n'); + setCaret(editor, editor.state.doc.content.size - 1); + press(editor, 'Enter'); // empty second item + press(editor, 'Enter'); // lifts out to a plain paragraph below the list + press(editor, 'Backspace'); // must rejoin the list, not spawn a bullet + + const md = serialize(editor).trim(); + expect(md).toBe('- item one'); + expect(md).not.toMatch(/^- *$/m); + }); + + test('Backspace on an empty nested item removes it (does not toggle the bullet)', () => { + const editor = mountEditor('- top\n - sub\n'); + setCaret(editor, editor.state.doc.content.size - 1); + press(editor, 'Enter'); // empty nested item after "sub" + press(editor, 'Backspace'); // removes the empty item, back to "sub" + + const md = serialize(editor); + expect(md).toMatch(/^- top\n {2}- sub\n?$/m); + expect(md.match(/- sub/g)).toHaveLength(1); + }); + + test('Backspace at the very start of the first item in a list lifts it out to a plain paragraph', () => { + const editor = mountEditor('- alpha\n- bravo\n'); + setCaret(editor, itemPos(editor, 0, 'start')); + press(editor, 'Backspace'); + + const md = serialize(editor); + expect(md).toContain('alpha'); + expect(md).toContain('- bravo'); + }); + + test('Enter at end of a task item still creates a new task item', () => { + const editor = mountEditor('- [ ] sf\n'); + setCaret(editor, itemPos(editor, 0, 'end')); + press(editor, 'Enter'); + + const md = serialize(editor); + expect(md).toMatch(/^- \[ \] sf\n- \[ \] ?$/m); + }); +}); diff --git a/packages/core/src/extensions/list.ts b/packages/core/src/extensions/list.ts index 984d84859..5cc227ec4 100644 --- a/packages/core/src/extensions/list.ts +++ b/packages/core/src/extensions/list.ts @@ -19,6 +19,13 @@ */ import { findParentNode, InputRule, mergeAttributes, Node, wrappingInputRule } from '@tiptap/core'; +import { + chainCommands, + joinTextblockBackward, + joinTextblockForward, + joinBackward as pmJoinBackward, + joinForward as pmJoinForward, +} from '@tiptap/pm/commands'; import type { NodeType, Node as PmNode } from '@tiptap/pm/model'; import { liftListItem as pmLiftListItem, wrapInList as pmWrapInList } from '@tiptap/pm/schema-list'; import type { EditorState, Transaction } from '@tiptap/pm/state'; @@ -315,8 +322,18 @@ export const ListNode = Node.create({ // default priority (100) matches stock TipTap and lets our splitListItem // run first; a previous `priority: 60` here regressed Enter on every list // type. +// +// priority: 101 — one above default so this extension's Backspace/Delete +// (joinTextblockBackward/Forward, see addKeyboardShortcuts) run before +// StarterKit's ListKeymap sub-extension (also default 100, registered later +// in sharedExtensions but stable-sorted ahead of same-priority extensions +// added earlier). ListKeymap's joinItemBackward/liftListItem branches target +// the fragmented BulletList/TaskItem schema; against this unified listItem +// schema they can lift a same-depth sibling clean out of the list (issue +// #609 orphan rows) before this extension's handler ever runs. export const ListItemNode = Node.create({ name: 'listItem', + priority: 101, content: 'paragraph block*', defining: true, @@ -458,6 +475,38 @@ export const ListItemNode = Node.create({ addKeyboardShortcuts() { return { Enter: () => this.editor.commands.splitListItem(this.name), + // Backspace/Delete at a listItem boundary — bypass the upstream + // ListKeymap extension's joinItemBackward/joinItemForward and + // has-sublist/depth-mismatch branches (@tiptap/extension-list, wired + // via StarterKit's `listKeymap` option in shared.ts). Those branches + // assume the fragmented BulletList/OrderedList/TaskList/TaskItem + // schema; against this unified single-`listItem` schema they can + // `liftListItem` a same-depth sibling clean out of the list entirely + // (previous item has a nested sublist → the merged-in item becomes a + // bare paragraph with no bullet) or re-nest a merged item at the wrong + // depth with its `checked` attr dropped (Delete across a depth + // change) — both read by users as "orphan" list rows (issue #609). + // + // `joinTextblockBackward`/`joinTextblockForward` (prosemirror-commands) + // are the general-purpose fix: at a boundary between two container + // nodes (listItem/listItem, or listItem/nested-list), they descend via + // lastChild/firstChild to the actual textblocks and merge only those — + // never re-parenting or dropping the surviving item's attrs. Only + // engage them when the cursor is inside a listItem so plain-document + // Backspace/Delete (paragraphs, headings, etc.) keeps the stock + // joinBackward/joinForward behavior. Falling back to joinBackward/ + // joinForward keeps the existing lift-out-of-list behavior for the + // first/last item in a list (no adjacent item to join into). + Backspace: () => { + if (!isCursorInsideListItem(this.editor.state)) return false; + const { state, view } = this.editor; + return chainCommands(joinTextblockBackward, pmJoinBackward)(state, view.dispatch, view); + }, + Delete: () => { + if (!isCursorInsideListItem(this.editor.state)) return false; + const { state, view } = this.editor; + return chainCommands(joinTextblockForward, pmJoinForward)(state, view.dispatch, view); + }, Tab: () => { // Only handle Tab when the cursor is inside a listItem — otherwise // pass through so other extensions (e.g., table) can handle it. @@ -482,6 +531,15 @@ export const ListItemNode = Node.create({ }, }); +/** True when the selection's anchor sits inside a `listItem` ancestor. */ +function isCursorInsideListItem(state: EditorState): boolean { + const { $from } = state.selection; + for (let d = $from.depth; d > 0; d--) { + if ($from.node(d).type.name === 'listItem') return true; + } + return false; +} + /** * Combined export for registration in shared.ts. * Register both ListNode and ListItemNode to get the full list experience. From e4c01c3a42161950b8a2b05151bd4bcd98b03a1c Mon Sep 17 00:00:00 2001 From: blokboy Date: Mon, 13 Jul 2026 15:45:43 -0500 Subject: [PATCH 2/2] test(open-knowledge): Edge case catch while testing Two new list-keymap.e2e.ts cases exercise the exact orphan scenarios via real Playwright keystrokes: Backspace merging into an item whose preceding sibling has a nested sublist, and Delete merging across a nested/top-level depth boundary. Testing locally also appears issue is resolved. --- packages/app/tests/stress/list-keymap.e2e.ts | 58 ++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/packages/app/tests/stress/list-keymap.e2e.ts b/packages/app/tests/stress/list-keymap.e2e.ts index f5f454aed..7aaf1747f 100644 --- a/packages/app/tests/stress/list-keymap.e2e.ts +++ b/packages/app/tests/stress/list-keymap.e2e.ts @@ -343,6 +343,64 @@ test.describe('OQ1: Tab/Shift-Tab scoping by cursor context', () => { expect(ytext).not.toMatch(/^- *$/m); }); + // #609 cause 2 — deleting/merging list items (as distinct from cause 1's + // paste mis-placement, covered by handle-paste.list-placement.test.ts). + // StarterKit's ListKeymap (its joinItemBackward/liftListItem branches + // target the fragmented BulletList/TaskItem schema) could lift a merged + // item clean out of the list into a bare, unmarked paragraph, or re-nest + // it at the wrong depth with `checked` dropped. list.ts's ListItemNode + // now binds Backspace/Delete itself via joinTextblockBackward/Forward at + // priority 101 (ahead of ListKeymap's default 100). + test('Backspace merging into a previous item with a nested sublist does not orphan the merged item', async ({ + page, + api, + }) => { + const docName = uniqueDocName('bksp-merge-sublist'); + await openDoc(api, page, docName); + await seedMarkdown(api, page, docName, '- [ ] top\n - [ ] child\n- [ ] next\n'); + + await page.locator('.ProseMirror:not(.composer-prosemirror)').focus(); + // Click into "next" — a top-level item whose preceding sibling ("top") + // has a nested sublist. + await page.locator('.ProseMirror li', { hasText: 'next' }).first().click(); + await page.keyboard.press('Home'); + await waitForPmSelectionInNode(page, 'listItem'); + + await page.keyboard.press('Backspace'); + + // "next" must merge as text into the list (onto "child", the deepest + // preceding textblock) — never survive as a bare paragraph with no + // bullet/checkbox sitting outside any list item. + await expect.poll(() => getYText(page)).toContain('childnext'); + const ytext = await getYText(page); + expect(ytext).not.toMatch(/\n\nnext/); + expect(ytext).toContain('- [ ] top'); + }); + + test('Delete at the end of a nested item merges the next top-level item at the correct depth', async ({ + page, + api, + }) => { + const docName = uniqueDocName('del-merge-depth'); + await openDoc(api, page, docName); + await seedMarkdown(api, page, docName, '- [ ] a\n - [ ] b\n - [ ] c\n- [ ] d\n'); + + await page.locator('.ProseMirror:not(.composer-prosemirror)').focus(); + await page.locator('.ProseMirror li', { hasText: /^c$/ }).first().click(); + await page.keyboard.press('End'); + await waitForPmSelectionInNode(page, 'listItem'); + + await page.keyboard.press('Delete'); + + // "d" merges onto "c" at the nested depth, keeping "c"'s checkbox on the + // surviving item and its indentation — not re-nested as a bare bullet + // at the wrong depth. + await expect.poll(() => getYText(page)).toMatch(/^ {2}- \[ \] cd$/m); + const ytext = await getYText(page); + expect(ytext).toContain('- [ ] a'); + expect(ytext).not.toMatch(/^- d$/m); + }); + test.fixme('Tab inside a codeBlock inserts 2 spaces', async ({ page, api }) => { const docName = uniqueDocName('tab-codeblock'); await openDoc(api, page, docName);