diff --git a/packages/views/editor/extensions/mention-suggestion.test.tsx b/packages/views/editor/extensions/mention-suggestion.test.tsx index 15ec6387a..63be6cb20 100644 --- a/packages/views/editor/extensions/mention-suggestion.test.tsx +++ b/packages/views/editor/extensions/mention-suggestion.test.tsx @@ -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(); + const items: MentionItem[] = [ + { id: "i-1", label: "MUL-1", type: "issue" }, + { id: "i-2", label: "MUL-2", type: "issue" }, + ]; + + render( + + + , + ); + + 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(); + const items: MentionItem[] = [{ id: "i-1", label: "MUL-1", type: "issue" }]; + + render( + + + , + ); + + 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(); + + render(); + + 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 diff --git a/packages/views/editor/extensions/mention-suggestion.tsx b/packages/views/editor/extensions/mention-suggestion.tsx index b246bb068..f2a443826 100644 --- a/packages/views/editor/extensions/mention-suggestion.tsx +++ b/packages/views/editor/extensions/mention-suggestion.tsx @@ -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( 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; diff --git a/packages/views/editor/extensions/slash-command-suggestion.test.tsx b/packages/views/editor/extensions/slash-command-suggestion.test.tsx index 549078331..ff5fefe82 100644 --- a/packages/views/editor/extensions/slash-command-suggestion.test.tsx +++ b/packages/views/editor/extensions/slash-command-suggestion.test.tsx @@ -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(); + const command = vi.fn(); + const selectableItems: SlashCommandItem[] = [ + { id: "s1", label: "deploy", description: "Ship changes" }, + { id: "s2", label: "review", description: "Review code" }, + ]; + + render( + + + , + ); + + 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(); + + render( + + + , + ); + + expect( + ref.current?.onKeyDown({ + event: new KeyboardEvent("keydown", { key: "Tab" }), + }), + ).toBe(false); + }); }); describe("SlashCommandList empty states", () => { diff --git a/packages/views/editor/extensions/slash-command-suggestion.tsx b/packages/views/editor/extensions/slash-command-suggestion.tsx index 8eda201b4..ff8aa958f 100644 --- a/packages/views/editor/extensions/slash-command-suggestion.tsx +++ b/packages/views/editor/extensions/slash-command-suggestion.tsx @@ -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; diff --git a/packages/views/editor/extensions/suggestion-popup.test.tsx b/packages/views/editor/extensions/suggestion-popup.test.tsx index efce5b9c3..4ceb3bac0 100644 --- a/packages/views/editor/extensions/suggestion-popup.test.tsx +++ b/packages/views/editor/extensions/suggestion-popup.test.tsx @@ -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( + 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 ( +
+ {items.map((item) => ( + + ))} +
+ ); + }, +); + +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({ + editor: this.editor, + char: "@", + pluginKey, + items: () => [item], + command: ({ editor: ed, range, props }) => { + ed.commands.insertContentAt(range, `@${props.label}`); + }, + render: createSuggestionPopupRender({ + pluginKey, + component: AcceptOnTabList, + getProps: (props: SuggestionProps) => ({ + 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(); + 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
    . 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(/
      { + 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
        ). + await waitFor(() => { + expect(ed.getText()).toContain("@Alice"); + }); + expect(ed.getHTML().match(/
          { + 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"); + }); +}); diff --git a/packages/views/editor/extensions/suggestion-popup.tsx b/packages/views/editor/extensions/suggestion-popup.tsx index 84e327f30..834af16be 100644 --- a/packages/views/editor/extensions/suggestion-popup.tsx +++ b/packages/views/editor/extensions/suggestion-popup.tsx @@ -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,