From 28b6f368b99eda7b74707efe381dd6c193efa176 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:37:42 +0800 Subject: [PATCH] fix(editor): index @mention selection by rendered order (MUL-3607) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @mention popup navigated and committed by indexing the flat `displayItems` array, but rendered rows in the re-bucketed order from `groupItems()` (current → recent → search → users → issues). In chat (context mode) the async server-search results are appended to the end of `displayItems` yet tagged `group:"search"`, so `groupItems()` hoists them near the top. The highlighted row and the committed item then point at different entries — you select one target but mention its neighbour (the reported "@bohan picks the next one" off-by-one). Make the flattened grouped order (`orderedItems`) the single index space for `selectedIndex`, arrow keys, Enter, and clicks, so the highlighted row is always the committed item. Plain issue-comment mentions (default mode) were already safe — no group tags means `groupItems()` is order-preserving — and stay unchanged. Adds a regression test asserting highlighted row == committed item when the list is reordered by a hoisted search result. Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- .../extensions/mention-suggestion.test.tsx | 57 ++++++++++++++++++- .../editor/extensions/mention-suggestion.tsx | 36 ++++++++---- 2 files changed, 81 insertions(+), 12 deletions(-) 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);