Skip to content

fix(sidebar): stop the workspace switcher stranding a phantom hover - #6096

Merged
waleedlatif1 merged 3 commits into
stagingfrom
investigate-workspace-hover-state
Jul 30, 2026
Merged

fix(sidebar): stop the workspace switcher stranding a phantom hover#6096
waleedlatif1 merged 3 commits into
stagingfrom
investigate-workspace-hover-state

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

The report

In the workspace switcher, a row keeps a hover-looking highlight after the pointer has moved away — so two rows appear hovered at once (the selected workspace, plus a stale one).

Cause

Not a hover at all. #5893 added keyboard navigation to the switcher, tracked by highlightedId. Three things combine:

  1. highlightedId is set from onMouseMove (workspace-header.tsx:578) — deliberately, since mouseenter would hijack the keyboard selection when scrollIntoView slides rows under a stationary cursor.
  2. Nothing clears it on the way out. There is no onMouseLeave in the file; it only resets when the whole menu closes.
  3. It paints via chipVariants({ active }), and in chip.tsx active: truebg-[var(--surface-active)] while active: falsehover-hover:bg-[var(--surface-active)]. Identical token (#ececec / #2c2c2c).

So the last row the pointer crossed keeps a background indistinguishable from hover, forever — next to the equally---surface-active current workspace.

It also happens with no hovering at all: the seeding effect (:214) puts the cursor on row 0 at open, so any user whose current workspace isn't first sees two marked rows immediately. Only reachable above WORKSPACE_SEARCH_THRESHOLD (3 workspaces), which is why it went unnoticed.

Fix

Paint the cursor only while the user is actually navigating by keyboard. Arrow keys enter that mode; any pointer motion leaves it. In pointer mode the only mark is real CSS :hover, which follows the pointer and leaves with it.

highlightedId still tracks the pointer, so Enter keeps targeting the row last touched — only whether it's drawn changes.

Why this approach

  • It's the pattern this codebase already has: packages/emcn/src/components/popover/popover.tsx tracks isKeyboardNav under the literal comment "Suppress hover when in keyboard mode to prevent dual highlights", and tag-dropdown.tsx consumes it.
  • It matches how Headless UI models a combobox: one modality-driven focus marker (data-focus, "focused via mouse or keyboard") kept separate from data-selected (the current value).
  • The list carries no aria-activedescendant, aria-selected, or listbox/option roles, so the highlight is purely visual and no a11y contract changes.

Why not onMouseLeave

It's a normal pattern (19 files use it) but the wrong tool here, and it wouldn't work: the seeding effect at :214 re-seeds highlightedId to filteredWorkspaces[0] whenever the current one is absent, so clearing to null bounces the highlight to row 1 rather than removing it. It would also silently retarget Enter (activeIndex falls back to 0), do nothing for the on-open case, and fire spuriously when a row's ... menu portals out.

Testing

New workspace-header.test.tsx with 4 tests, deliberately rendering the real @sim/emcn — mocking chipVariants would only assert the stub.

All three highlight tests were verified to fail without the fix and pass with it. Two fixture details that matter:

  • The current workspace is intentionally not first, or the row-0 seed masks the on-open double-mark (an earlier draft passed while broken because of this).
  • isMarked matches an exact class token, never a substring — hover-hover:bg-[var(--surface-active)] contains the active class, so a substring check reports every row as marked.

@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 30, 2026 6:45pm

Request Review

@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Sidebar UI interaction fix in the workspace switcher only; no auth, data, or API changes. Regression coverage is added for highlight behavior.

Overview
Fixes the workspace switcher showing two rows with the same --surface-active fill: the current workspace plus a stale “keyboard” row that looked like a stuck hover.

WorkspaceHeader now tracks isKeyboardNav (same idea as emcn’s popover). The list cursor is only painted when the user is in keyboard mode—arrow keys, typing in search, etc. Pointer mousemove turns keyboard mode off and still updates highlightedId for targeting, but leaves hover to real CSS :hover. Enter in the search field does nothing until keyboard mode is on, so opening the menu with focus in search no longer switches workspace with nothing visibly selected. State resets when the menu closes.

Adds workspace-header.test.tsx with six jsdom tests against the real @sim/emcn chip classes (exact bg-[var(--surface-active)] token, not substring), covering open-only marking, pointer pass-through, keyboard cursor, and Enter arming.

Reviewed by Cursor Bugbot for commit b9b2efd. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes phantom workspace-switcher highlights by separating keyboard cursor painting from pointer navigation and adds regression coverage for initial, pointer, typing, and keyboard interactions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx Adds input-modality state so persistent cursor highlighting appears only during keyboard navigation while pointer movement retains Enter targeting.
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.test.tsx Adds regression tests that now verify keyboard cursor painting on a non-selected workspace, resolving the prior assertion gap.

Reviews (2): Last reviewed commit: "fix(sidebar): do not arm Enter without a..." | Re-trigger Greptile

Waleed Latif added 3 commits July 30, 2026 11:44
The switcher's keyboard cursor is set from `onMouseMove` and only cleared
when the menu closes, and it paints in `--surface-active` — the same token
hover uses. So the last row the pointer crossed keeps a background
indistinguishable from hover long after the pointer has gone, sitting
alongside the equally-`--surface-active` current workspace as a second
phantom-hovered row. It shows without any hovering too: the cursor is
seeded to row 0 on open, so any user whose current workspace is not first
sees two marked rows immediately. Only bites above the 3-workspace search
threshold, which is why it went unnoticed.

Paint the cursor only while the user is actually navigating by keyboard.
Arrow keys enter that mode, any pointer motion leaves it, so in pointer
mode the sole mark is real CSS :hover — which follows the pointer and
leaves with it. `highlightedId` still tracks the pointer, so Enter keeps
targeting the row last touched; only whether it is drawn changes.

This is the pattern emcn's own popover already uses (`isKeyboardNav`,
commented "prevent dual highlights") and that `tag-dropdown` consumes, and
it matches how Headless UI models a combobox: one modality-driven focus
marker, separate from the selected value. The list carries no
`aria-activedescendant`/`aria-selected`, so the highlight is purely visual
and nothing in the a11y contract changes.
Review round 1: the keyboard-positive assertions landed on the current
workspace, which carries its own `isActive` fill, so they held whether or
not the cursor was painted — the tests could not have caught deleting
keyboard-cursor rendering.

Navigate with ArrowUp instead, wrapping from the seeded first row to the
last, and assert on that row. Verified both directions now: removing the
modality gate reddens three tests, removing keyboard painting reddens two.
Review round 1: gating the cursor's paint on keyboard mode left Enter still
acting on the seeded first row while that row was unmarked. The search
field takes focus on open, so Enter could switch workspace with nothing
shown as the target — emcn's popover instead holds its selection at -1 and
ignores Enter until keyboard nav begins.

Enter now acts only once a cursor is on screen, and typing counts as
keyboard intent so the common "filter, then Enter" flow lands on a visible
top result. Every path to Enter therefore has a marked target.
@waleedlatif1
waleedlatif1 force-pushed the investigate-workspace-hover-state branch from c35953b to b9b2efd Compare July 30, 2026 18:45
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b9b2efd. Configure here.

@waleedlatif1
waleedlatif1 merged commit 6647808 into staging Jul 30, 2026
27 checks passed
@waleedlatif1
waleedlatif1 deleted the investigate-workspace-hover-state branch July 30, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant