diff --git a/packages/views/editor/extensions/mention-suggestion.test.tsx b/packages/views/editor/extensions/mention-suggestion.test.tsx index 291837f5f..61ce80948 100644 --- a/packages/views/editor/extensions/mention-suggestion.test.tsx +++ b/packages/views/editor/extensions/mention-suggestion.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import { createRef, type ReactNode } from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { workspaceKeys } from "@multica/core/workspace/queries"; @@ -209,6 +209,61 @@ describe("createMentionSuggestion", () => { ).toBe(true); }); + // MUL-3607: groupItems() re-buckets the list (current → recent → search → + // users → issues), so a server "search" result that arrives at the END of + // the data array is rendered NEAR THE TOP. Keyboard nav, Enter, and clicks + // must follow the rendered order — otherwise the highlighted row and the + // committed item drift apart and you mention the neighbour of who you picked. + // (The invariant is type-agnostic; issue rows are used here because they + // render without workspace/avatar context. The chat @bohan case is the same + // bug — a person sitting below a hoisted search result.) + it("commits the highlighted row, not its neighbour, when groups reorder the list", () => { + const command = vi.fn<(item: MentionItem) => void>(); + const ref = createRef(); + + // A plain issue (issues bucket) plus a server-style search hit (search + // bucket). Data order is [MUL-2, MUL-1] but groupItems hoists the search + // row, so the RENDERED order is [MUL-1, MUL-2]. + const items: MentionItem[] = [ + { id: "i-plain", label: "MUL-2", type: "issue" }, + { id: "i-search", label: "MUL-1", type: "issue", group: "search" }, + ]; + + render( + + + , + ); + + const highlightedLabel = () => { + const buttons = Array.from(document.querySelectorAll("button")); + return buttons.find((b) => b.classList.contains("bg-accent"))?.textContent ?? ""; + }; + + const press = (key: string) => + act(() => { + ref.current?.onKeyDown({ event: new KeyboardEvent("keydown", { key }) }); + }); + + // First rendered row is the hoisted search result. Enter must commit it, + // not the issue that sits first in the data array. + const firstHighlighted = highlightedLabel(); + expect(firstHighlighted).toBe("MUL-1"); + press("Enter"); + expect(command).toHaveBeenCalledTimes(1); + expect(command.mock.calls[0]?.[0]?.label).toBe(firstHighlighted); + + command.mockClear(); + + // Arrow down one row, then Enter — still commits exactly the highlighted row. + press("ArrowDown"); + const secondHighlighted = highlightedLabel(); + expect(secondHighlighted).toBe("MUL-2"); + press("Enter"); + expect(command).toHaveBeenCalledTimes(1); + expect(command.mock.calls[0]?.[0]?.label).toBe(secondHighlighted); + }); + it("hides personal agents owned by someone else from a regular member", () => { const qc = fakeQc({ members: [ diff --git a/packages/views/editor/extensions/mention-suggestion.tsx b/packages/views/editor/extensions/mention-suggestion.tsx index 54a0c00e4..6417d31fa 100644 --- a/packages/views/editor/extensions/mention-suggestion.tsx +++ b/packages/views/editor/extensions/mention-suggestion.tsx @@ -232,9 +232,24 @@ export const MentionList = forwardRef( return mergeMentionItems(items, currentServerItems).slice(0, MAX_ITEMS); }, [items, normalizedQuery, searchedQuery, serverItems]); + // Keyboard navigation and selection must index the SAME order the popup + // renders. groupItems() re-buckets displayItems (current → recent → search + // → users → issues), so a later-in-data item — e.g. an async server + // "search" result appended to the end of displayItems — is hoisted into an + // earlier bucket. Indexing the pre-group displayItems then drifts from the + // rendered rows: the highlighted row and the committed item point at + // different entries (you mention the neighbour of who you picked). + // orderedItems is the flattened render order and is the single index space + // for selectedIndex, arrow keys, Enter, and clicks. + const groups = useMemo(() => groupItems(displayItems), [displayItems]); + const orderedItems = useMemo( + () => groups.flatMap((group) => group.items), + [groups], + ); + useEffect(() => { setSelectedIndex(0); - }, [displayItems]); + }, [orderedItems]); useEffect(() => { itemRefs.current[selectedIndex]?.scrollIntoView({ block: "nearest" }); @@ -242,13 +257,13 @@ export const MentionList = forwardRef( const selectItem = useCallback( (index: number) => { - const item = displayItems[index]; + const item = orderedItems[index]; if (!item) return; const wsId = getCurrentWsId(); if (wsId) recordMentionUsage(wsId, item); command(item); }, - [displayItems, command], + [orderedItems, command], ); useImperativeHandle(ref, () => ({ @@ -257,19 +272,19 @@ export const MentionList = forwardRef( // those keys belong to the IME (Enter commits composition, etc). if (isImeComposing(event)) return false; if (event.key === "ArrowUp") { - if (displayItems.length === 0) return true; + if (orderedItems.length === 0) return true; setSelectedIndex( - (i) => (i + displayItems.length - 1) % displayItems.length, + (i) => (i + orderedItems.length - 1) % orderedItems.length, ); return true; } if (event.key === "ArrowDown") { - if (displayItems.length === 0) return true; - setSelectedIndex((i) => (i + 1) % displayItems.length); + if (orderedItems.length === 0) return true; + setSelectedIndex((i) => (i + 1) % orderedItems.length); return true; } if (event.key === "Enter") { - if (displayItems.length === 0) return true; + if (orderedItems.length === 0) return true; selectItem(selectedIndex); return true; } @@ -277,7 +292,7 @@ export const MentionList = forwardRef( }, })); - if (displayItems.length === 0) { + if (orderedItems.length === 0) { const isWaitingForServer = normalizedQuery !== "" && (isSearching || searchedQuery !== normalizedQuery); @@ -291,8 +306,7 @@ export const MentionList = forwardRef( ); } - const groups = groupItems(displayItems); - const hasContextGroups = displayItems.some((item) => item.group === "current" || item.group === "recent"); + const hasContextGroups = orderedItems.some((item) => item.group === "current" || item.group === "recent"); const contextLayout = hasContextGroups; const groupLabel = (label: string): string => { if (label === "Current") return t(($) => $.mention.group_current);