mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-27 17:58:07 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Revert "perf(desktop): mount tooltips lazily"

Browse source 
Reverts the tooltip half of 4798994dc; keeps the idle-cost scenario.

Lazily mounting Radix on first hover measured well (105k -> 26k
TooltipProvider renders per drag) but broke 18 tests across 12 files.
Those tests are not incidental: the repo has an established convention of
asserting `[data-slot="tooltip-trigger"]` at mount to prove a control
carries a tooltip, and deferring the mount invalidates all of them at
once. There is also a real behavior risk the convention was protecting —
`asChild` puts the slot on the button element itself, so arming REPLACES
the node, which is exactly the kind of identity change that breaks focus
restoration and ref-holding call sites.

A 4x cut in tooltip churn is not worth reworking every tooltip assertion
in the app plus taking that risk, on a component with ~107 call sites.
If it's worth revisiting, the right shape is probably making
TooltipProvider itself cheap (one app-level provider) rather than
deferring the mount per call site — that preserves the DOM contract these
tests encode.

The genuine win in this branch stands on its own: the $layoutTree
subscription fix (commits 83 -> 12 on a sash drag) is unaffected.
This commit is contained in:
Brooklyn Nicholson 2026-07-26 18:00:02 -05:00
parent f64b719321
commit 0226d1162e
2 changed files with 1 additions and 96 deletions
Download patch file Download diff file Expand all files Collapse all files

56
apps/desktop/src/components/ui/tooltip.lazy.test.tsx
View file

@ -1,56 +0,0 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react'
import { describe, expect, it } from 'vitest'
import { Tip } from './tooltip'
// `Tip` mounts Radix lazily (on first hover/focus) because ~107 eager
// providers dominated unrelated interactions — dragging the sidebar splitter
// measured 105k TooltipProvider renders across a 60-frame gesture. The saving
// is only worth anything if the tip still actually appears, so this asserts
// the behavior the optimization has to preserve, not its implementation.
describe('Tip lazy mount', () => {
it('shows the tooltip on hover even though Radix mounts only once armed', async () => {
render(
<Tip label="Reveal in sidebar">
<button type="button">target</button>
</Tip>
)
const trigger = screen.getByRole('button', { name: 'target' })
expect(screen.queryByText('Reveal in sidebar')).toBeNull()
// Arms the lazy wrapper, then drives Radix's own trigger handlers for the
// same hover — the pointer is still inside, so the tip must open.
fireEvent.pointerEnter(trigger)
fireEvent.pointerMove(trigger)
await waitFor(() => {
expect(screen.getAllByText('Reveal in sidebar').length).toBeGreaterThan(0)
})
})
it('renders the child untouched when there is no label', () => {
const { container } = render(
<Tip label={undefined}>
<button type="button">bare</button>
</Tip>
)
expect(screen.getByRole('button', { name: 'bare' })).toBeTruthy()
expect(container.querySelector('[data-slot="tooltip-idle"]')).toBeNull()
})
it('does not mount Radix until the pointer arrives', () => {
const { container } = render(
<Tip label="Later">
<button type="button">idle</button>
</Tip>
)
// The idle wrapper is present; no Radix trigger has been created.
expect(container.querySelector('[data-slot="tooltip-idle"]')).toBeTruthy()
expect(container.querySelector('[data-slot="tooltip-trigger"]')).toBeNull()
})
})

41
apps/desktop/src/components/ui/tooltip.tsx
View file

@ -108,53 +108,14 @@ interface TipProps extends Omit<React.ComponentProps<typeof TooltipPrimitive.Con
// position-aware, themed. Self-contained (carries its own Provider) so it works
// anywhere without a provider ancestor. Renders the child untouched when label
// is falsy. Open state is trigger-hover only — never sticky, never click-blocking.
//
// Perf: the Radix machinery is mounted LAZILY, on first hover/focus. `Tip` has
// ~107 call sites, and Radix's `Tooltip` holds real state (trigger element,
// content id, open) while `Popper` subscribes to layout — so eagerly mounting
// all of them made every one of those subtrees re-render during unrelated
// interactions. Measured while dragging the sidebar splitter with five busy
// tiles: 105,385 TooltipProvider renders and ~15s of component time across a
// 60-frame gesture, which is what dropped the drag to ~1.5fps. Until the
// pointer actually arrives, a Tip is just its child plus one wrapper.
function Tip({ label, children, delayDuration = 0, ...props }: TipProps) {
// Once armed, stay armed: unmounting the machinery on pointer-out would
// close the tip mid-hover and re-pay the mount on every re-entry.
const [armed, setArmed] = React.useState<'focus' | 'pointer' | null>(null)
if (!label) {
return <>{children}</>
}
if (!armed) {
// `display: contents` so this wrapper never affects layout — the child
// participates in its parent's box exactly as it did before.
return (
<span
data-slot="tooltip-idle"
onFocus={event => {
suppressNonKeyboardFocusOpen(event)
if (!event.defaultPrevented) {
setArmed('focus')
}
}}
onPointerEnter={() => setArmed('pointer')}
style={{ display: 'contents' }}
>
{children}
</span>
)
}
// `defaultOpen` is required, not incidental: the pointerenter/focus that
// armed this has ALREADY fired, so Radix's own trigger handlers never see it
// and the tip would mount silently closed. Arming IS the hover, so open with
// it — Radix owns every subsequent open/close from here (pointerleave, blur,
// escape, pointerdown) exactly as before.
return (
<TooltipProvider delayDuration={delayDuration} disableHoverableContent>
<Tooltip defaultOpen disableHoverableContent>
<Tooltip disableHoverableContent>
<TooltipTrigger asChild>{children}</TooltipTrigger>
<TooltipContent {...props}>{label}</TooltipContent>
</Tooltip>