fix(hud): preserve user-dragged position across recording state changes#224
Merged
webadderall merged 3 commits intowebadderall:mainfrom Apr 13, 2026
Merged
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
HUD Position & Display Handling electron/windows.ts |
Introduce module-level hudUserPosition; capture final {x,y} on "hud-overlay-drag" "end"; apply clamped user position in applyHudOverlayBounds() when present; add screen listeners (display-removed, display-metrics-changed) to clear/validate position and reapply bounds; remove listeners on HUD "closed". |
Sequence Diagram(s)
sequenceDiagram
participant User as User
participant HUD as HUDOverlayWindow
participant Main as electron/windows.ts
participant Screen as Screen API
participant Display as DisplayWorkArea
User->>HUD: drag HUD (move)
HUD->>Main: emit "hud-overlay-drag" (end) with bounds
Main->>Main: set hudUserPosition = {x,y}
Main->>Main: call applyHudOverlayBounds()
Main->>Display: query current workArea
Display-->>Main: workArea
Main->>Main: clamp hudUserPosition to workArea, set window bounds
Screen->>Main: "display-metrics-changed" / "display-removed"
Main->>Main: validate hudUserPosition against displays
alt position invalid
Main->>Main: clear hudUserPosition and reapply default bounds
end
HUD->>Main: "closed"
Main->>Screen: remove listeners
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- fix(hud): resolve overlay snap to primary on multi-monitor #207: Also modifies HUD overlay positioning and display handling in
electron/windows.ts. - fix(windows): stabilize HUD drag by locking bounds size during move #167: Adds HUD drag-session state and related drag handlers in the same file; touches overlapping logic.
Poem
🐰
I nudged my HUD with gentle paws,
Saved the spot without a pause.
If screens move or pixels roam,
I check the bounds and bring it home. ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main change: preserving the user-dragged HUD position during recording state transitions, directly addressing the core problem solved by this PR. |
| Description check | ✅ Passed | The PR description covers all major template sections including purpose, motivation, type of change, testing guide, and checklist items are completed. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
electron/windows.ts (1)
505-526: Consider removing screen event listeners in the HUD window closed handler as a defensive best practice.The current code registers
screen.on("display-removed")andscreen.on("display-metrics-changed")listeners increateHudOverlayWindow()but never removes them. While the HUD window is only created once at app startup and never recreated, proper cleanup prevents future regressions if the window creation logic changes. Suggested approach: extract the handlers to named functions and remove them when the window closes.Suggested fix
const screen = getScreen(); - screen.on("display-removed", () => { + const handleDisplayRemoved = () => { hudUserPosition = null; - }); - screen.on("display-metrics-changed", () => { + }; + const handleDisplayMetricsChanged = () => { if (hudUserPosition) { const displays = screen.getAllDisplays(); const onScreen = displays.some( (d) => hudUserPosition!.x >= d.workArea.x && hudUserPosition!.x < d.workArea.x + d.workArea.width && hudUserPosition!.y >= d.workArea.y && hudUserPosition!.y < d.workArea.y + d.workArea.height, ); if (!onScreen) { hudUserPosition = null; applyHudOverlayBounds(hudOverlayExpanded); } } - }); + }; + screen.on("display-removed", handleDisplayRemoved); + screen.on("display-metrics-changed", handleDisplayMetricsChanged);win.on("closed", () => { + screen.removeListener("display-removed", handleDisplayRemoved); + screen.removeListener("display-metrics-changed", handleDisplayMetricsChanged); if (hudOverlayWindow === win) { hudOverlayWindow = null; } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/windows.ts` around lines 505 - 526, The screen event listeners registered via getScreen().on("display-removed", ...) and getScreen().on("display-metrics-changed", ...) leak because they are added inside createHudOverlayWindow() and never removed; extract the anonymous callbacks into named functions (e.g., handleDisplayRemoved and handleDisplayMetricsChanged) that reference hudUserPosition and call applyHudOverlayBounds(hudOverlayExpanded) as needed, register those named handlers with screen.on(...) when creating the HUD, and remove them with screen.removeListener("display-removed", handleDisplayRemoved) and screen.removeListener("display-metrics-changed", handleDisplayMetricsChanged) from the HUD window's 'closed' (or 'close') handler to ensure proper cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/windows.ts`:
- Around line 511-524: The handler for screen.on("display-metrics-changed") only
calls applyHudOverlayBounds(hudOverlayExpanded) when hudUserPosition is null
after an on-screen check, which misses cases where the saved point remains
inside but the window now overflows; always invoke
applyHudOverlayBounds(hudOverlayExpanded) at the end of the
display-metrics-changed callback (regardless of hudUserPosition/onScreen) so the
HUD is re-clamped to the current work area, keeping hudUserPosition updates and
the existing onScreen check intact.
---
Nitpick comments:
In `@electron/windows.ts`:
- Around line 505-526: The screen event listeners registered via
getScreen().on("display-removed", ...) and
getScreen().on("display-metrics-changed", ...) leak because they are added
inside createHudOverlayWindow() and never removed; extract the anonymous
callbacks into named functions (e.g., handleDisplayRemoved and
handleDisplayMetricsChanged) that reference hudUserPosition and call
applyHudOverlayBounds(hudOverlayExpanded) as needed, register those named
handlers with screen.on(...) when creating the HUD, and remove them with
screen.removeListener("display-removed", handleDisplayRemoved) and
screen.removeListener("display-metrics-changed", handleDisplayMetricsChanged)
from the HUD window's 'closed' (or 'close') handler to ensure proper cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The HUD overlay snaps back to the centered default position whenever recording starts because the idle-to-recording UI swap triggers a resize, and applyHudOverlayBounds() always recomputes a centered location. This is disruptive when the user has intentionally moved the bar out of the way before a timed recording. Remember the position after a drag ends and reuse it for subsequent bounds updates, clamped to the current work area. The position resets on app restart or when displays change so the bar cannot get stranded off-screen.
afostr
force-pushed
the
feature/preserve-hud-position
branch
from
c7be601 to
dc39031
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Description
This preserves the location of the recording hud bar when recording starts.
Without this the recording start state change snaps the bar back to the same spot, and this can be in the way of your recording.
Motivation
This solves a minor UX issue where the hud bar snaps back to a center location as the recording starts. I had tried a work around where I would use the timer delay start and clear the bar out of the way during the countdown, but as soon as the countdown finishes it snaps back on top of the app I need to record.
Of course, I can just drag it a way but that disrupts the flow/concentration of the task at hand.
Type of Change
Related Issue(s)
none found
Screenshots / Video
none
Video (wherever possible):
Testing Guide
I tested this by building and running the app.
I would then start a recording and observe that the location of the hud bar does not snap back to the center location as the video starts.
Checklist
(I skimmed issue list after and didn't see them)
(didn't touch release log)
Thank you for contributing!
Summary by CodeRabbit