Repository navigation
Clamp large window stranded off-screen after display disconnect (#2824) - #7084
Closed
ruan11223344 wants to merge 1 commit into
Closed
ruan11223344 wants to merge 1 commit into
ruan11223344 wants to merge 1 commit into
Conversation
…flow-ai#2824) CmuxMainWindow.constrainFrameRect refuses AppKit's constrain pass for any frame deemed "reachable", so an already-on-screen window is never nudged and cannot creep on sleep/wake. The reachability check required only 60pt of the window to overlap a screen in each dimension. That absolute floor is too lenient for large windows. When an external display is unplugged, a full-width window can be left hanging off the right edge of the remaining screen with several hundred points still visible — far above 60pt, so it was treated as reachable and left stranded, effectively off-screen and unusable. Require a proportional slice of the window to be visible (the larger of a 60pt floor and 60% of each dimension, capped at the window's extent). A window that is mostly or fully on-screen — including one whose titlebar merely pokes into the menu bar — is still preserved untouched, so the sleep/wake creep fix is unaffected; a window that is mostly off-screen is handed back to AppKit and pulled into view. Adds two regression cases to CmuxMainWindowConstrainFrameTests covering an 800pt window 42% past the right edge and a measured 1512pt full-width window left ~556pt visible after disconnect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@ruan11223344 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Frame Preservation Threshold
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
Contributor
Greptile SummaryThis PR fixes a window-placement regression (#2824) where disconnecting an external display left the cmux window stranded off the right edge of the remaining screen. The existing absolute-floor visibility check (60 pt) treated a large window with hundreds of points still technically on-screen as "reachable", so it was never handed back to AppKit's constrain pass.
Confidence Score: 5/5Safe to merge; the change is a self-contained threshold adjustment in a static helper with no side-effects on other code paths. The proportional threshold calculation is mathematically correct for all window sizes (verified by manually tracing each test case, including the tiny-window edge case where the outer No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["constrainFrameRect called by AppKit"] --> B["shouldPreserveFrameDuringConstrain"]
B --> C["Compute requiredWidth\nmin(w, max(60pt, w × 0.6))"]
C --> D["Compute requiredHeight\nmin(h, max(60pt, h × 0.6))"]
D --> E["For each screen visibleFrame"]
E --> F{"intersection ≥ required\nin both dimensions?"}
F -- "Yes (≥60% visible)" --> G["Return proposedFrame unchanged\n(sleep/wake creep prevented)"]
F -- "No (mostly off-screen)" --> H["Check next screen"]
H -- "No screens left" --> I["Return super.constrainFrameRect\n(AppKit pulls window back)"]
I --> J["Window clamped back into view\n✅ Fixes #2824"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["constrainFrameRect called by AppKit"] --> B["shouldPreserveFrameDuringConstrain"]
B --> C["Compute requiredWidth\nmin(w, max(60pt, w × 0.6))"]
C --> D["Compute requiredHeight\nmin(h, max(60pt, h × 0.6))"]
D --> E["For each screen visibleFrame"]
E --> F{"intersection ≥ required\nin both dimensions?"}
F -- "Yes (≥60% visible)" --> G["Return proposedFrame unchanged\n(sleep/wake creep prevented)"]
F -- "No (mostly off-screen)" --> H["Check next screen"]
H -- "No screens left" --> I["Return super.constrainFrameRect\n(AppKit pulls window back)"]
I --> J["Window clamped back into view\n✅ Fixes #2824"]
Reviews (1): Last reviewed commit: "Clamp large window stranded off-screen a..." | Re-trigger Greptile |
Collaborator
|
Thank you for this! The display-disconnect recovery shipped on main in #12053, including the regression coverage, so I’m closing this superseded PR :) |
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.
Fixes #2824.
Problem
When an external display is disconnected, the cmux window is left hanging off the right edge of the remaining screen — "effectively off screen" (per the issue), recoverable only by dragging it back or moving it to a new Space.
Reproduced on 0.64.17. Measuring the window via the Accessibility API right after a physical disconnect (built-in display, logical bounds
0,0 → 1512,982):So a 1512pt-wide window with only ~556pt visible on the right edge, its titlebar above the menu bar.
Root cause
CmuxMainWindow.constrainFrameRectdeliberately refuses AppKit's constrain pass for any frame judged "reachable", so an on-screen window is never repositioned and cannot creep on sleep/wake.shouldPreserveFrameDuringConstrainjudged reachability as ≥60pt of overlap in each dimension.That absolute floor is too lenient for large windows: a full-width window stranded off the right edge keeps hundreds of points visible — far above 60pt — so it was treated as reachable and left stranded. The existing tests only covered a ~20pt sliver, so this large-but-mostly-off case slipped through.
Fix
Require a proportional slice of the window to be visible: the larger of the 60pt floor and 60% of each dimension, capped at the window's own extent.
Tests
All existing
CmuxMainWindowConstrainFrameTestscases keep their results. Two regression cases added:testDoesNotPreserveLargeWindowStrandedOffRightEdge— an 800pt window 42% past the right edge.testDoesNotPreserveFullWidthWindowStrandedAfterDisconnect— the measured 1512pt full-width window left ~556pt visible.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Pulls windows back into view after a display disconnect by requiring a proportional visible area before skipping AppKit’s constrain pass. Fixes #2824.
Written for commit dffd54e. Summary will update on new commits.
Summary by CodeRabbit