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(() => {