Hide orchestration hover card as soon as cursor leaves the pill#10221
Hide orchestration hover card as soon as cursor leaves the pill#10221
Conversation
Exiting the hover card in certain ways (cursor leaving the app window, focus changes, layout drops mid-hover, the cursor passing through a stale Hoverable that suppresses the synthetic hover-out) could leave `hovered_pill` stuck on a stale id and the card visible until something else triggered a re-render. The pill's `on_hover(false)` callback didn't fire in those cases, so the `SetHoveredPill(None)` action was never dispatched. Add a defensive render-time check: only render the hover card if the underlying `MouseState::is_mouse_over_element` for the hovered pill still reports the cursor is over it. This makes the overlay strictly track the cursor — as soon as the pointer moves off the pill, the next render hides the card, regardless of whether the typed-action callback fired. Trade-off: the existing 80ms hover-out smoothing (intended to bridge the 6px gap between pill and card) becomes a no-op at the render layer because `is_mouse_over_element` is not delayed. The card was never meant to be interactive (its docstring already notes the card disappears as soon as the pointer leaves the pill body), so losing the smoothing is acceptable in exchange for guaranteed dismissal. Co-Authored-By: Oz <[email protected]>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a render-time guard so the orchestration pill hover card is only displayed while the corresponding pill's mouse-state handle still reports the cursor over that pill.
Concerns
- The change is user-visible but the PR includes no screenshot or video evidence; the repository review guidance requires visual evidence for UI-impacting behavior changes. For faster review, please upload screenshots or a video of the hover card dismissing when the cursor leaves the pill.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
cephalonaut
left a comment
cephalonaut
left a comment
There was a problem hiding this comment.
Looks good, though I wonder if this is a general problem with hovers in the app? I've seen code diff hovers get stuck as well.
2 participants
Description
Exiting the orchestration pill bar's hover details card in certain ways (cursor leaving the app window, focus changes, layout drops mid-hover, or the cursor passing through a stale
Hoverablethat suppresses the synthetic hover-out) could leave the card visible after the cursor was no longer over any pill. The pill'son_hover(false)callback didn't fire in those cases, so theSetHoveredPill(None)typed action was never dispatched andhovered_pillgot stuck on a stale id until something else triggered a re-render.Fix: add a defensive render-time check in
OrchestrationPillBar::render. Only render the hover card if the underlying pill'sMouseState::is_mouse_over_elementstill reports the cursor as being over it. The overlay now strictly tracks the cursor — as soon as the pointer moves off the pill, the next render hides the card, regardless of whether the typed-action callback fired.Trade-off: the existing 80ms
HOVER_CARD_OUT_DELAY(intended to bridge the 6px gap between pill and card) becomes a no-op at the render layer becauseis_mouse_over_elementis not delayed. The card was never meant to be interactive (its docstring already notes the card disappears as soon as the pointer leaves the pill body), so losing the smoothing is acceptable in exchange for guaranteed dismissal.Screenshots / Videos
N/A — this is a removal of a sticky-overlay state. Pre-fix: hover card stays visible after cursor moves off pill in certain exit paths. Post-fix: hover card disappears as soon as the cursor leaves the pill.
Testing
Manual repro:
The check uses
MouseState::is_mouse_over_elementwhich is already maintained byHoverableon every mouse-move event, so no new state is introduced.