mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
feat(editor): accept highlighted composer suggestion on Tab (MUL-3685) (#4570)
* feat(editor): accept highlighted composer suggestion on Tab Plain Tab now accepts the highlighted mention / slash-command suggestion, matching Enter, across every composer built on the shared TipTap editor (chat, issue description, comment/reply). A single shared isPickerAcceptKey predicate centralizes the accept-key policy so the two picker lists stay in sync instead of each re-deciding what counts as accept. Shift+Tab and Ctrl/Cmd/Alt+Tab are intentionally NOT accept keys, so reverse focus navigation and OS window switching are preserved. When no picker is open, Tab keeps its existing behavior (list indent / focus traversal). Adds unit coverage for both picker lists plus a plugin-order guard that fires Tab through real ProseMirror dispatch with the caret inside a list item, proving the suggestion layer outranks PatchedListItem's Tab -> sinkListItem. Scope: web/desktop shared composer only; mobile and the generic combobox are untouched. Refs: MUL-3685 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * test(editor): make Tab/list-item priority guard actually exercise sinkListItem The guard placed the caret in the first (only) bullet item, where sinkListItem is a no-op (no preceding sibling to nest under), so the test passed regardless of whether the suggestion layer intercepted Tab. Rebuild the fixture as a two-item list with the caret in the SECOND item, where Tab -> sinkListItem can fire, and add a sanity control proving a bare Tab (no picker) does sink that item — so the accept-wins assertion is meaningful. Production code unchanged. Refs: MUL-3685 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -209,6 +209,63 @@ describe("createMentionSuggestion", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
// MUL-3685: plain Tab accepts the highlighted row exactly like Enter.
|
||||
it("accepts the highlighted row on plain Tab, like Enter", () => {
|
||||
const command = vi.fn<(item: MentionItem) => void>();
|
||||
const ref = createRef<MentionListRef>();
|
||||
const items: MentionItem[] = [
|
||||
{ id: "i-1", label: "MUL-1", type: "issue" },
|
||||
{ id: "i-2", label: "MUL-2", type: "issue" },
|
||||
];
|
||||
|
||||
render(
|
||||
<I18nWrapper>
|
||||
<MentionList ref={ref} items={items} query="" command={command} />
|
||||
</I18nWrapper>,
|
||||
);
|
||||
|
||||
const handled = ref.current?.onKeyDown({
|
||||
event: new KeyboardEvent("keydown", { key: "Tab" }),
|
||||
});
|
||||
|
||||
expect(handled).toBe(true);
|
||||
expect(command).toHaveBeenCalledTimes(1);
|
||||
expect(command.mock.calls[0]?.[0]?.label).toBe("MUL-1");
|
||||
});
|
||||
|
||||
// Shift+Tab and any modifier+Tab stay focus navigation — they must NOT
|
||||
// accept, so the picker never traps reverse Tab traversal or OS switching.
|
||||
it("does not accept on Shift+Tab or modifier+Tab", () => {
|
||||
const command = vi.fn<(item: MentionItem) => void>();
|
||||
const ref = createRef<MentionListRef>();
|
||||
const items: MentionItem[] = [{ id: "i-1", label: "MUL-1", type: "issue" }];
|
||||
|
||||
render(
|
||||
<I18nWrapper>
|
||||
<MentionList ref={ref} items={items} query="" command={command} />
|
||||
</I18nWrapper>,
|
||||
);
|
||||
|
||||
const press = (init: KeyboardEventInit) =>
|
||||
ref.current?.onKeyDown({ event: new KeyboardEvent("keydown", init) });
|
||||
|
||||
expect(press({ key: "Tab", shiftKey: true })).toBe(false);
|
||||
expect(press({ key: "Tab", metaKey: true })).toBe(false);
|
||||
expect(press({ key: "Tab", ctrlKey: true })).toBe(false);
|
||||
expect(press({ key: "Tab", altKey: true })).toBe(false);
|
||||
expect(command).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("captures Tab while the popup has no selectable items, like Enter", () => {
|
||||
const ref = createRef<MentionListRef>();
|
||||
|
||||
render(<I18nWrapper><MentionList ref={ref} items={[]} query="协作" command={vi.fn()} /></I18nWrapper>);
|
||||
|
||||
expect(
|
||||
ref.current?.onKeyDown({ event: new KeyboardEvent("keydown", { key: "Tab" }) }),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
// MUL-3607: groupItems() re-buckets the list (current → recent → search →
|
||||
// users → issues), so an item that sits LATER in the data array can render
|
||||
// NEAR THE TOP. Selection must follow the rendered order — otherwise the
|
||||
|
||||
@@ -42,7 +42,7 @@ import {
|
||||
sortUserItemsByRecency,
|
||||
} from "./mention-recency";
|
||||
import { matchesPinyin } from "./pinyin-match";
|
||||
import { createSuggestionPopupRender } from "./suggestion-popup";
|
||||
import { createSuggestionPopupRender, isPickerAcceptKey } from "./suggestion-popup";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
@@ -288,7 +288,9 @@ export const MentionList = forwardRef<MentionListRef, MentionListProps>(
|
||||
setSelectedKey(mentionItemKey(orderedItems[next]!));
|
||||
return true;
|
||||
}
|
||||
if (event.key === "Enter") {
|
||||
// Enter is the canonical accept; plain Tab is an additive alias (see
|
||||
// isPickerAcceptKey). Shift/modifier+Tab fall through to focus nav.
|
||||
if (isPickerAcceptKey(event)) {
|
||||
if (orderedItems.length === 0) return true;
|
||||
selectItem(orderedItems[selectedIndex]);
|
||||
return true;
|
||||
|
||||
@@ -306,6 +306,54 @@ describe("SlashCommandList keyboard handling", () => {
|
||||
).toBe(true);
|
||||
expect(command).toHaveBeenCalledWith(selectableItems[0]);
|
||||
});
|
||||
|
||||
// MUL-3685: plain Tab accepts the highlighted item like Enter; Shift+Tab and
|
||||
// modifier+Tab fall through so reverse focus / OS switching are preserved.
|
||||
it("accepts the highlighted item on plain Tab, ignoring Shift/modifier+Tab", () => {
|
||||
const ref = createRef<SlashCommandListRef>();
|
||||
const command = vi.fn();
|
||||
const selectableItems: SlashCommandItem[] = [
|
||||
{ id: "s1", label: "deploy", description: "Ship changes" },
|
||||
{ id: "s2", label: "review", description: "Review code" },
|
||||
];
|
||||
|
||||
render(
|
||||
<I18nWrapper>
|
||||
<SlashCommandList
|
||||
ref={ref}
|
||||
items={selectableItems}
|
||||
query=""
|
||||
command={command}
|
||||
/>
|
||||
</I18nWrapper>,
|
||||
);
|
||||
|
||||
const press = (init: KeyboardEventInit) =>
|
||||
ref.current?.onKeyDown({ event: new KeyboardEvent("keydown", init) });
|
||||
|
||||
expect(press({ key: "Tab", shiftKey: true })).toBe(false);
|
||||
expect(press({ key: "Tab", metaKey: true })).toBe(false);
|
||||
expect(command).not.toHaveBeenCalled();
|
||||
|
||||
expect(press({ key: "Tab" })).toBe(true);
|
||||
expect(command).toHaveBeenCalledWith(selectableItems[0]);
|
||||
});
|
||||
|
||||
it("lets Tab fall through when there are no selectable items, like Enter", () => {
|
||||
const ref = createRef<SlashCommandListRef>();
|
||||
|
||||
render(
|
||||
<I18nWrapper>
|
||||
<SlashCommandList ref={ref} items={[]} query="" command={vi.fn()} />
|
||||
</I18nWrapper>,
|
||||
);
|
||||
|
||||
expect(
|
||||
ref.current?.onKeyDown({
|
||||
event: new KeyboardEvent("keydown", { key: "Tab" }),
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("SlashCommandList empty states", () => {
|
||||
|
||||
@@ -19,7 +19,7 @@ import { isImeComposing } from "@multica/core/utils";
|
||||
import { workspaceKeys } from "@multica/core/workspace/queries";
|
||||
import type { Agent, MemberWithUser } from "@multica/core/types";
|
||||
import { useT } from "../../i18n";
|
||||
import { createSuggestionPopupRender } from "./suggestion-popup";
|
||||
import { createSuggestionPopupRender, isPickerAcceptKey } from "./suggestion-popup";
|
||||
|
||||
const MAX_ITEMS = 20;
|
||||
|
||||
@@ -95,7 +95,9 @@ export const SlashCommandList = forwardRef<
|
||||
setSelectedIndex((i) => (i + 1) % items.length);
|
||||
return true;
|
||||
}
|
||||
if (event.key === "Enter") {
|
||||
// Enter is the canonical accept; plain Tab is an additive alias (see
|
||||
// isPickerAcceptKey). Shift/modifier+Tab fall through to focus nav.
|
||||
if (isPickerAcceptKey(event)) {
|
||||
if (items.length === 0) return false;
|
||||
selectItem(selectedIndex);
|
||||
return true;
|
||||
|
||||
@@ -6,7 +6,8 @@ import { PluginKey } from "@tiptap/pm/state";
|
||||
import { forwardRef, useImperativeHandle } from "react";
|
||||
import { afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { createSuggestionPopupRender } from "./suggestion-popup";
|
||||
import { createSuggestionPopupRender, isPickerAcceptKey } from "./suggestion-popup";
|
||||
import { PatchedListItem } from "./list-item";
|
||||
|
||||
interface TestItem {
|
||||
id: string;
|
||||
@@ -214,3 +215,178 @@ describe("createSuggestionPopupRender", () => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// isPickerAcceptKey — the shared accept-key policy (MUL-3685)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("isPickerAcceptKey", () => {
|
||||
const accepts = (init: KeyboardEventInit) =>
|
||||
isPickerAcceptKey(new KeyboardEvent("keydown", init));
|
||||
|
||||
it("treats Enter and plain Tab as accept keys", () => {
|
||||
expect(accepts({ key: "Enter" })).toBe(true);
|
||||
expect(accepts({ key: "Tab" })).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps Enter an accept key regardless of modifiers (Mod-Enter unchanged)", () => {
|
||||
expect(accepts({ key: "Enter", metaKey: true })).toBe(true);
|
||||
});
|
||||
|
||||
it("does not treat Shift+Tab or Ctrl/Cmd/Alt+Tab as accept keys", () => {
|
||||
expect(accepts({ key: "Tab", shiftKey: true })).toBe(false);
|
||||
expect(accepts({ key: "Tab", ctrlKey: true })).toBe(false);
|
||||
expect(accepts({ key: "Tab", metaKey: true })).toBe(false);
|
||||
expect(accepts({ key: "Tab", altKey: true })).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores unrelated keys", () => {
|
||||
expect(accepts({ key: "ArrowDown" })).toBe(false);
|
||||
expect(accepts({ key: "Escape" })).toBe(false);
|
||||
expect(accepts({ key: "a" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plugin-order guard (MUL-3685): when a suggestion is open inside a list item,
|
||||
// the suggestion layer's Tab handling must outrank PatchedListItem's
|
||||
// Tab -> sinkListItem keymap. This replicates the real extension ordering and
|
||||
// fires Tab through ProseMirror's actual handleKeyDown dispatch, so a future
|
||||
// reorder that lets the list keymap win is caught here.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const AcceptOnTabList = forwardRef<TestListRef, TestListProps>(
|
||||
function AcceptOnTabList({ items, command }, ref) {
|
||||
useImperativeHandle(ref, () => ({
|
||||
// Mirror the real mention/slash lists: accept the highlighted row on the
|
||||
// shared accept keys (Enter / plain Tab), fall through otherwise.
|
||||
onKeyDown: ({ event }) => {
|
||||
if (isPickerAcceptKey(event)) {
|
||||
command(items[0]!);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
},
|
||||
}));
|
||||
|
||||
return (
|
||||
<div data-testid="suggestion-popup">
|
||||
{items.map((item) => (
|
||||
<button key={item.id} type="button" onClick={() => command(item)}>
|
||||
{item.label}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
function makeListEditor() {
|
||||
const pluginKey = new PluginKey("test-list-suggestion");
|
||||
const item: TestItem = { id: "u1", label: "Alice" };
|
||||
|
||||
const TestListSuggestionExtension = Extension.create({
|
||||
name: "testListSuggestion",
|
||||
addProseMirrorPlugins() {
|
||||
return [
|
||||
Suggestion<TestItem, TestItem>({
|
||||
editor: this.editor,
|
||||
char: "@",
|
||||
pluginKey,
|
||||
items: () => [item],
|
||||
command: ({ editor: ed, range, props }) => {
|
||||
ed.commands.insertContentAt(range, `@${props.label}`);
|
||||
},
|
||||
render: createSuggestionPopupRender<TestItem, TestItem, TestListRef, TestListProps>({
|
||||
pluginKey,
|
||||
component: AcceptOnTabList,
|
||||
getProps: (props: SuggestionProps<TestItem, TestItem>) => ({
|
||||
items: props.items,
|
||||
command: props.command,
|
||||
}),
|
||||
onKeyDown: (ref, props) => ref?.onKeyDown(props) ?? false,
|
||||
}),
|
||||
}),
|
||||
];
|
||||
},
|
||||
});
|
||||
|
||||
editor = new Editor({
|
||||
// Mirror the real wiring (extensions/index.ts): StarterKit's stock list
|
||||
// item is disabled in favour of PatchedListItem, which binds
|
||||
// Tab -> sinkListItem / Shift-Tab -> liftListItem.
|
||||
extensions: [
|
||||
StarterKit.configure({ listItem: false }),
|
||||
PatchedListItem,
|
||||
TestListSuggestionExtension,
|
||||
],
|
||||
content: "",
|
||||
});
|
||||
render(<EditorContent editor={editor} />);
|
||||
return editor;
|
||||
}
|
||||
|
||||
// Build a two-item bullet list with the caret in the SECOND item. sinkListItem
|
||||
// can only indent an item that has a PRECEDING sibling, so the cursor must be
|
||||
// in item 2 for Tab -> sinkListItem to actually fire (Howard, MUL-3685 review):
|
||||
// in the first item sink is a no-op, and the guard would pass even if the
|
||||
// suggestion layer did nothing. Built from empty so `@` lands at the start of
|
||||
// item 2's paragraph (a valid suggestion boundary) without HTML-parse quirks.
|
||||
async function buildTwoItemList(ed: Editor) {
|
||||
await act(async () => {
|
||||
ed.commands.focus();
|
||||
ed.commands.toggleBulletList();
|
||||
ed.commands.insertContent("first");
|
||||
ed.commands.splitListItem("listItem");
|
||||
});
|
||||
}
|
||||
|
||||
async function openPickerInSecondListItem(ed: Editor) {
|
||||
await buildTwoItemList(ed);
|
||||
await triggerSuggestion(ed, "@a");
|
||||
}
|
||||
|
||||
describe("suggestion Tab priority over the list-item keymap", () => {
|
||||
it("sanity: a bare Tab in the second list item DOES sink it (guard is sink-capable)", async () => {
|
||||
const ed = makeListEditor();
|
||||
await buildTwoItemList(ed);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(ed.view.dom, { key: "Tab" });
|
||||
});
|
||||
|
||||
// No picker open: PatchedListItem's Tab -> sinkListItem nests item 2 under
|
||||
// item 1, producing a second <ul>. This proves the doc/selection actually
|
||||
// lets sinkListItem fire, so the accept-wins assertion below is meaningful
|
||||
// rather than passing because sink was a no-op.
|
||||
expect(ed.getHTML().match(/<ul/g)?.length ?? 0).toBe(2);
|
||||
});
|
||||
|
||||
it("accepts the highlighted row on Tab even when Tab would otherwise sink the item", async () => {
|
||||
const ed = makeListEditor();
|
||||
await openPickerInSecondListItem(ed);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(ed.view.dom, { key: "Tab" });
|
||||
});
|
||||
|
||||
// Accept won over PatchedListItem's Tab -> sinkListItem: the mention text
|
||||
// was inserted and item 2 was NOT nested (still a single <ul>).
|
||||
await waitFor(() => {
|
||||
expect(ed.getText()).toContain("@Alice");
|
||||
});
|
||||
expect(ed.getHTML().match(/<ul/g)?.length ?? 0).toBe(1);
|
||||
});
|
||||
|
||||
it("does not accept on Shift+Tab inside a list item — reverse nav is preserved", async () => {
|
||||
const ed = makeListEditor();
|
||||
await openPickerInSecondListItem(ed);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(ed.view.dom, { key: "Tab", shiftKey: true });
|
||||
});
|
||||
|
||||
// Shift+Tab is not an accept key, so the suggestion never committed.
|
||||
expect(ed.getText()).not.toContain("@Alice");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,32 @@ import { ReactRenderer } from "@tiptap/react";
|
||||
import { exitSuggestion, type SuggestionKeyDownProps, type SuggestionProps } from "@tiptap/suggestion";
|
||||
import type { PluginKey } from "@tiptap/pm/state";
|
||||
|
||||
/**
|
||||
* Keys that accept the currently highlighted suggestion row.
|
||||
*
|
||||
* `Enter` is the canonical accept (WAI-ARIA combobox guidance). Plain `Tab` is
|
||||
* an additive convenience that matches terminal / CLI / editor completion
|
||||
* muscle memory (MUL-3685). `Shift+Tab` and any `Ctrl/Cmd/Alt + Tab` are
|
||||
* deliberately NOT accept keys: they stay reverse focus navigation / OS window
|
||||
* switching, so standard keyboard accessibility is preserved.
|
||||
*
|
||||
* Centralizing the rule here keeps every picker built on
|
||||
* `createSuggestionPopupRender` (mention, slash-skill, builtin command, and any
|
||||
* future suggestion list) consistent instead of each list re-deciding what
|
||||
* counts as "accept". Callers use it in place of a bare `event.key === "Enter"`
|
||||
* check, so `Tab` becomes a strict alias of `Enter` inside their accept branch.
|
||||
*/
|
||||
export function isPickerAcceptKey(event: KeyboardEvent): boolean {
|
||||
if (event.key === "Enter") return true;
|
||||
return (
|
||||
event.key === "Tab" &&
|
||||
!event.shiftKey &&
|
||||
!event.ctrlKey &&
|
||||
!event.metaKey &&
|
||||
!event.altKey
|
||||
);
|
||||
}
|
||||
|
||||
interface SuggestionPopupRenderOptions<
|
||||
TItem,
|
||||
TSelected = TItem,
|
||||
|
||||
Reference in New Issue
Block a user