feat(desktop): floating hover-peek sidebar - #6086
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Width plumbing — Introduces Also in this PR — Search modal overlay/dialog z-index moved to Reviewed by Cursor Bugbot for commit abf1bde. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryAdds a macOS desktop hover-peek for the collapsed sidebar and related chrome fixes.
Confidence Score: 5/5This follow-up pass finds no remaining blocking failures and appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/components/workspace-chrome/use-sidebar-peek.ts | Hook for open/close/exit phases, pointer sampling, Escape/popper rules, and modal dismissal. |
| apps/sim/app/workspace/[workspaceId]/components/workspace-chrome/workspace-chrome.tsx | Wires peek only for inset desktop title bar; stable trigger wrapper; card chrome on shell. |
| apps/sim/stores/sidebar/store.ts | Publishes both --sidebar-width and --sidebar-expanded-width on set/toggle/sync/rehydrate. |
| apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx | isPeeking expands layout and skips title-bar spacer plus resize edge while floating. |
Sequence Diagram
sequenceDiagram
participant User
participant Toggle as Title-bar toggle
participant Peek as useSidebarPeek
participant Shell as sidebar-shell-outer
participant Sidebar as Sidebar
User->>Toggle: mouseenter
Toggle->>Peek: onTriggerEnter
Peek->>Peek: dwell PEEK_OPEN_DELAY_MS
Peek->>Shell: phase open (data-peek, absolute card chrome)
Shell->>Sidebar: isPeeking true (expanded layout)
User->>Peek: pointermove outside card/trigger/popper
Peek->>Peek: close after PEEK_CLOSE_DELAY_MS
Peek->>Shell: exiting animation then unmount flags
Reviews (3): Last reviewed commit: "fix(desktop): arm the peek before first ..." | Re-trigger Greptile
|
@cursor review |
On the macOS desktop shell the collapsed sidebar has no width at all
(`--sidebar-collapsed-width: 0`), so the only ways back were ⌘B and the
title-bar toggle. Hovering that toggle now floats the sidebar in as a card
over the content, inset from the window edge, and it retracts when the
pointer leaves. Clicking the same control still docks it for good.
The card is the existing `.sidebar-shell-outer` re-styled, not a second
surface: `<Sidebar>` is never re-mounted, so its scroll position and
expansion state survive a peek. One CSS rule re-points `--sidebar-width` at
a new `--sidebar-expanded-width`, and the inner shell and aside follow with
no per-element overrides. Chrome is all existing tokens — `--radius`,
`--border`, `shadow-overlay`, `--z-modal` — and the fill is the sidebar's
own `--surface-1`, so docked and floating are the same surface. Enter/exit
use the popper idiom (`fade-in-0 zoom-in-95`) that emcn's Radix surfaces
already use.
Also migrates the search palette from raw `z-40`/`z-50` to `--z-modal`. It
was the one overlay outranked by the new card, and the raw values also put
it below `--z-toast`, inverting the order globals.css documents.
Settings drive-bys, all pre-existing violations in files this touches:
tokenize two literal pixel text sizes in passwords-view, route its cards
through `SettingsResourceRow` instead of four re-derived chrome constants,
and collapse three `{null}`-bodied `SettingsSection`s in browser settings
into one section of real rows.
Review round 1 (Cursor Bugbot) found three ways the card could flicker: - A modal opening mid-dwell left the open timer pending, so the card mounted over the modal before the dismiss path could run. - A modal opening while the card was already animating out let it keep animating on top of the modal. - Re-hovering the toggle late in the exit window scheduled a fresh dwell without clearing the exit timer, so the card unmounted and then re-mounted — a visible gap with the pointer never leaving the toggle. The first two collapse into one fix: merge the two reset effects so an unavailable peek and a screen-owning modal both drop the card immediately, unconditionally clearing all three timers. Instant is also the right look, since the modal's scrim covers the card's position on the same frame. For the third, a re-hover during the exit now re-opens synchronously rather than scheduling another dwell — the card is already on screen, so the dwell that guards against accidental opens from a cold state does not apply. One regression test each.
Review round 2: the title-bar seed ran in a passive effect, which lands after paint — long enough for a `mouseenter` on the toggle to be dropped while the peek still read disabled, with nothing to retry it until the pointer left and returned. A regression from folding the seed into the window-state effect during cleanup. Promoted to a layout effect, which runs before paint and before any mouse event can be dispatched. The body is cheap synchronous reads plus a subscription; the bridge's getState stays async and blocks nothing.
7c737fd to
abf1bde
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ 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 abf1bde. Configure here.
1 participant
What
On the macOS desktop shell the collapsed sidebar has no width at all (
--sidebar-collapsed-width: 0), unlike web's 51px icon rail — so once collapsed the only ways back were ⌘B and the small title-bar toggle. Hovering that toggle now floats the sidebar in as a card over the content, inset from the window edge, retracting when the pointer leaves. Clicking the same control still docks it for good.Web and macOS native fullscreen (which falls back to the icon rail) are untouched.
How
The card is the existing
.sidebar-shell-outer, re-styled — not a second surface.<Sidebar>is 1800 lines with scroll and expansion state, so it must never re-mount; keeping the same element in the same tree position guarantees that. One CSS rule re-points--sidebar-widthat a new--sidebar-expanded-width, and the inner shell plus the aside follow with no per-element width overrides.rounded-lg(--radius, same 8px as the content pane it floats beside),border-[var(--border)](byte-identical to that pane's),shadow-overlayandz-[var(--z-modal)](what the app's other edge-anchored panels use). The fill is the sidebar's own--surface-1, so docked and floating are the same surface.fade-in-0 zoom-in-95). These are keyframe animations, not transitions, deliberately: an animation runs from mount, so no hidden "from" frame and norequestAnimationFrameto step into — rAF is throttled in an unfocused window and would strand the card mounted-but-invisible.w-auto, not a length:widthcannot interpolate to or fromauto, so entering and leaving snap instead of animating. With a length the card widened as it appeared and left a 248px ghost shrinking to zero in flow on retract.pointermovehit-test, notmouseleave: every menu the sidebar opens lives in a body portal, somouseleavewould retract the card the instant you reached for a menu item. The card and trigger are matched geometrically (coordinates can't go stale when the tooltip subtree re-renders); poppers are matched via the same predicate emcn's own modal uses.Also fixed
Search palette migrated from raw
z-40/z-50to--z-modal. It was the one overlay the new card outranked, and the raw values also placed it below--z-toast, inverting the orderglobals.cssdocuments. Fixing it at the root beat teaching the peek to dodge it.Settings drive-bys — all pre-existing violations in files this PR already touches:
passwords-view: two literal pixel text sizes → scale tokens (text-sm/text-caption), and its cards now route throughSettingsResourceRowinstead of four re-derived chrome constants (mirroring the establishedcustom-blockspattern).browser: three{null}-bodiedSettingsSections — the only such usage in the codebase — collapse into one section of real label+action rows.Testing
--sidebar-expanded-widthacross collapse/expand/resize/rehydrate.[8, 40, 250, 812], 8px radius, shadow on,transform-origintop-left, the 90ms dwell gate, and traversal from trigger into card without retracting.Not verified: the animation feel. Background windows throttle both timers and CSS animations, and the harness could not reliably bring the Electron window frontmost, so every measurement of the enter/exit animation was invalid. Geometry and chrome were verified with real numbers; the fade/zoom needs a human eye.
Review notes
Three real bugs were found and fixed during review, each with a test:
SidebarTooltipreturns barechildrenwhen disabled, so suppressing the tooltip remounted the button and nulled the ref mid-hover. Fixed by anchoring the hover region to a stable wrapper and hit-testing geometrically.[data-native-surface-overlay]:not([aria-modal="true"])— but emcn stamps that marker on the modal scrim and never setsaria-modal(Radix puts it on a different node), so the exclusion matched nothing and afixed inset-0scrim would have pinned the card open permanently.lint:checkfails repo-wide onterminal-session.tsx(noFocusedTestson xterm'sfit.fit()) — verified pre-existing on cleanstaging, untouched here.