Merge pull request #66737 from NousResearch/bb/tooltip-focus-open

fix(desktop): stop tooltips re-opening when a menu/dialog restores focus to its trigger

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<HTMLElement>,
+    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(
-      <Tip label="Layout editor — ⌘-click resets the layout">
-        <button type="button">layout</button>
-      </Tip>
-    )
+  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(
-      <Tip label="">
-        <button type="button">bare</button>
-      </Tip>
-    )
+    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(
-      <Tip label={<span className="flex items-center gap-2">broken label</span>}>
-        <button type="button">trigger</button>
-      </Tip>
-    )
-
-    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<HTMLElement>('[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(<TipHintLabel hint="Ctrl+`" text="PowerShell" />)
-    const withHint = screen.getByText('PowerShell').parentElement
-
-    expect(withHint?.classList.contains('inline-flex')).toBe(true)
-    expect(withHint?.classList.contains('flex')).toBe(false)
-
-    rerender(<TipHintLabel text="PowerShell" />)
-    expect(screen.getByText('PowerShell').tagName).not.toBe('SPAN')
+    expect(preventDefault).not.toHaveBeenCalled()
   })
 })

apps/desktop/src/components/ui/tooltip.tsx

@@ -28,8 +28,39 @@ function Tooltip({ ...props }: React.ComponentProps<typeof TooltipPrimitive.Root
   return <TooltipPrimitive.Root data-slot="tooltip" {...props} />
 }
 
-function TooltipTrigger({ ...props }: React.ComponentProps<typeof TooltipPrimitive.Trigger>) {
-  return <TooltipPrimitive.Trigger data-slot="tooltip-trigger" {...props} />
+// 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<HTMLElement>): 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<typeof TooltipPrimitive.Trigger>) {
+  return (
+    <TooltipPrimitive.Trigger
+      data-slot="tooltip-trigger"
+      onFocus={event => {
+        onFocus?.(event)
+        suppressNonKeyboardFocusOpen(event)
+      }}
+      {...props}
+    />
+  )
 }
 
 function TooltipContent({