fix(admin): don't steal slash menu selection on stationary pointer#1013
Merged
Conversation
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.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-perf-coordinator | 248c0d2 | May 13 2026, 10:39 AM |
🦋 Changeset detectedLatest commit: 248c0d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-i18n | 248c0d2 | May 13 2026, 10:39 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 248c0d2 | May 13 2026, 10:40 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 248c0d2 | May 13 2026, 10:40 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 248c0d2 | May 13 2026, 10:40 AM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a CI-only flake in the admin Portable Text editor’s slash-command menu where a stationary pointer could trigger mouseenter on render and unexpectedly override the keyboard-selected menu item. It also removes temporary diagnostic helpers and CI artifact upload steps that were added to identify the root cause.
Changes:
- Gate slash-menu hover selection so
mouseenteronly updates selection after real pointer movement since the menu opened. - Update the slash-menu hover test to reflect the new “pointer must have moved” contract.
- Revert temporary CI/test diagnostics introduced for investigating the flake, and add a changeset for the patch release.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/admin/src/components/PortableTextEditor.tsx |
Adds a “mouse moved” gate to prevent stationary-pointer mouseenter from stealing selection in the slash menu. |
packages/admin/tests/editor/slash-menu.test.tsx |
Removes diagnostic helpers and adapts the hover test to open the new gate before hovering. |
.github/workflows/ci.yml |
Removes the failure-only artifact upload step for vitest browser screenshots (diagnostics revert). |
.changeset/fix-slash-menu-mouseenter-race.md |
Adds a patch changeset describing the behavior fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1126
to
+1127
| onPointerMove={() => { | ||
| hasMouseMovedRef.current = true; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the recurring "Browser Tests" flake in
tests/editor/slash-menu.test.tsx. Also reverts the temporary diagnostic instrumentation from #1010 (workflow artifact upload +diagWaitForhelpers), which we no longer need.Root cause
The slash command menu renders right at the text cursor's
clientRect. Each rendered item hasonMouseEnter={() => setSelectedIndex(index)}to support the "hover to select" UX.The problem:
mouseenterfires when an element appears under a stationary pointer, not only when the pointer moves over it. In CI, Vitest's docs explicitly call this out:So whenever an earlier test left the pointer at a Y coordinate that happens to overlap a slash menu item in the next test, that item's
mouseenterfires the instant the menu renders and overrides the keyboard-driven default selection of 0.This was strictly intermittent locally (we never reproduced on macOS even at 40x CDP CPU throttle) but near-deterministic on Linux CI when the slash-menu suite ran slowly enough for the menu render to happen after the pointer settled.
Evidence
The diagnostic dumps from #1010 caught it red-handed on the very next CI run after merging. From the failing run logs (job):
Pattern: the wrong item is consistently
expected + 4or close to it -- a constant offset that lines up with the runner's lingering Y coord.Fix
Add a
hasMouseMovedRefto the menu container. It startsfalsewhen the menu opens, flips totrueonly onpointermove, and is reset on close. The per-itemonMouseEnterhandler now no-ops while the flag isfalse:```tsx
onPointerMove={() => { hasMouseMovedRef.current = true; }}
// ...
onMouseEnter={() => {
if (hasMouseMovedRef.current) {
setSelectedIndex(index);
}
}}
```
This preserves the legitimate "hover to select" UX after real pointer movement, but ignores
mouseenterevents that fire merely because an item appeared under a stationary pointer.Existing test update
The
highlights item on mouse hovertest callsuserEvent.hover(items[2]). Under Playwright,hoverteleports the cursor to the target and firespointerenterbut notpointermove. The test now dispatches a syntheticpointermoveon the menu container first so the gate opens, then callsuserEvent.hoveras before. This matches the new contract -- selection only follows the pointer once the pointer is actually being moved.Revert of #1010 diagnostics
This PR also reverts:
Upload failure screenshotsstep in.github/workflows/ci.ymldumpMenuState+diagWaitForhelpers inpackages/admin/tests/editor/slash-menu.test.tsx(and switches the 5 wrapped sites back to plainvi.waitFor)The diag instrumentation did its job: caught the bug on the first CI run after merging.
Verification
pnpm --filter @emdash-cms/admin typecheckcleanpnpm lintshows no new diagnostics on changed filestests/editor/slash-menu.test.tsx(23 tests each)pnpm formatranCloses #
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure