fix(desktop): prevent context menu and popover flash-close during streaming generation#3608
Conversation
…eaming generation Root cause: AnchoredPopover and ContextMenu both listen for scroll events on window to close themselves. During AI streaming generation, the transcript area auto-scrolls on every incoming token (Transcript.tsx:218-226), which triggers window scroll events and instantly closes any open menu/popover before the user can interact with it. Fix: remove the close-on-scroll listener from both components. Menus still close on Escape key, window resize, click-outside (ContextMenu via pointerdown, AnchoredPopover via backdrop), which are the standard and predictable ways to dismiss a menu. Also fixes AnchoredPopover's useLayoutEffect missing dependency array [open, align, offset, placement] to reduce unnecessary position recalculations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d8cb764d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const next = calculatePosition(anchor, menu, align, offset, placement); | ||
| setPosition((current) => (samePosition(current, next) ? current : next)); | ||
| }); | ||
| }, [open, align, offset, placement]); |
There was a problem hiding this comment.
Recompute popover position when content changes
Because this layout effect now only reruns for open/placement props, an already-open popover is not measured again when its children change size. For example, ModelSwitcher opens with the empty state and then asynchronously calls setModels(...); with the default upward placement, the initial top is based on the short empty menu, so once the model rows arrive the fixed-position menu grows downward into the trigger/status area (or off-screen) instead of remaining anchored above it. Consider keeping the measurement on content updates or using a ResizeObserver so dynamic popovers are repositioned after their dimensions change.
Useful? React with 👍 / 👎.
| window.addEventListener("resize", close); | ||
| window.addEventListener("scroll", closeOnScroll, true); | ||
| window.addEventListener("keydown", closeOnEscape); |
There was a problem hiding this comment.
Close context menus when their scrollable list moves
With the scroll listener removed, a menu opened in a scrollable surface such as the history list (styles.css lines 6138-6140) stays fixed on screen when the user scrolls that list with a wheel/trackpad, while the row underneath moves away. The menu actions still target the session captured when it was opened (HistoryPanel.tsx lines 52-53 and 438), so after scrolling it is easy to invoke an action on a different item than the one now visually under the menu. Consider closing or repositioning on user-initiated non-menu scrolls while excluding the streaming autoscroll case.
Useful? React with 👍 / 👎.
esengine
left a comment
There was a problem hiding this comment.
Reviewed the diff: the fix is correct, focused, and CI is green. Approving for merge.
Problem
During AI streaming generation, right-click context menus and popover dropdowns (branch selector, recent files menu, etc.) flash open and immediately close, making them unreadable and unclickable.
Root Cause
Both
AnchoredPopoverandContextMenulisten forscrollevents onwindowto close themselves. During streaming, the transcript area auto-scrolls on every incoming token (Transcript.tsx:218-226), which fires windowscrollevents that instantly close any open menu.Fix
AnchoredPopover.tsx— Remove thecloseOnScrolllistener. The popover still closes on Escape key, window resize, and backdrop click. Also fixeduseLayoutEffectmissing dependency array[open, align, offset, placement]to reduce unnecessary layout recalculations during streaming re-renders.ContextMenu.tsx— Remove thecloseOnScrolllistener. The menu still closes on Escape key, window resize, and click-outside (viapointerdown).Impact
AnchoredPopoverinstances (branch selector, recent files menu)/slash command menu and@file menu are not affected (they never had global scroll listeners)Closes #TBD