Skip to content

Commit 3ec18bc

Browse files
Waleed Latifwaleedlatif1
authored andcommitted
fix(desktop): close the peek's three timer races
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.
1 parent a88b104 commit 3ec18bc

2 files changed

Lines changed: 72 additions & 9 deletions

File tree

apps/sim/app/workspace/[workspaceId]/components/workspace-chrome/use-sidebar-peek.test.tsx

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,59 @@ describe('useSidebarPeek', () => {
300300
expect(active.state().isPeekActive).toBe(false)
301301
})
302302

303+
it('never mounts the card when a modal opens mid-dwell', () => {
304+
active = renderPeek(true)
305+
306+
act(() => {
307+
active?.triggerEnter()
308+
vi.advanceTimersByTime(OPEN_DELAY_MS - 20)
309+
})
310+
active.setDismissed(true)
311+
act(() => {
312+
vi.advanceTimersByTime(OPEN_DELAY_MS * 2)
313+
})
314+
315+
expect(active.state().isPeekActive).toBe(false)
316+
})
317+
318+
it('drops an already-exiting card the instant a modal opens', () => {
319+
active = renderPeek(true)
320+
openPeek(active)
321+
movePointerTo(POINT.onContent)
322+
act(() => {
323+
vi.advanceTimersByTime(CLOSE_DELAY_MS)
324+
})
325+
expect(active.state().isPeekActive).toBe(true)
326+
327+
active.setDismissed(true)
328+
329+
expect(active.state().isPeekActive).toBe(false)
330+
})
331+
332+
it('snaps a card that is animating out back open on re-hover', () => {
333+
active = renderPeek(true)
334+
openPeek(active)
335+
movePointerTo(POINT.onContent)
336+
act(() => {
337+
vi.advanceTimersByTime(CLOSE_DELAY_MS)
338+
})
339+
// Mid-exit: mounted but no longer open.
340+
expect(active.state().isPeekActive).toBe(true)
341+
expect(active.state().isPeekOpen).toBe(false)
342+
343+
// Re-hover late in the exit window; the pending exit timer must not win.
344+
act(() => {
345+
vi.advanceTimersByTime(EXIT_DURATION_MS - 20)
346+
active?.triggerEnter()
347+
})
348+
expect(active.state().isPeekOpen).toBe(true)
349+
350+
act(() => {
351+
vi.advanceTimersByTime(EXIT_DURATION_MS * 2)
352+
})
353+
expect(active.state().isPeekOpen).toBe(true)
354+
})
355+
303356
it('retracts when a modal opens, even with the pointer inside', () => {
304357
active = renderPeek(true)
305358
openPeek(active)

apps/sim/app/workspace/[workspaceId]/components/workspace-chrome/use-sidebar-peek.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,29 +145,39 @@ export function useSidebarPeek(enabled: boolean, dismissed = false): SidebarPeek
145145
if (!enabled || dismissed) return
146146
clearTimer(closeTimerRef)
147147
clearTimer(openTimerRef)
148+
// Still on screen and animating out: snap it back instead of waiting out another
149+
// dwell, which the pending exit timer would win — unmounting the card and then
150+
// re-mounting it, a visible flicker with the pointer never leaving the toggle.
151+
if (phase === 'exiting') {
152+
open()
153+
return
154+
}
148155
openTimerRef.current = setTimeout(() => {
149156
openTimerRef.current = null
150157
open()
151158
}, PEEK_OPEN_DELAY_MS)
152-
}, [dismissed, enabled, open])
159+
}, [dismissed, enabled, open, phase])
153160

154161
const onTriggerLeave = useCallback(() => {
155162
clearTimer(openTimerRef)
156163
}, [])
157164

158-
/** Drop the card outright — no exit animation — once the peek stops being available. */
165+
/**
166+
* Drop the card outright — no exit animation — the moment the peek stops being
167+
* available (⌘B, fullscreen) or a modal takes the screen.
168+
*
169+
* Unconditional rather than gated on the current phase, because every phase needs
170+
* clearing: a pending dwell would otherwise fire and mount the card over the modal,
171+
* and an in-flight exit would keep animating on top of it. Instant is also right
172+
* visually — the modal's own scrim covers the card's position on the same frame.
173+
*/
159174
useEffect(() => {
160-
if (enabled) return
175+
if (enabled && !dismissed) return
161176
clearTimer(openTimerRef)
162177
clearTimer(closeTimerRef)
163178
clearTimer(exitTimerRef)
164179
setPhase('closed')
165-
}, [enabled])
166-
167-
/** A modal is a destination and owns the screen, so the transient card yields to it. */
168-
useEffect(() => {
169-
if (dismissed && phase === 'open') close()
170-
}, [close, dismissed, phase])
180+
}, [dismissed, enabled])
171181

172182
useEffect(() => {
173183
if (phase !== 'open') return

0 commit comments

Comments
 (0)