mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-06 22:09:44 +02:00
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 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -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<string, () => 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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
// <listItem> + <paragraph> 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),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user