From 248c0d23288b2110e643599950d1f529df4b0bf0 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Wed, 13 May 2026 11:38:47 +0100 Subject: [PATCH] fix(admin): don't steal slash menu selection on stationary pointer When the slash menu opens, it renders right at the text cursor. If the user's mouse pointer happens to sit at the same coords -- which is the default in CI test environments where pointer position persists across tests in the same file -- the menu's items received pointerenter the moment they appeared and called setSelectedIndex, overriding the keyboard-driven default selection of 0. In CI this manifested as 5 deterministic-on-slow-runner failures in tests/editor/slash-menu.test.tsx (highlights-the-first-item-by-default, ArrowDown/ArrowUp navigation, wrap-around, Enter-converts-to-heading). Diagnostic dumps from #1010 showed the menu rendering with item 3/4/5 selected instead of the expected item -- a constant offset that lined up with the runner's lingering pointer Y coord. Gate the per-item onMouseEnter handler on a hasMouseMovedRef that only flips to true after a pointermove event on the menu container. Reset the ref each time the menu closes. This preserves mouse-hover-selects once the user is actually moving the pointer over the menu, but ignores pointer-was-already-here, which is what we want. The existing 'highlights item on mouse hover' test relies on userEvent.hover, which under Playwright teleports the cursor to the target and fires pointerenter but not pointermove -- so the test now explicitly dispatches a pointermove on the menu container first, matching the new contract. Reverts the temporary diagnostic instrumentation from #1010 (workflow artifact upload + diagWaitFor helpers in slash-menu.test.tsx) now that we have the fix. Closes #1010 follow-up. --- .changeset/fix-slash-menu-mouseenter-race.md | 6 ++ .github/workflows/ci.yml | 12 --- .../src/components/PortableTextEditor.tsx | 28 ++++++- .../admin/tests/editor/slash-menu.test.tsx | 75 ++++--------------- 4 files changed, 48 insertions(+), 73 deletions(-) create mode 100644 .changeset/fix-slash-menu-mouseenter-race.md diff --git a/.changeset/fix-slash-menu-mouseenter-race.md b/.changeset/fix-slash-menu-mouseenter-race.md new file mode 100644 index 0000000000..29d9bc00de --- /dev/null +++ b/.changeset/fix-slash-menu-mouseenter-race.md @@ -0,0 +1,6 @@ +--- +"@emdash-cms/admin": patch +"emdash": patch +--- + +Fixes the slash command menu's initial selection getting overridden when the menu opens under a stationary pointer. The menu items previously reacted to `mouseenter` unconditionally, so an item rendered beneath the cursor would steal selection from the keyboard default before any user interaction. Mouse-hover-selects still works, but only after the user actually moves the pointer over the menu. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1692786cc8..0a401f5f29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,18 +204,6 @@ jobs: - run: pnpm exec playwright install --with-deps chromium if: steps.playwright-cache.outputs.cache-hit != 'true' - run: pnpm run --filter @emdash-cms/admin test - # Upload vitest browser-mode failure screenshots so we can diagnose - # CI-only flakes (e.g. the recurring slash-menu race). Only runs on - # failure, only uploads the screenshot directories. - - name: Upload failure screenshots - if: failure() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: browser-test-screenshots - path: | - packages/admin/tests/**/__screenshots__/** - if-no-files-found: ignore - retention-days: 1 test-e2e-rollup: name: E2E Tests diff --git a/packages/admin/src/components/PortableTextEditor.tsx b/packages/admin/src/components/PortableTextEditor.tsx index 898ef110bd..35db819132 100644 --- a/packages/admin/src/components/PortableTextEditor.tsx +++ b/packages/admin/src/components/PortableTextEditor.tsx @@ -1097,6 +1097,22 @@ function SlashCommandMenu({ } }, [state.selectedIndex, state.isOpen]); + // Track whether the mouse has actually moved since the menu opened. + // The menu typically opens right at the text cursor, which may sit under + // a stationary mouse pointer. Reacting to mouseenter immediately would + // reset the selection to whichever item happens to be under the pointer + // the moment the menu renders -- overriding the keyboard-driven default + // (selectedIndex: 0) and any subsequent arrow-key navigation. + // + // Only flip the gate on mousemove, which fires only on real pointer + // movement, not on elements appearing under a stationary pointer. + const hasMouseMovedRef = React.useRef(false); + React.useEffect(() => { + if (!state.isOpen) { + hasMouseMovedRef.current = false; + } + }, [state.isOpen]); + if (!state.isOpen) return null; return createPortal( @@ -1107,6 +1123,9 @@ function SlashCommandMenu({ }} style={floatingStyles} className="z-[100] rounded-lg border bg-kumo-overlay p-1 shadow-lg min-w-[220px] max-h-[300px] overflow-y-auto" + onPointerMove={() => { + hasMouseMovedRef.current = true; + }} > {state.items.length === 0 ? (

{t`No results`}

@@ -1123,7 +1142,14 @@ function SlashCommandMenu({ : "hover:bg-kumo-tint/50", )} onClick={() => onCommand(item)} - onMouseEnter={() => setSelectedIndex(index)} + onMouseEnter={() => { + // Only react if the user has actually moved the + // mouse since the menu opened -- not when items + // appear under a stationary pointer. + if (hasMouseMovedRef.current) { + setSelectedIndex(index); + } + }} >
diff --git a/packages/admin/tests/editor/slash-menu.test.tsx b/packages/admin/tests/editor/slash-menu.test.tsx index 94b068e89f..90c454fc7c 100644 --- a/packages/admin/tests/editor/slash-menu.test.tsx +++ b/packages/admin/tests/editor/slash-menu.test.tsx @@ -189,57 +189,6 @@ function isItemSelected(el: HTMLElement): boolean { return el.className.split(WHITESPACE_SPLIT_REGEX).includes("bg-kumo-tint"); } -/** - * DIAGNOSTIC (temporary, see #1004 follow-up): dump everything useful about - * the slash menu state to console.log. Called from a catch block when - * vi.waitFor times out, so the failing CI log contains the actual menu - * contents and DOM state at the moment of failure. Remove once the - * underlying race is understood and fixed. - */ -function dumpMenuState(label: string): void { - const menu = getSlashMenu(); - const lines: string[] = [ - `[slash-menu diag] === ${label} ===`, - `[slash-menu diag] menu present: ${menu !== null}`, - `[slash-menu diag] activeElement: ${document.activeElement?.tagName} (class=${document.activeElement?.className?.slice(0, 80)})`, - `[slash-menu diag] body > div count: ${document.querySelectorAll("body > div").length}`, - ]; - if (menu) { - const items = getSlashMenuItems(menu); - lines.push(`[slash-menu diag] items rendered: ${items.length}`); - lines.push(`[slash-menu diag] menu textContent length: ${menu.textContent?.length ?? 0}`); - lines.push(`[slash-menu diag] menu first 200 chars: ${menu.textContent?.slice(0, 200)}`); - items.forEach((el, i) => { - const dataIndex = el.getAttribute("data-index"); - const selected = isItemSelected(el); - const classes = el.className; - lines.push( - `[slash-menu diag] item ${i} (data-index=${dataIndex}) selected=${selected} classes=${classes.slice(0, 200)}`, - ); - }); - // menu.outerHTML truncated to 2000 chars - lines.push(`[slash-menu diag] menu outerHTML: ${menu.outerHTML.slice(0, 2000)}`); - } - console.log(lines.join("\n")); -} - -/** - * DIAGNOSTIC wrapper around vi.waitFor that dumps menu state on timeout. - * Same semantics as vi.waitFor; only adds logging on failure. - */ -async function diagWaitFor( - label: string, - predicate: () => T | Promise, - options?: { timeout?: number; interval?: number }, -): Promise { - try { - return await vi.waitFor(predicate, options); - } catch (err) { - dumpMenuState(label); - throw err; - } -} - // ============================================================================= // Slash Command Menu // ============================================================================= @@ -338,8 +287,7 @@ describe("Slash Command Menu", () => { await waitForSlashMenu(); - await diagWaitFor( - "highlights the first item by default", + await vi.waitFor( () => { const menu = getSlashMenu()!; const items = getSlashMenuItems(menu); @@ -357,7 +305,7 @@ describe("Slash Command Menu", () => { await userEvent.keyboard("{ArrowDown}"); - await diagWaitFor("moves selection down with ArrowDown", () => { + await vi.waitFor(() => { const menu = getSlashMenu()!; const items = getSlashMenuItems(menu); expect(isItemSelected(items[1]!)).toBe(true); @@ -373,13 +321,13 @@ describe("Slash Command Menu", () => { // Move down, then back up await userEvent.keyboard("{ArrowDown}"); - await diagWaitFor("ArrowUp from second item: first ArrowDown", () => { + await vi.waitFor(() => { const items = getSlashMenuItems(getSlashMenu()!); expect(isItemSelected(items[1]!)).toBe(true); }); await userEvent.keyboard("{ArrowUp}"); - await diagWaitFor("ArrowUp from second item: back to first", () => { + await vi.waitFor(() => { const items = getSlashMenuItems(getSlashMenu()!); expect(isItemSelected(items[0]!)).toBe(true); }); @@ -393,7 +341,7 @@ describe("Slash Command Menu", () => { await userEvent.keyboard("{ArrowUp}"); - await diagWaitFor("wraps selection around (ArrowUp from first)", () => { + await vi.waitFor(() => { const menu = getSlashMenu()!; const items = getSlashMenuItems(menu); const lastItem = items.at(-1)!; @@ -412,7 +360,7 @@ describe("Slash Command Menu", () => { await waitForSlashMenuClosed(); - await diagWaitFor("Enter converts to heading: h1 should exist", () => { + await vi.waitFor(() => { expect(pm.querySelector("h1")).toBeTruthy(); }); }); @@ -539,8 +487,15 @@ describe("Slash Command Menu", () => { const menu = await waitForSlashMenu(); const items = getSlashMenuItems(menu); - // React listens for pointerenter/mouseenter on the element. - // Use userEvent.hover which properly dispatches pointer + mouse events. + // The menu gates mouseenter on a "has the user actually moved the + // pointer since the menu opened?" flag, to avoid jumping selection + // when the menu renders under a stationary pointer (which happens + // in CI because pointer position persists across tests). Dispatch a + // real pointermove on the menu first so the gate is open before we + // hover an item. userEvent.hover by itself only teleports the + // cursor to the target and fires pointerenter -- no pointermove. + menu.dispatchEvent(new PointerEvent("pointermove", { bubbles: true, pointerType: "mouse" })); + await userEvent.hover(items[2]!); await vi.waitFor(() => {