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() }