From 553419f8ef1392bec220467d2a1f0dca20dfdceb Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:31:56 +0800 Subject: [PATCH] fix(editor): indent multi-item list selection on Tab (MUL-3697) (#4587) Stock prosemirror-schema-list `sinkListItem` returns false without dispatching whenever `range.startIndex === 0`, so selecting a list from the top and pressing Tab did nothing. Bullet, ordered, and task lists all routed through the same command and were equally affected. Wrap the shared Tab keymap (PatchedListItem + PatchedTaskItem) with `sinkListItemRange`: try stock sink first, and when it bails on a multi-item range whose first item is the list's first child, re-run the stock command on a selection narrowed to start inside the second selected item. The first item stays as an anchor and the rest nest under it (Notion/GitHub nested-list behaviour), in a single undoable transaction. Shift-Tab / liftListItem already handles ranges and the first-item case, so it is unchanged. No schema change. Co-authored-by: Claude Opus 4.8 Co-authored-by: multica-agent --- .../views/editor/extensions/list-item.test.ts | 278 +++++++++++++++++- packages/views/editor/extensions/list-item.ts | 71 ++++- 2 files changed, 331 insertions(+), 18 deletions(-) diff --git a/packages/views/editor/extensions/list-item.test.ts b/packages/views/editor/extensions/list-item.test.ts index 843f5052b..acef310c8 100644 --- a/packages/views/editor/extensions/list-item.test.ts +++ b/packages/views/editor/extensions/list-item.test.ts @@ -1,7 +1,8 @@ import { afterEach, describe, expect, it } from "vitest"; import { Editor } from "@tiptap/core"; import StarterKit from "@tiptap/starter-kit"; -import { PatchedListItem } from "./list-item"; +import { TaskList } from "@tiptap/extension-list"; +import { PatchedListItem, PatchedTaskItem } from "./list-item"; interface JsonNode { type: string; @@ -14,7 +15,12 @@ function makeEditor(content: JsonNode) { document.body.appendChild(element); return new Editor({ element, - extensions: [StarterKit.configure({ listItem: false }), PatchedListItem], + extensions: [ + StarterKit.configure({ listItem: false }), + PatchedListItem, + TaskList, + PatchedTaskItem, + ], content, }); } @@ -37,26 +43,100 @@ function listItemTextPos(editor: Editor, index: number): number { return pos; } -/** Mimic the editor's Enter keymap: invoke the bound Enter shortcut directly. */ -function pressEnter(editor: Editor): boolean { - const listItemExt = editor.extensionManager.extensions.find( - (e) => e.name === "listItem", - ); - if (!listItemExt) throw new Error("listItem extension not registered"); +/** + * Mimic an editor keymap by invoking a bound shortcut directly. We can't drive + * real key events reliably in jsdom, so we resolve the keymap an extension + * registers and call the entry for `key`. The shared list keymap closes over + * `editor` (not `this`), so the rebind only needs a faithful `this`. + */ +function pressShortcut(editor: Editor, extName: string, key: string): boolean { + const ext = editor.extensionManager.extensions.find((e) => e.name === extName); + if (!ext) throw new Error(`${extName} extension not registered`); const shortcuts = ( - listItemExt.config.addKeyboardShortcuts as + ext.config.addKeyboardShortcuts as | (() => Record boolean>) | undefined )?.bind({ editor, - name: "listItem", - options: listItemExt.options, - type: editor.schema.nodes.listItem, - storage: listItemExt.storage, + name: extName, + options: ext.options, + type: editor.schema.nodes[extName], + storage: ext.storage, } as never)(); - const enter = shortcuts?.Enter; - if (!enter) throw new Error("Enter shortcut not bound"); - return enter(); + const fn = shortcuts?.[key]; + if (!fn) throw new Error(`${key} shortcut not bound on ${extName}`); + return fn(); +} + +/** Mimic the editor's Enter keymap: invoke the bound Enter shortcut directly. */ +function pressEnter(editor: Editor): boolean { + return pressShortcut(editor, "listItem", "Enter"); +} + +/** Indented bullet outline of the doc — nesting depth, item text only. */ +const LIST_TYPES = ["bulletList", "orderedList", "taskList"]; +const ITEM_TYPES = ["listItem", "taskItem"]; +function outline(json: JsonNode): string { + const lines: string[] = []; + function rec(node: JsonNode, depth: number) { + for (const child of node.content ?? []) { + if (LIST_TYPES.includes(child.type)) { + rec(child, depth + 1); + } else if (ITEM_TYPES.includes(child.type)) { + const text = child.content?.[0]?.content?.[0]?.text ?? ""; + lines.push(" ".repeat(Math.max(0, depth - 1)) + "- " + text); + for (const gc of child.content ?? []) { + if (LIST_TYPES.includes(gc.type)) rec(gc, depth + 1); + } + } else { + rec(child, depth); + } + } + } + rec(json, 0); + return lines.join("\n"); +} + +/** Inside-paragraph position of the index-th item of `typeName` (doc order). */ +function itemPos(editor: Editor, typeName: string, index: number): number { + const positions: number[] = []; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === typeName) positions.push(pos + 2); + return true; + }); + const pos = positions[index]; + if (pos === undefined) throw new Error(`no ${typeName} at index ${index}`); + return pos; +} + +/** Select from the start of item `fromIdx`'s text to the end of item `toIdx`'s. */ +function selectItemRange( + editor: Editor, + typeName: string, + fromIdx: number, + toIdx: number, + itemLen = 3, +) { + editor.commands.setTextSelection({ + from: itemPos(editor, typeName, fromIdx), + to: itemPos(editor, typeName, toIdx) + itemLen, + }); +} + +/** A flat three-item list ("aaa","bbb","ccc") of the given node types. */ +function flatList(listType: string, itemType: string): JsonNode { + return { + type: "doc", + content: [ + { + type: listType, + content: ["aaa", "bbb", "ccc"].map((t) => ({ + type: itemType, + content: [{ type: "paragraph", content: [{ type: "text", text: t }] }], + })), + }, + ], + }; } describe("PatchedListItem Enter behaviour", () => { @@ -179,3 +259,169 @@ describe("PatchedListItem Enter behaviour", () => { expect(outerText).toBe("outer"); }); }); + +describe("PatchedListItem Tab indent (MUL-3697)", () => { + let editor: Editor | undefined; + afterEach(() => { + editor?.destroy(); + editor = undefined; + document.body.innerHTML = ""; + }); + + const pressTab = (e: Editor) => pressShortcut(e, "listItem", "Tab"); + + it("is a no-op with the cursor in the first item (nothing to nest under)", () => { + editor = makeEditor(flatList("bulletList", "listItem")); + editor.commands.setTextSelection(itemPos(editor, "listItem", 0)); + // C2: startIndex === 0 with a single item -> clean false, doc unchanged. + expect(pressTab(editor)).toBe(false); + expect(outline(editor.getJSON() as JsonNode)).toBe("- aaa\n- bbb\n- ccc"); + }); + + it("indents a single non-first item under its predecessor", () => { + editor = makeEditor(flatList("bulletList", "listItem")); + editor.commands.setTextSelection(itemPos(editor, "listItem", 1)); + expect(pressTab(editor)).toBe(true); + expect(outline(editor.getJSON() as JsonNode)).toBe( + "- aaa\n - bbb\n- ccc", + ); + }); + + it("indents a whole-list selection (items 1..3): first stays, 2 and 3 nest", () => { + editor = makeEditor(flatList("bulletList", "listItem")); + selectItemRange(editor, "listItem", 0, 2); + expect(pressTab(editor)).toBe(true); + // The reported bug: this used to be a no-op because range.startIndex === 0. + expect(outline(editor.getJSON() as JsonNode)).toBe( + "- aaa\n - bbb\n - ccc", + ); + }); + + it("indents a mid-list selection (items 2..3) under the first", () => { + editor = makeEditor(flatList("bulletList", "listItem")); + selectItemRange(editor, "listItem", 1, 2); + expect(pressTab(editor)).toBe(true); + expect(outline(editor.getJSON() as JsonNode)).toBe( + "- aaa\n - bbb\n - ccc", + ); + }); + + it("returns false cleanly when the selection is not in a list (C2: no range)", () => { + editor = makeEditor({ + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "plain" }] }, + ], + }); + editor.commands.setTextSelection(3); + expect(pressTab(editor)).toBe(false); + }); + + it("indents in a single, undoable transaction (C3)", () => { + editor = makeEditor(flatList("bulletList", "listItem")); + selectItemRange(editor, "listItem", 0, 2); + + const view = editor.view; + const original = view.dispatch.bind(view); + let dispatches = 0; + view.dispatch = (tr) => { + dispatches += 1; + original(tr); + }; + try { + expect(pressTab(editor)).toBe(true); + } finally { + view.dispatch = original; + } + // One dispatch -> one transaction -> one undo step. + expect(dispatches).toBe(1); + + editor.commands.undo(); + expect(outline(editor.getJSON() as JsonNode)).toBe("- aaa\n- bbb\n- ccc"); + }); +}); + +describe("Tab indent across list types (MUL-3697)", () => { + let editor: Editor | undefined; + afterEach(() => { + editor?.destroy(); + editor = undefined; + document.body.innerHTML = ""; + }); + + it("indents an ordered-list whole selection (not only unordered)", () => { + editor = makeEditor(flatList("orderedList", "listItem")); + selectItemRange(editor, "listItem", 0, 2); + expect(pressShortcut(editor, "listItem", "Tab")).toBe(true); + expect(outline(editor.getJSON() as JsonNode)).toBe( + "- aaa\n - bbb\n - ccc", + ); + expect((editor.getJSON() as JsonNode).content?.[0]?.type).toBe( + "orderedList", + ); + }); + + it("indents a task-list whole selection via the taskItem keymap", () => { + editor = makeEditor(flatList("taskList", "taskItem")); + selectItemRange(editor, "taskItem", 0, 2); + expect(pressShortcut(editor, "taskItem", "Tab")).toBe(true); + expect(outline(editor.getJSON() as JsonNode)).toBe( + "- aaa\n - bbb\n - ccc", + ); + }); +}); + +describe("Shift-Tab dedent regression (MUL-3697)", () => { + let editor: Editor | undefined; + afterEach(() => { + editor?.destroy(); + editor = undefined; + document.body.innerHTML = ""; + }); + + it("lifts a multi-item nested selection back to the top level (unchanged)", () => { + editor = makeEditor({ + type: "doc", + content: [ + { + type: "bulletList", + content: [ + { + type: "listItem", + content: [ + { type: "paragraph", content: [{ type: "text", text: "aaa" }] }, + { + type: "bulletList", + content: [ + { + type: "listItem", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "bbb" }], + }, + ], + }, + { + type: "listItem", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "ccc" }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }); + // Select the two nested items (bbb, ccc) and dedent. + selectItemRange(editor, "listItem", 1, 2); + expect(pressShortcut(editor, "listItem", "Shift-Tab")).toBe(true); + expect(outline(editor.getJSON() as JsonNode)).toBe("- aaa\n- bbb\n- ccc"); + }); +}); diff --git a/packages/views/editor/extensions/list-item.ts b/packages/views/editor/extensions/list-item.ts index 11b295577..276878767 100644 --- a/packages/views/editor/extensions/list-item.ts +++ b/packages/views/editor/extensions/list-item.ts @@ -1,5 +1,65 @@ import { type Editor, InputRule } from "@tiptap/core"; import { ListItem, TaskItem } from "@tiptap/extension-list"; +import { sinkListItem as pmSinkListItem } from "@tiptap/pm/schema-list"; +import { type Command, TextSelection } from "@tiptap/pm/state"; +import type { NodeType } from "@tiptap/pm/model"; + +/** + * Tab indent that also works for a multi-item selection whose first item is the + * first child of its (sub)list. + * + * Stock `sinkListItem` (prosemirror-schema-list) bails — returns false without + * dispatching — whenever `range.startIndex === 0`, because the first item has no + * preceding sibling to nest under. That is correct for a collapsed cursor in the + * first item, but it also kills the natural "select the whole list from the top + * and press Tab" gesture: the command sees the first item at index 0 and does + * nothing (MUL-3697). + * + * The structurally-correct behaviour in a nested-list model (matching Notion / + * GitHub) is: keep the first selected item as an anchor and sink the rest under + * it. We get that by re-running the *stock* command on a selection narrowed to + * start inside the SECOND selected item (so its `startIndex` becomes 1) while + * keeping the original `$to`. The narrowed selection is computed on a derived + * state and never dispatched on its own, so the whole operation is a single + * dispatch / single undo step. + * + * Shift-Tab / `liftListItem` has no equivalent limitation (it handles ranges and + * the first-item case correctly), so only Tab needs this wrapper. + */ +function sinkListItemRange(itemType: NodeType): Command { + return (state, dispatch) => { + // Normal path — cursor or range not starting at the first item. This also + // covers the genuine no-op for a collapsed cursor in the first item: stock + // returns false and the fallback guards below also return false. + if (pmSinkListItem(itemType)(state, dispatch)) return true; + + const { $from, $to } = state.selection; + const range = $from.blockRange( + $to, + (node) => node.childCount > 0 && node.firstChild?.type === itemType, + ); + // Clean false (no dispatch) when the fallback does not apply: no list range, + // the range does not start at the first item, fewer than two items are + // selected, or the item type does not match (C2). + if (!range) return false; + if (range.startIndex !== 0) return false; + if (range.endIndex - range.startIndex < 2) return false; + if (range.parent.child(range.startIndex).type !== itemType) return false; + + // Move $from into the second selected item, keep $to in the last selected + // item (C1 — do not collapse onto the second item). +2 steps over the + // + open tokens into inline content; `between` snaps + // to a valid text position if the item does not start with a paragraph. + const secondItemStart = + range.start + range.parent.child(range.startIndex).nodeSize + 2; + const narrowed = state.apply( + state.tr.setSelection( + TextSelection.between(state.doc.resolve(secondItemStart), $to), + ), + ); + return pmSinkListItem(itemType)(narrowed, dispatch); + }; +} /** * Shared list keymap with proper "double-Enter exits list" behaviour. @@ -19,7 +79,8 @@ import { ListItem, TaskItem } from "@tiptap/extension-list"; * empty items are unaffected because `splitListItem` handles them correctly * and returns true. * - * Tab / Shift-Tab indent / dedent the item. + * Tab indents the item(s) — see `sinkListItemRange` for the multi-item + * first-item handling. Shift-Tab dedents via the stock command. */ function listItemKeymap(editor: Editor, name: string) { return { @@ -28,7 +89,13 @@ function listItemKeymap(editor: Editor, name: string) { () => commands.splitListItem(name), () => commands.liftListItem(name), ]), - Tab: () => editor.commands.sinkListItem(name), + Tab: () => { + const itemType = editor.schema.nodes[name]; + if (!itemType) return false; + return sinkListItemRange(itemType)(editor.state, (tr) => + editor.view.dispatch(tr), + ); + }, "Shift-Tab": () => editor.commands.liftListItem(name), }; }