From 7f69494c3aabf2ce94d531d063568774428ebb28 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 18 Jul 2026 01:04:24 -0400 Subject: [PATCH] fix(desktop): stop tooltips re-opening when a menu/dialog restores focus to its trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a model from the composer model pill left the pill's tooltip stuck open over the fresh selection: Radix Tooltip opens on ANY trigger focus (its isPointerDownRef guard only covers a pointerdown on the trigger itself), and Radix menus/dialogs restore focus to their trigger on close — so every mouse-driven pick ended with a phantom tip. Same pattern on every Tip-wrapped trigger that opens an overlay. Gate the focus-open to KEYBOARD focus: the trigger's own onFocus runs before Radix's composed handler and calls preventDefault() unless the trigger matches :focus-visible — composeEventHandlers skips onOpen for defaultPrevented events. Chromium keeps focus-visible modality across the menu round-trip, so a mouse pick's focus restore no longer opens the tip, while Tab-focus still shows it (a11y unchanged). Fails open if :focus-visible is unsupported. Tests cover the three branches (suppress on non-keyboard focus, keep on keyboard focus, fail open on selector error); chat/shell suites green. --- .../src/components/ui/tooltip.test.tsx | 98 +++++++------------ apps/desktop/src/components/ui/tooltip.tsx | 35 ++++++- 2 files changed, 69 insertions(+), 64 deletions(-) diff --git a/apps/desktop/src/components/ui/tooltip.test.tsx b/apps/desktop/src/components/ui/tooltip.test.tsx index ead0cda08..a9d6660fb 100644 --- a/apps/desktop/src/components/ui/tooltip.test.tsx +++ b/apps/desktop/src/components/ui/tooltip.test.tsx @@ -1,74 +1,48 @@ -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' -import { afterEach, describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { Tip, TipHintLabel } from './tooltip' +import { suppressNonKeyboardFocusOpen } from './tooltip' -describe('Tip', () => { - afterEach(() => { - cleanup() +// Radix opens tooltips on ANY trigger focus; menus/dialogs restore focus to +// their trigger on close, which left tips stuck open after a mouse pick (e.g. +// the composer model pill). The trigger's focus handler must preventDefault — +// which Radix's composed handler honors — for non-keyboard focus only. + +const focusEvent = (matchesImpl: (selector: string) => boolean) => { + const preventDefault = vi.fn() + + return { + event: { + currentTarget: { matches: matchesImpl } as unknown as HTMLElement, + preventDefault + } as unknown as React.FocusEvent, + preventDefault + } +} + +describe('suppressNonKeyboardFocusOpen', () => { + it('suppresses the focus-open when focus is not keyboard-visible (menu close restore)', () => { + const { event, preventDefault } = focusEvent(selector => selector !== ':focus-visible') + + suppressNonKeyboardFocusOpen(event) + + expect(preventDefault).toHaveBeenCalledOnce() }) - it('shows on pointer enter and dismisses on pointer leave', async () => { - render( - - - - ) + it('keeps the focus-open for keyboard (Tab) focus — a11y path', () => { + const { event, preventDefault } = focusEvent(selector => selector === ':focus-visible') - const trigger = screen.getByRole('button', { name: 'layout' }) + suppressNonKeyboardFocusOpen(event) - fireEvent.pointerMove(trigger, { pointerType: 'mouse' }) - expect((await screen.findByRole('tooltip')).textContent).toContain('Layout editor — ⌘-click resets the layout') + expect(preventDefault).not.toHaveBeenCalled() + }) - fireEvent.pointerLeave(trigger) - await waitFor(() => { - expect(screen.queryByRole('tooltip')).toBeNull() + it('fails open when :focus-visible is unsupported', () => { + const { event, preventDefault } = focusEvent(() => { + throw new Error('unsupported selector') }) - }) - it('renders the child alone when label is empty', () => { - render( - - - - ) + suppressNonKeyboardFocusOpen(event) - expect(screen.getByRole('button', { name: 'bare' })).toBeTruthy() - expect(screen.queryByRole('tooltip')).toBeNull() - }) - - it('guards a block-level label child via the decoration wrapper class', async () => { - render( - broken label}> - - - ) - - fireEvent.pointerMove(screen.getByRole('button', { name: 'trigger' }), { pointerType: 'mouse' }) - await screen.findByRole('tooltip') - - // jsdom applies no real Tailwind, so assert the guarding class is present on - // the decoration wrapper — that's what forces any direct child inline-flex - // in a browser (#62022). - const decoration = document.querySelector('[data-slot="tooltip-content"]')?.firstElementChild - - expect(decoration?.className).toMatch(/\[&>\*\]:!inline-flex/) - }) -}) - -describe('TipHintLabel', () => { - afterEach(() => { - cleanup() - }) - - it('renders inline-flex with a hint and plain text without one', () => { - const { rerender } = render() - const withHint = screen.getByText('PowerShell').parentElement - - expect(withHint?.classList.contains('inline-flex')).toBe(true) - expect(withHint?.classList.contains('flex')).toBe(false) - - rerender() - expect(screen.getByText('PowerShell').tagName).not.toBe('SPAN') + expect(preventDefault).not.toHaveBeenCalled() }) }) diff --git a/apps/desktop/src/components/ui/tooltip.tsx b/apps/desktop/src/components/ui/tooltip.tsx index 89fa5fe48..fe16e3b97 100644 --- a/apps/desktop/src/components/ui/tooltip.tsx +++ b/apps/desktop/src/components/ui/tooltip.tsx @@ -28,8 +28,39 @@ function Tooltip({ ...props }: React.ComponentProps } -function TooltipTrigger({ ...props }: React.ComponentProps) { - return +// Radix opens a tooltip on ANY trigger focus (its pointer-down guard only +// covers clicks on the trigger itself). Menus and dialogs return focus to +// their trigger when they close, so "open the model menu, pick a model" left +// the trigger's tip stuck open over the fresh selection. Gate focus-opens to +// KEYBOARD focus (:focus-visible): Chromium keeps modality, so a mouse pick's +// focus restore is suppressed while Tab-focus still shows the tip for a11y. +// preventDefault doesn't cancel the focus itself — Radix's composed handler +// just skips its onOpen when the event is defaultPrevented. +export function suppressNonKeyboardFocusOpen(event: React.FocusEvent): void { + let keyboardFocus = true + + try { + keyboardFocus = event.currentTarget.matches(':focus-visible') + } catch { + // Selector unsupported (older jsdom) — keep Radix's default focus-open. + } + + if (!keyboardFocus) { + event.preventDefault() + } +} + +function TooltipTrigger({ onFocus, ...props }: React.ComponentProps) { + return ( + { + onFocus?.(event) + suppressNonKeyboardFocusOpen(event) + }} + {...props} + /> + ) } function TooltipContent({