fix(hud): dismiss the HUD popovers when the window loses focus - #450
Conversation
The language menu and the device-settings panel already close on Escape and on a pointerdown outside them, and both of those still work — six of the eight new tests pass against the unpatched component. What was missing is the case the issue actually describes. The HUD's native window is only about 904x698, bottom-centred, and mostly invisible reserve. A click anywhere else on screen never reaches this renderer as a pointerdown, so nothing hit-tests it and the popover stays open — while that same click takes keyboard focus away, after which Escape cannot be delivered to this renderer either. The menu is then stuck until the trigger button is found and pressed again, which is exactly what #435 reports for both halves. `blur` is the one signal that does cross that boundary, so close the pair on it. Escape itself needs no change: it already works whenever the window has focus, and after this a popover can only be open in a window that has focus. Registered in bubble phase on purpose — element blur does not bubble, so this never fires while focus moves between the menu's own buttons. Adds the dismissal coverage that was missing entirely: Escape (and that it does not change the locale), outside pointerdown, inside pointerdown, window blur, and a non-Escape key, for the language menu and the device-settings panel alike. Fixes #435
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughChangesHUD popover dismissal
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change dismisses HUD popovers when the window loses focus and includes focused test coverage; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
A HUD popover — the language menu or the device-settings panel — stays open when you click anywhere outside the HUD's own window, and cannot then be dismissed by any means except finding its trigger again.
Fixes #435.
One listener:
Root cause
The HUD's native window is ~904×698, centred at the bottom of the primary display. A click that lands beyond that rectangle reaches this renderer as nothing at all — there is no DOM
pointerdownto hit-test, so no renderer-side listener can ever see it. The same click takes keyboard focus away, which makesEscapeundeliverable here too. The popover is then stuck: both dismiss paths the component owns are, by construction, unreachable from that state.bluris the one signal that does cross. The renderer receives it — measured on a packaged build, not assumed — and nothing anywhere in the app acted on it:grepfor ablurlistener oversrc/components/launch/andelectron/returns none, andcreateHudOverlayWindownever wiresBrowserWindow's.Closing on blur removes the state rather than patching its symptoms: after this, "a popover is open in a window that has no keyboard focus" cannot occur, so the two existing paths only ever have to work while the window is focused — which they do.
Bubble phase is deliberate. Element
blurdoes not bubble, so a window-level bubble listener fires only for the window itself and never while focus moves between the menu's own buttons. Capture phase would have broken keyboard navigation inside the menu.Escape is not broken — and the issue title used to say it was
Worth stating plainly, because this branch was reviewed once against the old title and refused for not touching the
Escapepath. It should not touch it.handleEscapeworks. A maintainer pressingEscon a real keyboard closes the menu. Two automated passes concluded otherwise; both were measuring their own tooling.Settled with a probe attached to the HUD renderer over the remote debugging port — observation only, recording every
keydownin capture, bubble and document-capture phases, then driving the HUD with the same synthetic input:aisTrusted: true,hasFocus: true, targetBUTTONEscape, immediately afterSo synthetic keys reach the HUD and
Escapespecifically is swallowed upstream of the app. The control key is what makes that reading conclusive: without sendingaalongside, the same data reads as "no key gets through".The issue has been re-titled to the behaviour that does reproduce.
Verification
Measured on the packaged 1.9.6 build with real OS input,
LaunchWindow.tsx/HudControls.tsxbyte-identical tomain:pointerdownpath is fine)Escapecannot recover it — the bug0x00200000with a popover open, i.e.WS_EX_TRANSPARENTcleared, confirming the window really is input-opaque at that moment and a click reached it rather than passing throughGates on the rebased branch (
origin/main@1cc63df4):Eight new tests in
LaunchWindow.test.tsxcover all three dismiss paths for both surfaces, and that Escape leaves the locale unchanged. They are jsdom-dispatched, so per AGENTS.md they pin renderer wiring and prove nothing about real reachability on a click-through window. They are necessary, not sufficient; the real-input results above are the other half.What this does not cover
blurwas measured firing on Windows. Nobody has checked that it fires when a click lands outside the HUD on macOS, or on a Spaces switch. If it turns out not to, the belt-and-braces fix is awin.on("blur")increateHudOverlayWindowforwarded over IPC — deliberately not added here, since the renderer event demonstrably arrives on the platform this was measured on and a second code path for a signal that already comes through is not free.electron/main.tsparents to the focused window. A parented modal blurs its owner, so that panel now closes behind the dialog instead of staying open underneath it. The dialog carries the verdict and the panel is one click away, so this seems acceptable — but it is a real difference.🤖 Generated with Claude Code
Summary by CodeRabbit