[HDX-4405] fix(app): prevent stranded tooltip in virtualised table rows#2380
[HDX-4405] fix(app): prevent stranded tooltip in virtualised table rows#2380alex-fedotyev wants to merge 4 commits into
Conversation
Per-row Tooltip.Floating instances got stranded in their Portal when a virtual row unmounted before onMouseLeave fired — a race that occurs when the mouse moves rapidly across a TanStack Virtual list. Fix: replace one Tooltip.Floating per virtual row with a single shared Tooltip.Floating wrapping the whole <tbody>. The floating tooltip now lives on <tbody>, which never unmounts, so its Portal-rendered content can never be left open after the triggering element disappears. Row-level onMouseEnter/onMouseLeave handlers update a shared hoveredRowDescription state; the tooltip's disabled prop gates visibility so rows without a resolved URL (error-toast branch) never show a hint. A tbody-level onMouseLeave acts as a safety net to clear the description if a rapid mouse move causes a row to unmount before its own leave handler fires. Test: adds a regression test that verifies the tooltip disappears on mouseLeave (the stranded-tooltip scenario).
alex-fedotyev
added
the
ai-generated
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
E2E Test Results✅ All tests passed • 192 passed • 3 skipped • 1312s
Tests ran across 4 shards in parallel. |
Verifies that the Tooltip.Floating hint appears on row hover and disappears when the mouse moves away, covering the stranded-tooltip regression introduced by the per-row Tooltip.Floating in PR #2321. Changes: - dashboard-table-linking.spec.ts: new 'Tooltip hint appears on hover and disappears on mouse-leave' test. Creates a table tile with a Search row-click action, hovers the first row to confirm the hint appears, then moves the mouse away to confirm it hides cleanly. - DashboardPage.ts: adds getFirstTableRow() and hoverFirstTableRowAndGetTooltip() page-object helpers.
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Deep Review✅ No critical issues found. 🟡 P2 -- recommended
🔵 P3 nitpicks (5)
Reviewers (5): correctness, julik-frontend-races, kieran-typescript, testing, maintainability. Testing gaps:
|
P2 — correctness:
- Store hoveredVirtualIndex (row index) instead of hoveredRowDescription
(string) so the label re-derives via useMemo on every render. If the
virtualiser drops or replaces the hovered row (scroll, auto-refetch,
rapid cursor movement) the new row's action is shown immediately;
stale text from the unmounted row can never persist.
- Rows with url:null or empty description now correctly disable the
tooltip regardless of what the prior hover state was.
P2 — testing:
- Replaced the simple mouseLeave regression test with one that exercises
the actual race: hover index 0 (URL row), then enter index 1 (no-URL
row) without firing mouseLeave on index 0. Asserts tooltip hides by
inspecting the Mantine inline display style on the Portal container.
P3 — maintainability:
- label={hoveredRowDescription} — drop the ?? '' fallback that obscured
the disabled gating relationship (Mantine accepts null as ReactNode).
- disabled={!hoveredRowDescription} — guards against empty-string
descriptions that would mount a zero-width floating tooltip.
- Unconditional onMouseEnter/onMouseLeave on each <tr> with a hoisted
clearHovered useCallback, replacing the conditional handler pattern
that forked JSX unnecessarily.
- Collapse dual comment blocks into one rationale at the Tooltip.Floating
call site; the state declaration now has a single-line pointer.
- Add data-testid="row-action-hint" to the Tooltip.Floating label span
so E2E tests locate the tooltip by stable testid rather than by
hard-coupled copy strings.
MikeShi42
left a comment
MikeShi42
left a comment
There was a problem hiding this comment.
fix makes sense - though i think it's a bit "aggressive" that the tooltip follows the cursor so closely, would it make more sense to have the action on the right side of the table?
2 participants
Summary
After #2321 introduced per-row
Tooltip.Floatinginstances inside a TanStack Virtual list, the tooltip would occasionally stay rendered on screen when the mouse moved rapidly away from a row. The root cause is a race condition: when the mouse exits a row quickly, the virtual row can unmount beforeonMouseLeavefires, leaving the tooltip's Portal-rendered content stuck asopened: truewith no element left to dismiss it.Fix: Hoist the single
Tooltip.Floatingup to wrap<tbody>instead of each individual<tr>. Because<tbody>never unmounts, the tooltip's state is never orphaned. Row-levelonMouseEnter/onMouseLeavehandlers update a sharedhoveredRowDescriptionstate; the tooltip'sdisabledprop gates visibility so rows without a resolved URL (error-toast branch) never show a hint. A<tbody>-levelonMouseLeaveacts as a safety net to clear the description if a rapid mouse move causes a row to unmount before its own leave handler fires.Screenshots or video
How to test on Vercel preview
Preview routes: /dashboards
Steps:
onClickaction (e.g. "Search Traces" or "Open Dashboard").References