Skip to content

Commit 0cd8c6d

Browse files
authored
fix(admin): don't steal slash menu selection on stationary pointer (#1013)
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.
1 parent b6e5b66 commit 0cd8c6d

4 files changed

Lines changed: 48 additions & 73 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@emdash-cms/admin": patch
3+
"emdash": patch
4+
---
5+
6+
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.

.github/workflows/ci.yml

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,18 +204,6 @@ jobs:
204204
- run: pnpm exec playwright install --with-deps chromium
205205
if: steps.playwright-cache.outputs.cache-hit != 'true'
206206
- run: pnpm run --filter @emdash-cms/admin test
207-
# Upload vitest browser-mode failure screenshots so we can diagnose
208-
# CI-only flakes (e.g. the recurring slash-menu race). Only runs on
209-
# failure, only uploads the screenshot directories.
210-
- name: Upload failure screenshots
211-
if: failure()
212-
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
213-
with:
214-
name: browser-test-screenshots
215-
path: |
216-
packages/admin/tests/**/__screenshots__/**
217-
if-no-files-found: ignore
218-
retention-days: 1
219207

220208
test-e2e-rollup:
221209
name: E2E Tests

packages/admin/src/components/PortableTextEditor.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,22 @@ function SlashCommandMenu({
10971097
}
10981098
}, [state.selectedIndex, state.isOpen]);
10991099

1100+
// Track whether the mouse has actually moved since the menu opened.
1101+
// The menu typically opens right at the text cursor, which may sit under
1102+
// a stationary mouse pointer. Reacting to mouseenter immediately would
1103+
// reset the selection to whichever item happens to be under the pointer
1104+
// the moment the menu renders -- overriding the keyboard-driven default
1105+
// (selectedIndex: 0) and any subsequent arrow-key navigation.
1106+
//
1107+
// Only flip the gate on mousemove, which fires only on real pointer
1108+
// movement, not on elements appearing under a stationary pointer.
1109+
const hasMouseMovedRef = React.useRef(false);
1110+
React.useEffect(() => {
1111+
if (!state.isOpen) {
1112+
hasMouseMovedRef.current = false;
1113+
}
1114+
}, [state.isOpen]);
1115+
11001116
if (!state.isOpen) return null;
11011117

11021118
return createPortal(
@@ -1107,6 +1123,9 @@ function SlashCommandMenu({
11071123
}}
11081124
style={floatingStyles}
11091125
className="z-[100] rounded-lg border bg-kumo-overlay p-1 shadow-lg min-w-[220px] max-h-[300px] overflow-y-auto"
1126+
onPointerMove={() => {
1127+
hasMouseMovedRef.current = true;
1128+
}}
11101129
>
11111130
{state.items.length === 0 ? (
11121131
<p className="text-sm text-kumo-subtle px-3 py-2">{t`No results`}</p>
@@ -1123,7 +1142,14 @@ function SlashCommandMenu({
11231142
: "hover:bg-kumo-tint/50",
11241143
)}
11251144
onClick={() => onCommand(item)}
1126-
onMouseEnter={() => setSelectedIndex(index)}
1145+
onMouseEnter={() => {
1146+
// Only react if the user has actually moved the
1147+
// mouse since the menu opened -- not when items
1148+
// appear under a stationary pointer.
1149+
if (hasMouseMovedRef.current) {
1150+
setSelectedIndex(index);
1151+
}
1152+
}}
11271153
>
11281154
<item.icon className="h-4 w-4 text-kumo-subtle flex-shrink-0" />
11291155
<div className="flex flex-col">

packages/admin/tests/editor/slash-menu.test.tsx

Lines changed: 15 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -189,57 +189,6 @@ function isItemSelected(el: HTMLElement): boolean {
189189
return el.className.split(WHITESPACE_SPLIT_REGEX).includes("bg-kumo-tint");
190190
}
191191

192-
/**
193-
* DIAGNOSTIC (temporary, see #1004 follow-up): dump everything useful about
194-
* the slash menu state to console.log. Called from a catch block when
195-
* vi.waitFor times out, so the failing CI log contains the actual menu
196-
* contents and DOM state at the moment of failure. Remove once the
197-
* underlying race is understood and fixed.
198-
*/
199-
function dumpMenuState(label: string): void {
200-
const menu = getSlashMenu();
201-
const lines: string[] = [
202-
`[slash-menu diag] === ${label} ===`,
203-
`[slash-menu diag] menu present: ${menu !== null}`,
204-
`[slash-menu diag] activeElement: ${document.activeElement?.tagName} (class=${document.activeElement?.className?.slice(0, 80)})`,
205-
`[slash-menu diag] body > div count: ${document.querySelectorAll("body > div").length}`,
206-
];
207-
if (menu) {
208-
const items = getSlashMenuItems(menu);
209-
lines.push(`[slash-menu diag] items rendered: ${items.length}`);
210-
lines.push(`[slash-menu diag] menu textContent length: ${menu.textContent?.length ?? 0}`);
211-
lines.push(`[slash-menu diag] menu first 200 chars: ${menu.textContent?.slice(0, 200)}`);
212-
items.forEach((el, i) => {
213-
const dataIndex = el.getAttribute("data-index");
214-
const selected = isItemSelected(el);
215-
const classes = el.className;
216-
lines.push(
217-
`[slash-menu diag] item ${i} (data-index=${dataIndex}) selected=${selected} classes=${classes.slice(0, 200)}`,
218-
);
219-
});
220-
// menu.outerHTML truncated to 2000 chars
221-
lines.push(`[slash-menu diag] menu outerHTML: ${menu.outerHTML.slice(0, 2000)}`);
222-
}
223-
console.log(lines.join("\n"));
224-
}
225-
226-
/**
227-
* DIAGNOSTIC wrapper around vi.waitFor that dumps menu state on timeout.
228-
* Same semantics as vi.waitFor; only adds logging on failure.
229-
*/
230-
async function diagWaitFor<T>(
231-
label: string,
232-
predicate: () => T | Promise<T>,
233-
options?: { timeout?: number; interval?: number },
234-
): Promise<T> {
235-
try {
236-
return await vi.waitFor(predicate, options);
237-
} catch (err) {
238-
dumpMenuState(label);
239-
throw err;
240-
}
241-
}
242-
243192
// =============================================================================
244193
// Slash Command Menu
245194
// =============================================================================
@@ -338,8 +287,7 @@ describe("Slash Command Menu", () => {
338287

339288
await waitForSlashMenu();
340289

341-
await diagWaitFor(
342-
"highlights the first item by default",
290+
await vi.waitFor(
343291
() => {
344292
const menu = getSlashMenu()!;
345293
const items = getSlashMenuItems(menu);
@@ -357,7 +305,7 @@ describe("Slash Command Menu", () => {
357305

358306
await userEvent.keyboard("{ArrowDown}");
359307

360-
await diagWaitFor("moves selection down with ArrowDown", () => {
308+
await vi.waitFor(() => {
361309
const menu = getSlashMenu()!;
362310
const items = getSlashMenuItems(menu);
363311
expect(isItemSelected(items[1]!)).toBe(true);
@@ -373,13 +321,13 @@ describe("Slash Command Menu", () => {
373321

374322
// Move down, then back up
375323
await userEvent.keyboard("{ArrowDown}");
376-
await diagWaitFor("ArrowUp from second item: first ArrowDown", () => {
324+
await vi.waitFor(() => {
377325
const items = getSlashMenuItems(getSlashMenu()!);
378326
expect(isItemSelected(items[1]!)).toBe(true);
379327
});
380328

381329
await userEvent.keyboard("{ArrowUp}");
382-
await diagWaitFor("ArrowUp from second item: back to first", () => {
330+
await vi.waitFor(() => {
383331
const items = getSlashMenuItems(getSlashMenu()!);
384332
expect(isItemSelected(items[0]!)).toBe(true);
385333
});
@@ -393,7 +341,7 @@ describe("Slash Command Menu", () => {
393341

394342
await userEvent.keyboard("{ArrowUp}");
395343

396-
await diagWaitFor("wraps selection around (ArrowUp from first)", () => {
344+
await vi.waitFor(() => {
397345
const menu = getSlashMenu()!;
398346
const items = getSlashMenuItems(menu);
399347
const lastItem = items.at(-1)!;
@@ -412,7 +360,7 @@ describe("Slash Command Menu", () => {
412360

413361
await waitForSlashMenuClosed();
414362

415-
await diagWaitFor("Enter converts to heading: h1 should exist", () => {
363+
await vi.waitFor(() => {
416364
expect(pm.querySelector("h1")).toBeTruthy();
417365
});
418366
});
@@ -539,8 +487,15 @@ describe("Slash Command Menu", () => {
539487
const menu = await waitForSlashMenu();
540488
const items = getSlashMenuItems(menu);
541489

542-
// React listens for pointerenter/mouseenter on the element.
543-
// Use userEvent.hover which properly dispatches pointer + mouse events.
490+
// The menu gates mouseenter on a "has the user actually moved the
491+
// pointer since the menu opened?" flag, to avoid jumping selection
492+
// when the menu renders under a stationary pointer (which happens
493+
// in CI because pointer position persists across tests). Dispatch a
494+
// real pointermove on the menu first so the gate is open before we
495+
// hover an item. userEvent.hover by itself only teleports the
496+
// cursor to the target and fires pointerenter -- no pointermove.
497+
menu.dispatchEvent(new PointerEvent("pointermove", { bubbles: true, pointerType: "mouse" }));
498+
544499
await userEvent.hover(items[2]!);
545500

546501
await vi.waitFor(() => {

0 commit comments

Comments
 (0)