feat(hud): add shouldResizeHudOverlayFallback function and tests#770
feat(hud): add shouldResizeHudOverlayFallback function and tests#770maxmedina05 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds and tests ChangesHUD overlay fallback resize
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
electron/windows.ts (1)
314-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant conditional check.
Due to the control flow in this function, the arguments passed to
shouldResizeHudOverlayFallbackare statically known at this point:
mousePassthroughSupportedis guaranteed to befalsebecause of theif (!mousePassthroughSupported)check on line 315.hudOverlayRecordingActiveis guaranteed to befalsebecause of the early return on line 307 (if (hudOverlayRecordingActive) { ... return; }).Consequently,
shouldResizeHudOverlayFallback(false, false)will always evaluate totrue, making thisifblock a tautology. Consider whether the helper function is necessary here or if the logic should be consolidated.♻️ Proposed consolidation
If you intend to keep the existing early returns, you can simplify this block by removing the redundant condition:
const mousePassthroughSupported = isHudOverlayMousePassthroughSupported(); if (!mousePassthroughSupported) { - if (shouldResizeHudOverlayFallback(mousePassthroughSupported, hudOverlayRecordingActive)) { - setHudOverlayFallbackExpanded(!ignore); - } + setHudOverlayFallbackExpanded(!ignore); hudOverlayWindow.setIgnoreMouseEvents(false); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/windows.ts` around lines 314 - 316, In the fallback branch after the `!mousePassthroughSupported` check, remove the redundant `shouldResizeHudOverlayFallback` conditional and retain its body directly. Preserve the existing early return for `hudOverlayRecordingActive` and the surrounding mouse-passthrough flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@electron/windows.ts`:
- Around line 314-316: In the fallback branch after the
`!mousePassthroughSupported` check, remove the redundant
`shouldResizeHudOverlayFallback` conditional and retain its body directly.
Preserve the existing early return for `hudOverlayRecordingActive` and the
surrounding mouse-passthrough flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48f83e02-a37e-472c-90db-fcf01ce188ce
📒 Files selected for processing (3)
electron/hudOverlayBounds.test.tselectron/hudOverlayBounds.tselectron/windows.ts
1 participant
Description
Re-lands
e2802bf(#612), which was reverted as collateral ind2796fb(#656). On LinuxisHudOverlayMousePassthroughSupported()is always false, so the HUD is a fixed 860x160 window instead of a full work-area overlay. Menus taller than 160 DIP get clipped at the window edge. #612 grew the window to 540 DIP while a menu is open; the revert restored theprocess.platform !== "linux"guard that skips that resize.Part of a series of Linux fixes for Ubuntu 24.04; each PR is standalone and touches disjoint files.
Motivation
#656 set out to revert Windows HUD passthrough regressions introduced after v1.3.1, and swept up the Linux HUD fallback in the same "revert the whole area" pass. No issue or review comment attributes any regression to the Linux part.
Confirmed with
git merge-base: v1.3.3 containse2802bfand notd2796fb. That is exactly why the shipped 1.3.3 AppImage renders HUD menus correctly while currentmainclips them.This restores the fix as a small predicate —
shouldResizeHudOverlayFallback(mousePassthroughSupported, recordingActive)— that only expands the non-passthrough HUD when idle, and keeps it compact while recording. It does not touch the Windows passthrough paths #656 was protecting.Type of Change
Related Issue(s)
Re-lands #612 (reverted by #656).
Screenshots / Video
Not a new UI feature — the difference is that Linux HUD menus render fully instead of clipping at the 160 DIP window edge. A before/after of an open HUD menu (e.g. the language menu) on Linux shows the clipped vs. full menu.
Testing Guide
npx vitest --run electron/hudOverlayBounds.test.ts npm run dev # on Linux, open a HUD menu (e.g. language); it should render fully, not clipVerified on Ubuntu 24.04 (GNOME, X11): HUD menus render fully instead of clipping. The predicate keeps the HUD compact while recording is active and only expands when idle.
Checklist
Notes
If the Linux fallback was reverted deliberately rather than as collateral, say so and I'll close this — but I could not find any report attributing a regression to it.
Investigated and drafted with Claude Code. The behaviour described is from real runs on Ubuntu 24.04, not generated — I can re-run on request.
Thank you for contributing!
Summary by CodeRabbit