From a8d7151b49da4ae72bf1e1c6e762ac9282d584ce Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:14:08 -0700 Subject: [PATCH] Require a full hover delay before each tab preview (#30388) * Require a full hover delay before each tab preview Tab previews opened almost immediately after a previous preview had been shown, so sweeping across the tab strip caused a flurry of popups. Raise the tab tooltip delay to 800 ms and drop the warm-start chain: the shared open-state context, the 300 ms skip delay, and the slide animation that carried position between cards. Every tab now waits its full dwell time, and keyboard focus still shows the preview immediately. * Test that tab hover cards wait a full dwell on every hover The tab hover card no longer slides instantly between neighboring tabs. Each hover now waits for the full delay before showing a preview. The e2e test still expected instant sliding, so it was rewritten to match. - Remove the frame-recording helpers and the slide and reversal assertions. - Check that no card appears before the dwell, and that the card appears after it. - Check that leaving a tab cancels a pending preview, and that time spent on other tabs does not count toward the dwell. - Check that re-entering the same tab requires a fresh dwell. --- .../src/components/tab-bar/TabHoverCard.tsx | 82 +------- .../tab-bar/TabStripTooltipProvider.test.tsx | 191 ++++++------------ .../tab-bar/TabStripTooltipProvider.tsx | 76 +------ tests/e2e/tab-hover-card.spec.ts | 124 +++--------- 4 files changed, 97 insertions(+), 376 deletions(-) diff --git a/src/renderer/src/components/tab-bar/TabHoverCard.tsx b/src/renderer/src/components/tab-bar/TabHoverCard.tsx index b8949d696c2..b116eb5b5cd 100644 --- a/src/renderer/src/components/tab-bar/TabHoverCard.tsx +++ b/src/renderer/src/components/tab-bar/TabHoverCard.tsx @@ -1,10 +1,5 @@ -import { useCallback, useContext, useId, useRef, type ReactNode } from 'react' +import type { ReactNode } from 'react' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { - TabCardOpenContext, - TabCardPlacementContext, - TAB_TOOLTIP_SKIP_DELAY_MS -} from './TabStripTooltipProvider' export function TabHoverCard({ children, @@ -19,79 +14,8 @@ export function TabHoverCard({ programName: string description?: string }): React.JSX.Element { - const placement = useContext(TabCardPlacementContext) - const content = useRef(null) - const cardId = useId() - const openContext = useContext(TabCardOpenContext) - const notifyOpenChange = openContext?.onOpenChange - - const captureClosedPlacement = useCallback( - (element: HTMLDivElement | null) => { - const previous = placement?.current - if (!element || previous?.element !== element) { - return - } - const { left, top } = element.getBoundingClientRect() - previous.left = left - previous.top = top - previous.closedAt = performance.now() - }, - [placement] - ) - - const setContentElement = useCallback( - (element: HTMLDivElement | null) => { - const previous = content.current - if (element) { - // StrictMode reattaches refs without closing the tooltip. - notifyOpenChange?.(cardId, true) - } else if (previous) { - notifyOpenChange?.(cardId, false) - captureClosedPlacement(previous) - } - content.current = element - }, - [captureClosedPlacement, cardId, notifyOpenChange] - ) - - const handlePlaced = (): void => { - const element = content.current - if (!element || !placement) { - return - } - const rect = element.getBoundingClientRect() - const previous = placement.current - if ( - previous && - previous.closedAt !== null && - performance.now() - previous.closedAt < TAB_TOOLTIP_SKIP_DELAY_MS && - !window.matchMedia('(prefers-reduced-motion: reduce)').matches - ) { - // Continue from the painted position when reversing direction during a slide. - const origin = previous.element.isConnected - ? previous.element.getBoundingClientRect() - : previous - element.animate( - [ - { translate: `${origin.left - rect.left}px ${origin.top - rect.top}px`, opacity: 1 }, - { translate: '0 0', opacity: 1 } - ], - { duration: 150, easing: 'ease-out' } - ) - } - placement.current = { element, left: rect.left, top: rect.top, closedAt: null } - } - return ( - { - openContext?.onOpenChange(cardId, open) - if (!open && content.current?.isConnected) { - captureClosedPlacement(content.current) - } - }} - > + { @@ -104,13 +28,11 @@ export function TabHoverCard({ {children}
diff --git a/src/renderer/src/components/tab-bar/TabStripTooltipProvider.test.tsx b/src/renderer/src/components/tab-bar/TabStripTooltipProvider.test.tsx index a14fc884d5f..798d144f541 100644 --- a/src/renderer/src/components/tab-bar/TabStripTooltipProvider.test.tsx +++ b/src/renderer/src/components/tab-bar/TabStripTooltipProvider.test.tsx @@ -1,155 +1,92 @@ // @vitest-environment happy-dom import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' -import { StrictMode, useContext } from 'react' +import { StrictMode } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { TabHoverCard } from './TabHoverCard' -import { - TAB_TOOLTIP_DELAY_MS, - TAB_TOOLTIP_SKIP_DELAY_MS, - TabCardOpenContext, - TabStripTooltipProvider -} from './TabStripTooltipProvider' +import { TAB_TOOLTIP_DELAY_MS, TabStripTooltipProvider } from './TabStripTooltipProvider' -function CardEvents({ id }: { id: string }): React.JSX.Element { - const strip = useContext(TabCardOpenContext) - return ( - <> - - - {strip?.isWarm ? 'immediate' : 'delayed'} - +function renderTabs(): void { + render( + + + } programName="Terminal"> + + + } programName="Codex"> + + + + ) } -function renderTooltip(label: string, tip: string): void { - render( - - - - - - {tip} - - - ) +function hover(name: string): void { + fireEvent.pointerMove(screen.getByRole('button', { name }), { pointerType: 'mouse' }) +} + +function advance(ms: number): void { + act(() => vi.advanceTimersByTime(ms)) } describe('TabStripTooltipProvider', () => { beforeEach(() => { vi.useFakeTimers() + renderTabs() }) afterEach(() => { - vi.useRealTimers() cleanup() + vi.useRealTimers() }) - it('holds a tab tooltip until the full delay elapses', () => { - renderTooltip('tab label', 'tab tooltip') - fireEvent.pointerMove(screen.getByRole('button', { name: 'tab label' }), { - pointerType: 'mouse' - }) - - act(() => { - vi.advanceTimersByTime(TAB_TOOLTIP_DELAY_MS - 1) - }) - expect(screen.queryByText('tab tooltip')).toBeNull() - - act(() => { - vi.advanceTimersByTime(1) - }) - expect(screen.getByText('tab tooltip')).toBeTruthy() + it('holds the first preview until an 800 ms hover', () => { + hover('first tab') + advance(799) + expect(screen.queryByText('Build')).toBeNull() + advance(1) + expect(screen.getAllByText('Build').length).toBeGreaterThan(0) }) - it('opens the next tab immediately after a card was shown', () => { - render( - - - - - - first tip - - - - - - second tip - - - ) + it('requires a full delay on the next tab after a preview was shown', () => { + hover('first tab') + advance(TAB_TOOLTIP_DELAY_MS) + expect(screen.getAllByText('Build').length).toBeGreaterThan(0) - const first = screen.getByRole('button', { name: 'first tab' }) - const second = screen.getByRole('button', { name: 'second tab' }) - - fireEvent.pointerMove(first, { pointerType: 'mouse' }) - act(() => { - vi.advanceTimersByTime(TAB_TOOLTIP_DELAY_MS) - }) - expect(screen.getByText('first tip')).toBeTruthy() - - fireEvent.pointerLeave(first) - fireEvent.pointerMove(second, { pointerType: 'mouse' }) - - act(() => { - vi.advanceTimersByTime(0) - }) - expect(screen.getByText('second tip')).toBeTruthy() - expect(screen.queryByText('first tip')).toBeNull() - - fireEvent.pointerLeave(second) - act(() => { - vi.advanceTimersByTime(301) - }) - fireEvent.pointerMove(first, { pointerType: 'mouse' }) - act(() => { - vi.advanceTimersByTime(TAB_TOOLTIP_DELAY_MS - 1) - }) - expect(screen.queryByText('first tip')).toBeNull() - act(() => { - vi.advanceTimersByTime(1) - }) - expect(screen.getByText('first tip')).toBeTruthy() + fireEvent.pointerLeave(screen.getByRole('button', { name: 'first tab' })) + hover('second tab') + advance(TAB_TOOLTIP_DELAY_MS - 1) + expect(screen.queryByText('Build')).toBeNull() + expect(screen.queryByText('Codex Chat')).toBeNull() + advance(1) + expect(screen.getAllByText('Codex Chat').length).toBeGreaterThan(0) }) - it('ignores a late close from the previous card and resets the delay after the active card closes', () => { - render( - - - - - ) - const delay = screen.getByLabelText('first delay') - expect(delay.textContent).toBe('delayed') - fireEvent.click(screen.getByText('open first')) - fireEvent.click(screen.getByText('close first')) - fireEvent.click(screen.getByText('open second')) - fireEvent.click(screen.getByText('close first')) - act(() => vi.advanceTimersByTime(TAB_TOOLTIP_SKIP_DELAY_MS + 1)) - expect(delay.textContent).toBe('immediate') - - fireEvent.click(screen.getByText('close second')) - act(() => vi.advanceTimersByTime(TAB_TOOLTIP_SKIP_DELAY_MS - 1)) - expect(delay.textContent).toBe('immediate') - act(() => vi.advanceTimersByTime(1)) - expect(delay.textContent).toBe('delayed') + it('cancels previews when passing quickly across tabs', () => { + hover('first tab') + advance(400) + fireEvent.pointerLeave(screen.getByRole('button', { name: 'first tab' })) + hover('second tab') + advance(400) + expect(screen.queryByText('Build')).toBeNull() + expect(screen.queryByText('Codex Chat')).toBeNull() + fireEvent.pointerLeave(screen.getByRole('button', { name: 'second tab' })) + advance(TAB_TOOLTIP_DELAY_MS) + expect(screen.queryByText('Codex Chat')).toBeNull() }) - it('keeps the strip warm while a card is open in StrictMode', () => { - render( - - - - } programName="Terminal"> - - - - - ) - act(() => screen.getByRole('button', { name: 'tab' }).focus()) - expect(screen.getByLabelText('probe delay').textContent).toBe('immediate') - act(() => vi.advanceTimersByTime(TAB_TOOLTIP_SKIP_DELAY_MS + 1)) - expect(screen.getByLabelText('probe delay').textContent).toBe('immediate') + it('restarts the full delay when returning to the same tab', () => { + hover('first tab') + advance(TAB_TOOLTIP_DELAY_MS) + fireEvent.pointerLeave(screen.getByRole('button', { name: 'first tab' })) + hover('first tab') + advance(TAB_TOOLTIP_DELAY_MS - 1) + expect(screen.queryByText('Build')).toBeNull() + advance(1) + expect(screen.getAllByText('Build').length).toBeGreaterThan(0) + }) + + it('still shows previews immediately for keyboard focus', () => { + act(() => screen.getByRole('button', { name: 'first tab' }).focus()) + expect(screen.getAllByText('Build').length).toBeGreaterThan(0) }) }) diff --git a/src/renderer/src/components/tab-bar/TabStripTooltipProvider.tsx b/src/renderer/src/components/tab-bar/TabStripTooltipProvider.tsx index 5bcbbc9335d..4d33ac67d00 100644 --- a/src/renderer/src/components/tab-bar/TabStripTooltipProvider.tsx +++ b/src/renderer/src/components/tab-bar/TabStripTooltipProvider.tsx @@ -1,77 +1,13 @@ -import { - createContext, - useCallback, - useEffect, - useMemo, - useRef, - useState, - type ReactNode, - type RefObject -} from 'react' +import type { ReactNode } from 'react' import { TooltipProvider } from '@/components/ui/tooltip' -export const TAB_TOOLTIP_DELAY_MS = 500 -export const TAB_TOOLTIP_SKIP_DELAY_MS = 300 - -type TabCardPlacement = { - element: HTMLElement - left: number - top: number - closedAt: number | null -} - -export const TabCardPlacementContext = createContext | null>( - null -) - -export const TabCardOpenContext = createContext<{ - isWarm: boolean - onOpenChange: (id: string, open: boolean) => void -} | null>(null) +export const TAB_TOOLTIP_DELAY_MS = 800 export function TabStripTooltipProvider({ children }: { children: ReactNode }): React.JSX.Element { - const placement = useRef(null) - const openCardId = useRef(null) - const cooldownTimer = useRef | null>(null) - const [isWarm, setIsWarm] = useState(false) - const onOpenChange = useCallback((id: string, open: boolean) => { - // A previous card's close may arrive after the next card opens. - if (!open && openCardId.current !== id) { - return - } - if (cooldownTimer.current !== null) { - clearTimeout(cooldownTimer.current) - cooldownTimer.current = null - } - openCardId.current = open ? id : null - if (open) { - setIsWarm(true) - } else { - cooldownTimer.current = setTimeout(() => { - cooldownTimer.current = null - setIsWarm(false) - }, TAB_TOOLTIP_SKIP_DELAY_MS) - } - }, []) - useEffect( - () => () => { - if (cooldownTimer.current !== null) { - clearTimeout(cooldownTimer.current) - } - }, - [] - ) - const openContext = useMemo(() => ({ isWarm, onOpenChange }), [isWarm, onOpenChange]) return ( - - - - {children} - - - + // Every tab needs its own dwell time, even after another preview was shown. + + {children} + ) } diff --git a/tests/e2e/tab-hover-card.spec.ts b/tests/e2e/tab-hover-card.spec.ts index 43daeb0b1e7..0e2c560ea38 100644 --- a/tests/e2e/tab-hover-card.spec.ts +++ b/tests/e2e/tab-hover-card.spec.ts @@ -1,64 +1,8 @@ -import type { JSHandle, Page } from '@playwright/test' import { expect, test } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { startHiddenRendererFrameCapture } from './helpers/hidden-renderer-frame-capture' -type CardFrame = { - title: string - left: number - count: number - sliding: boolean - at: number - states: string[] -} -type CardRecording = { stop: () => CardFrame[] } - -async function startCardRecording(page: Page): Promise> { - return page.evaluateHandle(() => { - const frames: CardFrame[] = [] - const started = performance.now() - let nextFrame = 0 - const record = (): void => { - const cards = document.querySelectorAll('[data-tab-hover-card]') - const element = cards[0] - frames.push({ - at: performance.now() - started, - states: Array.from(cards, (card) => card.getAttribute('data-state') ?? ''), - title: element?.querySelector('[data-tab-hover-card-title]')?.textContent?.trim() ?? '', - left: element?.getBoundingClientRect().left ?? -1, - count: cards.length, - sliding: - element - ?.getAnimations() - .some( - (animation) => - animation.effect instanceof KeyframeEffect && - animation.effect.getKeyframes().some((frame) => frame.translate !== undefined) - ) ?? false - }) - if (performance.now() - started < 10_000) { - nextFrame = requestAnimationFrame(record) - } - } - record() - return { - stop: () => { - cancelAnimationFrame(nextFrame) - return frames - } - } - }) -} - -async function stopCardRecording(recording: JSHandle): Promise { - try { - return await recording.evaluate((recording) => recording.stop()) - } finally { - await recording.dispose() - } -} - -test('whole-tab hover cards slide immediately between neighboring tabs', async ({ +test('whole-tab hover cards wait independently on every hover', async ({ electronApp, orcaPage }) => { @@ -96,7 +40,7 @@ test('whole-tab hover cards slide immediately between neighboring tabs', async ( // The left padding and icon used to sit outside the terminal title's trigger. await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) - await orcaPage.waitForTimeout(200) + await orcaPage.waitForTimeout(600) await expect(card).toHaveCount(0) await expect(cardTitle).toHaveText('Build and test the hover cards') const program = card.locator('[data-tab-hover-card-program]') @@ -105,51 +49,32 @@ test('whole-tab hover cards slide immediately between neighboring tabs', async ( const titleBox = await cardTitle.boundingBox() const programBox = await program.boundingBox() expect(programBox?.y).toBeGreaterThan((titleBox?.y ?? 0) + (titleBox?.height ?? 0)) - await orcaPage.waitForTimeout(200) - - const recording = await startCardRecording(orcaPage) await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) - await orcaPage.waitForTimeout(200) - const frames = await stopCardRecording(recording) - const firstTargetFrame = frames.findIndex((frame) => frame.title === 'Review changes') - expect(firstTargetFrame, JSON.stringify(frames)).toBeGreaterThan(0) - expect( - frames - .slice(0, firstTargetFrame) - .every((frame) => frame.title === 'Build and test the hover cards') - ).toBe(true) - expect(frames.slice(firstTargetFrame).every((frame) => frame.title === 'Review changes')).toBe( - true - ) - expect(frames.some((frame) => frame.sliding)).toBe(true) - expect(frames[firstTargetFrame]?.left).toBeLessThan(secondBox.x - 10) + await orcaPage.waitForTimeout(600) + await expect(card).toHaveCount(0) await expect(cardTitle).toHaveText('Review changes') - await orcaPage.waitForTimeout(500) + + // Leaving cancels a pending preview; time spent on other tabs does not count. + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) + await orcaPage.waitForTimeout(300) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) + await orcaPage.waitForTimeout(300) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height + 100) + await orcaPage.waitForTimeout(900) + await expect(card).toHaveCount(0) await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) - await expect(cardTitle).toHaveText('Build and test the hover cards', { timeout: 150 }) await orcaPage.waitForTimeout(600) - await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) + await expect(card).toHaveCount(0) + await expect(cardTitle).toHaveText('Build and test the hover cards') - const reversalRecording = await startCardRecording(orcaPage) - for (let index = 0; index < 12; index += 1) { - const target = index % 2 === 0 ? firstBox : secondBox - const title = index % 2 === 0 ? 'Build and test the hover cards' : 'Review changes' - await orcaPage.mouse.move(target.x + 3, target.y + target.height / 2) - await expect(cardTitle).toHaveText(title, { timeout: 150 }) - await orcaPage.waitForTimeout(30) - await expect(card).toHaveCount(1) - } - const reversalFrames = await stopCardRecording(reversalRecording) - expect( - reversalFrames.every((frame) => frame.count === 1), - JSON.stringify(reversalFrames) - ).toBe(true) - for (const title of ['Build and test the hover cards', 'Review changes']) { - expect(reversalFrames.some((frame) => frame.title === title && frame.sliding)).toBe(true) - } + // Re-entering the same tab also requires a fresh dwell. + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height + 100) + await expect(card).toHaveCount(0) + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) + await orcaPage.waitForTimeout(600) + await expect(card).toHaveCount(0) + await expect(cardTitle).toHaveText('Build and test the hover cards') await first.locator('[data-tab-close-button]').hover() await orcaPage.waitForTimeout(100) @@ -170,8 +95,10 @@ test('whole-tab hover cards slide immediately between neighboring tabs', async ( await first.locator('[data-tab-close-button]').focus() await expect(orcaPage.locator('[data-tab-close-tooltip]')).toHaveCount(0) await expect(card).toHaveCount(1) + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height + 100) + await expect(card).toHaveCount(0) await first.focus() - await expect(cardTitle).toHaveText('Build and test the hover cards') + await expect(cardTitle).toHaveText('Build and test the hover cards', { timeout: 150 }) await expect(orcaPage.locator('[data-tab-close-tooltip]')).toHaveCount(0) await first.press('Escape') await expect(card).toHaveCount(0) @@ -185,7 +112,6 @@ test('whole-tab hover cards slide immediately between neighboring tabs', async ( await orcaPage.waitForTimeout(200) await expect(card).toHaveCount(0) await expect(cardTitle).toHaveText('Review changes') - await orcaPage.waitForTimeout(500) } finally { await stopCapture() }