From 96579e88acb79c8a9a04729dbe4138098bb97f6e Mon Sep 17 00:00:00 2001 From: Aarav Sareen <96787824+arvsrn@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:49:03 +0530 Subject: [PATCH] fix(app): snap vertical tab preview to its tab (#63074) --- .../titlebar/tab-popover-machine.test.ts | 61 +----------- .../src/shell/titlebar/tab-popover-machine.ts | 75 +++----------- .../app/src/shell/titlebar/tab-popover.tsx | 98 ------------------- 3 files changed, 18 insertions(+), 216 deletions(-) diff --git a/packages/app/src/shell/titlebar/tab-popover-machine.test.ts b/packages/app/src/shell/titlebar/tab-popover-machine.test.ts index 8957370ab2f..7654b47cdeb 100644 --- a/packages/app/src/shell/titlebar/tab-popover-machine.test.ts +++ b/packages/app/src/shell/titlebar/tab-popover-machine.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test" import { INITIAL_TAB_PREVIEW_STATE, - selectTabPreviewFollowCard, selectTabPreviewInstant, selectTabPreviewOpenDelay, TAB_PREVIEW_OPEN_DELAY_MS, @@ -12,14 +11,10 @@ import { type TabPreviewState, } from "./tab-popover-machine" -type El = { readonly id: string } - const owner = (): TabPreviewOwner => ({ close: () => {} }) -const el = (id: string): El => ({ id }) - -const replay = (events: TabPreviewEvent[], state: TabPreviewState = INITIAL_TAB_PREVIEW_STATE) => - events.reduce<{ state: TabPreviewState; effects: TabPreviewEffect[] }>( +const replay = (events: TabPreviewEvent[], state: TabPreviewState = INITIAL_TAB_PREVIEW_STATE) => + events.reduce<{ state: TabPreviewState; effects: TabPreviewEffect[] }>( (current, event) => { const next = transitionTabPreview(current.state, event) @@ -28,48 +23,22 @@ const replay = (events: TabPreviewEvent[], state: TabPreviewState = INIT { state, effects: [] }, ) -const attach = (by: TabPreviewOwner, trigger: El, card: El, reducedMotion = false): TabPreviewEvent => ({ - type: "follow.attached", - owner: by, - trigger, - card, - reducedMotion, -}) - describe("tab preview", () => { - test("closing a following card stops following its row", () => { - const a = owner() - const row = el("row-a") - const card = el("card-a") - const open = replay([{ type: "preview.opened", owner: a }, attach(a, row, card)]) - expect(open.effects).toEqual([{ type: "follow.start", trigger: row, card }]) - expect(selectTabPreviewFollowCard(open.state, row)).toBe(card) - - const closed = transitionTabPreview(open.state, { type: "preview.closed", owner: a }) - expect(closed.effects).toEqual([{ type: "follow.stop" }, { type: "warm.start", attempt: 1, delayMs: 500 }]) - // Pointer moves over the closed row no longer place a card. - expect(selectTabPreviewFollowCard(closed.state, row)).toBeUndefined() - }) - test("a neighbouring tab takes over at once and closes the previous preview", () => { const a = owner() const b = owner() - const rowA = el("row-a") - const open = replay([{ type: "preview.opened", owner: a }, attach(a, rowA, el("card-a"))]).state + const open = replay([{ type: "preview.opened", owner: a }]).state expect(selectTabPreviewInstant(open)).toBe(false) expect(selectTabPreviewOpenDelay(open)).toBe(0) const handoff = transitionTabPreview(open, { type: "preview.opened", owner: b }) - expect(handoff.effects).toEqual([{ type: "follow.stop" }, { type: "owner.close", owner: a }]) + expect(handoff.effects).toEqual([{ type: "owner.close", owner: a }]) expect(selectTabPreviewInstant(handoff.state)).toBe(true) - expect(selectTabPreviewFollowCard(handoff.state, rowA)).toBeUndefined() // The closed tab reports back; it no longer owns the preview. const stale = transitionTabPreview(handoff.state, { type: "preview.closed", owner: a }) expect(stale.state).toBe(handoff.state) expect(stale.effects).toEqual([]) - // A follow from the previous card's late mount is ignored too. - expect(transitionTabPreview(handoff.state, attach(a, rowA, el("card-a"))).state).toBe(handoff.state) }) test("the warm window skips the open delay until it expires", () => { @@ -100,28 +69,6 @@ describe("tab preview", () => { expect(selectTabPreviewInstant(cold.state)).toBe(false) }) - test("reduced motion keeps the card in place", () => { - const a = owner() - const row = el("row-a") - const open = replay([{ type: "preview.opened", owner: a }, attach(a, row, el("card-a"), true)]) - expect(open.effects).toEqual([]) - expect(selectTabPreviewFollowCard(open.state, row)).toBeUndefined() - }) - - test("the same card attaching again does not restart following", () => { - const a = owner() - const row = el("row-a") - const card = el("card-a") - const open = replay([{ type: "preview.opened", owner: a }, attach(a, row, card)]).state - expect(transitionTabPreview(open, attach(a, row, card)).state).toBe(open) - - const remounted = transitionTabPreview(open, attach(a, row, el("card-a2"))) - expect(remounted.effects).toEqual([ - { type: "follow.stop" }, - { type: "follow.start", trigger: row, card: { id: "card-a2" } }, - ]) - }) - test("closes and disposals of a tab that is not showing change nothing", () => { const a = owner() const cold = INITIAL_TAB_PREVIEW_STATE diff --git a/packages/app/src/shell/titlebar/tab-popover-machine.ts b/packages/app/src/shell/titlebar/tab-popover-machine.ts index ab87ed8d799..951c9595176 100644 --- a/packages/app/src/shell/titlebar/tab-popover-machine.ts +++ b/packages/app/src/shell/titlebar/tab-popover-machine.ts @@ -10,71 +10,49 @@ export type TabPreviewOwner = { readonly close: () => void } -/** The vertical row a card follows the pointer over, and the card itself. */ -export type TabPreviewFollow = { - readonly trigger: El - readonly card: El -} - /** * The one tab preview shared by every tab strip. `cold` waits the full open delay; `open` and `warm` (just closed) * open the next preview at once. `instant` marks a preview opened during such a streak, which shows without animating. */ -export type TabPreviewState = +export type TabPreviewState = | { readonly status: "cold"; readonly attempt: number } | { readonly status: "open" readonly attempt: number readonly owner: TabPreviewOwner readonly instant: boolean - readonly follow: TabPreviewFollow | undefined } | { readonly status: "warm"; readonly attempt: number; readonly instant: boolean } -export type TabPreviewEvent = +export type TabPreviewEvent = | { readonly type: "preview.opened"; readonly owner: TabPreviewOwner } | { readonly type: "preview.closed"; readonly owner: TabPreviewOwner } | { readonly type: "owner.disposed"; readonly owner: TabPreviewOwner } - | { - readonly type: "follow.attached" - readonly owner: TabPreviewOwner - readonly trigger: El - readonly card: El - readonly reducedMotion: boolean - } | { readonly type: "warm.expired"; readonly attempt: number } -export type TabPreviewEffect = +export type TabPreviewEffect = | { readonly type: "owner.close"; readonly owner: TabPreviewOwner } - | { readonly type: "follow.start"; readonly trigger: El; readonly card: El } - | { readonly type: "follow.stop" } | { readonly type: "warm.start"; readonly attempt: number; readonly delayMs: number } | { readonly type: "warm.clear" } -export type TabPreviewTransition = { - readonly state: TabPreviewState - readonly effects: readonly TabPreviewEffect[] +export type TabPreviewTransition = { + readonly state: TabPreviewState + readonly effects: readonly TabPreviewEffect[] } -export const INITIAL_TAB_PREVIEW_STATE: TabPreviewState = Object.freeze({ status: "cold", attempt: 0 }) +export const INITIAL_TAB_PREVIEW_STATE: TabPreviewState = Object.freeze({ status: "cold", attempt: 0 }) const NONE: readonly never[] = [] -const STOP: readonly TabPreviewEffect[] = [{ type: "follow.stop" }] - -export function transitionTabPreview( - state: TabPreviewState, - event: TabPreviewEvent, -): TabPreviewTransition { +export function transitionTabPreview(state: TabPreviewState, event: TabPreviewEvent): TabPreviewTransition { if (event.type === "preview.opened") { if (state.status === "open" && state.owner === event.owner) return unchanged(state) - const next: TabPreviewState = { + const next: TabPreviewState = { status: "open", attempt: state.attempt, owner: event.owner, instant: state.status !== "cold", - follow: undefined, } if (state.status === "cold") return { state: next, effects: NONE } @@ -82,7 +60,7 @@ export function transitionTabPreview( if (state.status === "warm") return { state: next, effects: [{ type: "warm.clear" }] } // Another tab's preview takes over at once and closes the one that was up. - return { state: next, effects: [...stop(state), { type: "owner.close", owner: state.owner }] } + return { state: next, effects: [{ type: "owner.close", owner: state.owner }] } } if (event.type === "preview.closed" || event.type === "owner.disposed") { @@ -91,21 +69,7 @@ export function transitionTabPreview( return { state: { status: "warm", attempt, instant: state.instant }, - effects: [...stop(state), { type: "warm.start", attempt, delayMs: TAB_PREVIEW_SKIP_WINDOW_MS }], - } - } - - if (event.type === "follow.attached") { - if (state.status !== "open" || state.owner !== event.owner) return unchanged(state) - - if (event.reducedMotion) - return state.follow ? { state: { ...state, follow: undefined }, effects: STOP } : unchanged(state) - - if (state.follow?.trigger === event.trigger && state.follow.card === event.card) return unchanged(state) - - return { - state: { ...state, follow: { trigger: event.trigger, card: event.card } }, - effects: [...stop(state), { type: "follow.start", trigger: event.trigger, card: event.card }], + effects: [{ type: "warm.start", attempt, delayMs: TAB_PREVIEW_SKIP_WINDOW_MS }], } } @@ -115,25 +79,14 @@ export function transitionTabPreview( } /** Kobalte reads the open delay when the pointer enters a trigger, so a streak skips it. */ -export function selectTabPreviewOpenDelay(state: TabPreviewState) { +export function selectTabPreviewOpenDelay(state: TabPreviewState) { return state.status === "cold" ? TAB_PREVIEW_OPEN_DELAY_MS : 0 } -export function selectTabPreviewInstant(state: TabPreviewState) { +export function selectTabPreviewInstant(state: TabPreviewState) { return state.status !== "cold" && state.instant } -/** The card that follows the pointer over `trigger`, when `trigger` owns the open preview in the vertical strip. */ -export function selectTabPreviewFollowCard(state: TabPreviewState, trigger: El | undefined) { - if (state.status !== "open" || state.follow === undefined || state.follow.trigger !== trigger) return - - return state.follow.card -} - -function stop(state: Extract, { status: "open" }>): readonly TabPreviewEffect[] { - return state.follow ? STOP : NONE -} - -function unchanged(state: TabPreviewState): TabPreviewTransition { +function unchanged(state: TabPreviewState): TabPreviewTransition { return { state, effects: NONE } } diff --git a/packages/app/src/shell/titlebar/tab-popover.tsx b/packages/app/src/shell/titlebar/tab-popover.tsx index 3c433c54ccb..cb63756e1b4 100644 --- a/packages/app/src/shell/titlebar/tab-popover.tsx +++ b/packages/app/src/shell/titlebar/tab-popover.tsx @@ -1,11 +1,9 @@ import { HoverCard } from "@kobalte/core/hover-card" -import { prefersReducedMotion } from "@opencode/ui/motion" import { createComputed, createSignal, For, onCleanup, Show, untrack, type JSXElement } from "solid-js" import { useLanguage } from "@/runtime/i18n/language" import { createMediaQuery } from "@solid-primitives/media" import { INITIAL_TAB_PREVIEW_STATE, - selectTabPreviewFollowCard, selectTabPreviewInstant, selectTabPreviewOpenDelay, transitionTabPreview, @@ -21,17 +19,6 @@ const CLOSE_DELAY = 0 // One preview is up at a time across every tab strip, so the machine is shared. const [previewState, setPreviewState] = createSignal(INITIAL_TAB_PREVIEW_STATE) -// In the vertical strip, the card follows the pointer's height over its row, clamped to the tabs, so moving down the -// tabs reads as one card following the cursor. The last pointer height over a row, and the rects the card clamps -// against, measured once per card. -let pointerY = 0 - -let rects: { row: DOMRect; bounds: DOMRect; card: number } | undefined - -let measureFrame: number | undefined - -let unwatch: (() => void) | undefined - let warmTimer: ReturnType | undefined function dispatch(event: TabPreviewEvent) { @@ -45,19 +32,6 @@ function dispatch(event: TabPreviewEvent) { function run(effect: TabPreviewEffect) { if (effect.type === "owner.close") return effect.owner.close() - if (effect.type === "follow.start") return follow(effect.trigger, effect.card) - - if (effect.type === "follow.stop") { - unwatch?.() - unwatch = undefined - - if (measureFrame !== undefined) cancelAnimationFrame(measureFrame) - measureFrame = undefined - rects = undefined - - return - } - clearTimeout(warmTimer) warmTimer = undefined @@ -116,12 +90,6 @@ export function TabPreviewPopover(props: { as="div" data-component="session-tab-popover-trigger" tabIndex={-1} - onPointerMove={(event: PointerEvent) => { - pointerY = event.clientY - const card = selectTabPreviewFollowCard(previewState(), triggerEl) - - if (card && triggerEl) place(triggerEl, card) - }} > {props.trigger} @@ -133,15 +101,6 @@ export function TabPreviewPopover(props: { const theme = triggerEl?.closest("[data-theme]")?.getAttribute("data-theme") if (theme) el.setAttribute("data-theme", theme) - - if (triggerEl && props.orientation === "vertical") - dispatch({ - type: "follow.attached", - owner, - trigger: triggerEl, - card: el, - reducedMotion: prefersReducedMotion(), - }) }} data-component="session-tab-popover" // During a rapid tab-hopping streak the preview appears and disappears without animating; only the first, @@ -183,60 +142,3 @@ export function TabPreviewPopover(props: { ) } - -function follow(trigger: HTMLElement, card: HTMLElement) { - rects = undefined - watch(trigger, card) - place(trigger, card) - - // The card has no height while it mounts, so clamp again once it's laid out. - if (rects) return - measureFrame = requestAnimationFrame(() => { - measureFrame = undefined - place(trigger, card) - }) -} - -// Moves the card straight to the pointer, with no spring. -function place(trigger: HTMLElement, card: HTMLElement) { - if (!card.isConnected || prefersReducedMotion()) return - card.style.transform = `translateY(${cursorOffset(trigger, card)}px)` -} - -// Keeps the pointer level with the middle of the row-sized slot the card would sit in by default, so the offset is 0 -// at the row's centre and runs continuously across row boundaries, clamped to the sidebar. -function cursorOffset(trigger: HTMLElement, card: HTMLElement) { - const measured = rects ?? measure(trigger, card) - - // The card has no height until it's attached, so a measurement taken as it mounts isn't kept. - if (measured.card > 0) rects = measured - const top = pointerY - measured.row.height / 2 - const clamped = Math.min(Math.max(top, measured.bounds.top), measured.bounds.bottom - measured.card) - - return clamped - measured.row.top -} - -function measure(trigger: HTMLElement, card: HTMLElement) { - const bounds = (trigger.closest('[data-slot="vertical-tabs"]') ?? trigger).getBoundingClientRect() - - return { row: trigger.getBoundingClientRect(), bounds, card: card.offsetHeight } -} - -// While a card is up, scrolling or resizing the tabs moves the rows under the cached rects, so measure again and -// re-clamp the card even if the pointer stays still. -function watch(trigger: HTMLElement, card: HTMLElement) { - const refresh = () => { - rects = undefined - place(trigger, card) - } - - const observer = new ResizeObserver(refresh) - observer.observe(trigger.closest('[data-slot="vertical-tabs"]') ?? trigger) - window.addEventListener("scroll", refresh, { capture: true, passive: true }) - window.addEventListener("resize", refresh, { passive: true }) - unwatch = () => { - observer.disconnect() - window.removeEventListener("scroll", refresh, { capture: true }) - window.removeEventListener("resize", refresh) - } -}