Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fix-slash-menu-mouseenter-race.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 0 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 27 additions & 1 deletion packages/admin/src/components/PortableTextEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Comment on lines +1126 to +1127
}}
>
{state.items.length === 0 ? (
<p className="text-sm text-kumo-subtle px-3 py-2">{t`No results`}</p>
Expand All @@ -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);
}
}}
>
<item.icon className="h-4 w-4 text-kumo-subtle flex-shrink-0" />
<div className="flex flex-col">
Expand Down
75 changes: 15 additions & 60 deletions packages/admin/tests/editor/slash-menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(
label: string,
predicate: () => T | Promise<T>,
options?: { timeout?: number; interval?: number },
): Promise<T> {
try {
return await vi.waitFor(predicate, options);
} catch (err) {
dumpMenuState(label);
throw err;
}
}

// =============================================================================
// Slash Command Menu
// =============================================================================
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
});
Expand All @@ -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)!;
Expand All @@ -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();
});
});
Expand Down Expand Up @@ -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(() => {
Expand Down
Loading