Skip to content

Commit 2b45ec0

Browse files
committed
Bugfix: Stop tooltip jumping to window corner
- A virtual-scroll row recycled while hovered removes its DOM node without firing `mouseleave`, so the 400ms show-timer leaked and later fired against a detached node. A detached element reports an all-zero `getBoundingClientRect()`, so the tooltip landed in the window's top-left corner instead of over the row. Reproduced in a large directory (45k+ files) where async recursive-size enrichment re-renders rows under a stationary cursor. - Root cause: `destroy()` only cancelled a pending timer when `activeElement === node`, but `activeElement` is null during the show delay. Now the timer's owning node is tracked via `timerNode` and cancelled on teardown. - Defense in depth: `showTooltip` / `positionTooltip` bail when the trigger is detached (`!el.isConnected`). - Added `tooltip.test.ts` covering both teardown paths plus a connected-trigger sanity check, and a gotcha in `lib/ui/CLAUDE.md`.
1 parent 58d1096 commit 2b45ec0

3 files changed

Lines changed: 119 additions & 0 deletions

File tree

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { tooltip } from './tooltip'
3+
4+
describe('tooltip', () => {
5+
beforeEach(() => {
6+
document.body.innerHTML = ''
7+
vi.useFakeTimers()
8+
})
9+
10+
afterEach(() => {
11+
vi.useRealTimers()
12+
})
13+
14+
function makeTrigger(): HTMLElement {
15+
const el = document.createElement('div')
16+
el.textContent = 'row'
17+
document.body.appendChild(el)
18+
return el
19+
}
20+
21+
// Regression: a virtual-scroll row recycled while hovered used to leak its 400ms show-timer (removing
22+
// a node fires no `mouseleave`), which then fired against the detached node — `getBoundingClientRect`
23+
// returns all-zero for a detached element, so the tooltip landed in the top-left corner of the window.
24+
it('does not show when the trigger is destroyed during the show delay', () => {
25+
const el = makeTrigger()
26+
const action = tooltip(el, 'Folder info')
27+
el.dispatchEvent(new MouseEvent('mouseenter'))
28+
29+
// Svelte tearing down a recycled row: it removes the node and calls the action's destroy().
30+
el.remove()
31+
action.destroy?.()
32+
33+
vi.advanceTimersByTime(500)
34+
35+
expect(document.querySelector('.cmdr-tooltip.visible')).toBeNull()
36+
})
37+
38+
// Defense in depth: even if the timer fires against a detached node (no destroy ran — e.g. the trigger
39+
// was removed as part of a larger subtree), the tooltip must never become visible in the corner.
40+
it('does not show when the timer fires against a detached node', () => {
41+
const el = makeTrigger()
42+
tooltip(el, 'Folder info')
43+
el.dispatchEvent(new MouseEvent('mouseenter'))
44+
45+
el.remove()
46+
47+
vi.advanceTimersByTime(500)
48+
49+
expect(document.querySelector('.cmdr-tooltip.visible')).toBeNull()
50+
})
51+
52+
// Sanity: a still-connected trigger shows and positions the tooltip normally (not dumped at the corner).
53+
it('shows and positions the tooltip for a connected trigger', () => {
54+
const el = makeTrigger()
55+
// happy-dom does no layout, so feed a real rect for the positioning math.
56+
vi.spyOn(el, 'getBoundingClientRect').mockReturnValue({
57+
left: 100,
58+
top: 100,
59+
right: 150,
60+
bottom: 120,
61+
width: 50,
62+
height: 20,
63+
x: 100,
64+
y: 100,
65+
toJSON: () => ({}),
66+
})
67+
68+
tooltip(el, 'Folder info')
69+
el.dispatchEvent(new MouseEvent('mouseenter'))
70+
vi.advanceTimersByTime(500)
71+
72+
const tip = document.querySelector('.cmdr-tooltip')
73+
expect(tip).not.toBeNull()
74+
expect(tip?.classList.contains('visible')).toBe(true)
75+
expect(tip?.textContent).toBe('Folder info')
76+
expect((tip as HTMLElement).style.top).not.toBe('0px')
77+
})
78+
})

apps/desktop/src/lib/tooltip/tooltip.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ let tooltipEl: HTMLDivElement | null = null
1414
let tooltipIdCounter = 0
1515
let activeElement: HTMLElement | null = null
1616
let showTimer: ReturnType<typeof setTimeout> | null = null
17+
/** The element a pending show-timer belongs to, so we can cancel it if that element is torn down. */
18+
let timerNode: HTMLElement | null = null
1719

1820
/** Shared container for tooltips (keeps them inside a landmark to satisfy axe's `region` rule). */
1921
let tooltipContainer: HTMLDivElement | null = null
@@ -82,7 +84,21 @@ export function escapeHtml(text: string): string {
8284
return div.innerHTML
8385
}
8486

87+
/**
88+
* A trigger removed from the DOM reports an all-zero `getBoundingClientRect()`, so positioning against
89+
* it would dump the tooltip in the top-left corner. `isConnected` is the precise signal for that.
90+
*/
91+
function isTriggerDetached(el: HTMLElement): boolean {
92+
return !el.isConnected
93+
}
94+
8595
function positionTooltip(triggerEl: HTMLElement): void {
96+
// Guards the live-update path (content changes while shown) against a trigger that vanished meanwhile.
97+
if (isTriggerDetached(triggerEl)) {
98+
hideTooltip()
99+
return
100+
}
101+
86102
const tip = ensureTooltipElement()
87103
const triggerRect = triggerEl.getBoundingClientRect()
88104
const tipRect = tip.getBoundingClientRect()
@@ -103,6 +119,11 @@ function positionTooltip(triggerEl: HTMLElement): void {
103119
}
104120

105121
function showTooltip(triggerEl: HTMLElement, param: TooltipParam): void {
122+
// The trigger may have been removed from the DOM during the show delay (e.g. a virtual-scroll row
123+
// recycled while hovered). Never show against a detached element: its rect is all-zero, which would
124+
// place the tooltip in the top-left corner.
125+
if (isTriggerDetached(triggerEl)) return
126+
106127
const tip = ensureTooltipElement()
107128
setTooltipContent(tip, param)
108129
triggerEl.setAttribute('aria-describedby', tip.id)
@@ -132,12 +153,15 @@ function cancelTimer(): void {
132153
clearTimeout(showTimer)
133154
showTimer = null
134155
}
156+
timerNode = null
135157
}
136158

137159
function startShowTimer(triggerEl: HTMLElement, param: TooltipParam): void {
138160
cancelTimer()
161+
timerNode = triggerEl
139162
showTimer = setTimeout(() => {
140163
showTimer = null
164+
timerNode = null
141165
showTooltip(triggerEl, param)
142166
}, SHOW_DELAY_MS)
143167
}
@@ -213,6 +237,14 @@ export function tooltip(node: HTMLElement, param: TooltipParam): ActionReturn<To
213237
node.removeEventListener('blur', handleBlur)
214238
node.removeEventListener('keydown', handleKeyDown)
215239

240+
// Cancel a pending show-timer owned by this node. Svelte removes a virtual-scroll row's DOM node
241+
// without firing `mouseleave`, so without this the timer would fire later against a detached node
242+
// and the tooltip would land in the top-left corner. `activeElement` is still null during the
243+
// delay window, so the `activeElement === node` branch below wouldn't catch it.
244+
if (timerNode === node) {
245+
cancelTimer()
246+
}
247+
216248
// If this element's tooltip is showing, hide it
217249
if (activeElement === node) {
218250
hideTooltip()

apps/desktop/src/lib/ui/CLAUDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ import { tooltip } from '$lib/tooltip/tooltip'
9393
The tooltip element has `white-space: pre-line` and uses global CSS classes, so `<span class="size-mb">` etc. work
9494
inside `{ html }` tooltips. The `html` variant renders via `innerHTML`; only use with trusted content.
9595

96+
**Gotcha (detached trigger → corner tooltip)**: the show is deferred by a 400ms timer. A virtual-scroll row recycled
97+
while hovered is removed from the DOM **without** firing `mouseleave`, so the timer would otherwise survive and later
98+
fire against a detached node — whose `getBoundingClientRect()` is all-zero, dumping the tooltip in the window's top-left
99+
corner. Two guards prevent this and must both stay: (1) the action's `destroy()` cancels a pending timer it owns
100+
(tracked via `timerNode`), since `activeElement` is still null during the delay window; (2) `showTooltip` /
101+
`positionTooltip` bail when `isTriggerDetached(el)` (`!el.isConnected`). Don't replace the `isConnected` check with a
102+
zero-rect heuristic — happy-dom reports zero rects for every connected element, so it false-positives the whole test
103+
suite. Covered by `tooltip.test.ts`.
104+
96105
## Button
97106

98107
Variants: `primary` | `secondary` (default) | `danger`. Sizes: `regular` (default) | `mini`. Extends

0 commit comments

Comments
 (0)