Skip to content

ref(ui): convert useHoverOverlay to state machine#113628

Merged
natemoo-re merged 9 commits intomasterfrom
nm/ui/tooltips
Apr 22, 2026
Merged

ref(ui): convert useHoverOverlay to state machine#113628
natemoo-re merged 9 commits intomasterfrom
nm/ui/tooltips

Conversation

@natemoo-re
Copy link
Copy Markdown
Member

Converts useHoverOverlay hook, which powers Tooltip and Hovercard components, to a proper state machine powered by a HoverOverlayGroupProvider.

Tooltips will now share warmup/cooldown timing (first hover requires 400ms warmup, neighbors within 500ms open instantly), siblings now snap-close even mid-exit-animation.

Also fixes a small bug where onBlur was called onMount.

Before After
Nav Tooltip
nav-tooltip-old.mov
nav-tooltip-new.mov
Icon Hovercard
icon-tooltip-old.mov
icon-tooltip-new.mov

Comment thread static/app/utils/useHoverOverlay.tsx
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f01793d. Configure here.

Comment thread static/app/utils/useHoverOverlay.tsx
Comment thread static/app/utils/useHoverOverlay.tsx
Comment thread static/app/views/navigation/index.tsx
Copy link
Copy Markdown
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a small nit:

Comment thread static/app/utils/useHoverOverlay.tsx Outdated
natemoo-re added a commit that referenced this pull request Apr 22, 2026
Addresses two issues raised by Cursor Bugbot on #113628:

- A cooling overlay that got snap-closed still had its hideTimer running,
  which later fired `startGroupCoolDown` against the newly-warm group. The
  group could then go cold while the incoming overlay was still open,
  forcing neighboring triggers to pay the full warmup delay.

- `warmUpGroup` fired every listener in the group, including idle overlays
  that had never opened. On pages with many tooltips (tables/lists) this
  caused O(N) snap-close state updates and re-renders per hover. Track
  whether an overlay may still be on-screen or mid-exit-animation and skip
  the snap bookkeeping for overlays that have never been hovered.
natemoo-re and others added 8 commits April 22, 2026 16:50
Co-authored-by: Dominik Dorfmeister 🔮 <dominik.dorfmeister@sentry.io>
Addresses two issues raised by Cursor Bugbot on #113628:

- A cooling overlay that got snap-closed still had its hideTimer running,
  which later fired `startGroupCoolDown` against the newly-warm group. The
  group could then go cold while the incoming overlay was still open,
  forcing neighboring triggers to pay the full warmup delay.

- `warmUpGroup` fired every listener in the group, including idle overlays
  that had never opened. On pages with many tooltips (tables/lists) this
  caused O(N) snap-close state updates and re-renders per hover. Track
  whether an overlay may still be on-screen or mid-exit-animation and skip
  the snap bookkeeping for overlays that have never been hovered.
@natemoo-re natemoo-re enabled auto-merge (squash) April 22, 2026 22:15
Comment on lines 392 to 397
useEffect(() => {
return () => {
maybeClearRefTimeout(delayHideTimeoutRef);
maybeClearRefTimeout(delayOpenTimeoutRef);
maybeClearRefTimeout(openTimerRef);
maybeClearRefTimeout(hideTimerRef);
};
}, []);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The useEffect cleanup in useHoverOverlay fails to reset the delay group's warm state on unmount, causing subsequent tooltips in the group to skip their warmup delay.
Severity: MEDIUM

Suggested Fix

In the useEffect cleanup function, check if the overlay was visible (statusRef.current === 'open' || statusRef.current === 'cooling') upon unmounting. If it was, call startGroupCoolDown(group) to correctly reset the group's warm state, mirroring the logic used in the reset and handleMouseLeave functions.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/utils/useHoverOverlay.tsx#L392-L397

Potential issue: When a component using `useHoverOverlay` unmounts while its overlay is
in an 'open' or 'cooling' state, the `useEffect` cleanup logic fails to reset the
`isWarm` state of its delay group. As a result, all subsequent tooltips or hovercards
within the same group will permanently skip their intended warmup delay for the
remainder of the user's session. This is particularly problematic for the global default
group, as it can affect all unscoped tooltips across the application after a single
triggering event, such as navigating away from a page while a tooltip is visible.

Comment on lines +340 to +342
mayBeAnimatingOutRef.current = false;
setSnapClosed(true);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: An overlay that was previously open but is now idle can unnecessarily call setSnapClosed(true) when a sibling overlay opens, causing a wasteful re-render.
Severity: LOW

Suggested Fix

The mayBeAnimatingOutRef flag should be reset to false whenever the overlay's status transitions to 'idle'. This should be done not only within the warmUpGroup listener but also in the handleMouseLeave paths where commitStatus('idle') is called for overlays that close without a delay.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/utils/useHoverOverlay.tsx#L340-L342

Potential issue: The `mayBeAnimatingOutRef` flag is set to `true` when an overlay opens
but is not reset to `false` when the overlay becomes 'idle' through a normal mouse leave
event (one without a close delay). Consequently, when another overlay in the same group
opens, it triggers the `warmUpGroup` listener for the now-idle overlay. Because
`mayBeAnimatingOutRef` is still `true`, the listener doesn't exit early and proceeds to
call `setSnapClosed(true)`. This causes an unnecessary re-render of an already invisible
component, leading to performance degradation on pages with many tooltips.

@natemoo-re natemoo-re merged commit 64596d0 into master Apr 22, 2026
64 checks passed
@natemoo-re natemoo-re deleted the nm/ui/tooltips branch April 22, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants