Repository navigation
Fix window movability regression - #4983
lederniermagicien wants to merge 4 commits into
Conversation
|
@lederniermagicien is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
📝 WalkthroughWalkthroughThis PR makes main-window drag configuration presentation-mode and suppression-aware, adds a reason-matching finish API for window-move suppression, integrates the matching finish into folder-drag lifecycle, and adds tests and localized InfoPlist strings. ChangesWindow Move Suppression with Reason Matching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR restores OS-level window movability for the cmux main window in standard mode (setting
Confidence Score: 5/5Safe to merge; the movability baseline, suppression lifecycle, mode-change handling, and actor annotations are all correct and well-tested. The core logic — setting No files require special attention. Important Files Changed
Reviews (10): Last reviewed commit: "Align window suppression actor isolation" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if activeWindowMoveSuppressionSequenceReason(window: window) == nil { | ||
| window.isMovable = !WorkspacePresentationModeSettings.isMinimal(defaults: defaults) | ||
| } else { | ||
| ensureWindowMoveSuppressionSequenceIsImmovable(window: window) | ||
| } |
There was a problem hiding this comment.
previousMovableState after mode change mid-suppression
beginWindowMoveSuppressionSequence captures previousMovableState = window.isMovable at suppression start — now true in standard mode. When the mode transitions from standard → minimal while a suppression is active, this branch calls ensureWindowMoveSuppressionSequenceIsImmovable (correct during the drag) but never updates the stored previousMovableState. When the suppression ends, finishWindowMoveSuppressionSequence → restoreWindowDragging(previousMovableState: true) leaves the window movable, even though minimal mode expects isMovable = false. Before this PR this was harmless because previousMovableState was always false; the new standard-mode true makes it observable whenever a mode change fires mid-drag (e.g., via the command palette keyboard shortcut).
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
|
@codex review |
@lederniermagicien I have started the AI code review. It will take a few minutes to complete. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Actions performedReview triggered.
|
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Again pls @codex review |
@lederniermagicien I have started the AI code review. It will take a few minutes to complete. |
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Actions performedReview triggered.
|
|
To use Codex here, create a Codex account and connect to github. |
|
Last one I hope @codex review |
|
To use Codex here, create a Codex account and connect to github. |
@lederniermagicien I have started the AI code review. It will take a few minutes to complete. |
|
(ᵔᴥᵔ)🐇 ✅ Actions performedReview triggered.
|
|
To use Codex here, create a Codex account and connect to github. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
teamleaderleo
added
the
area: layout
teamleaderleo
added
the
S2: major
This branch was successfully deployed
2 participants
Summary
isMovableso macOS tiling and third-party window managers can move cmux windows again.isMovableByWindowBackgrounddisabled so app content, titlebar controls, folder proxy icon drags, and Bonsplit tab gestures do not become implicit background window drags.performDragwhen they intentionally move the window.vendor/bonsplit; this PR only addresses the cmux-side regression.Testing
CMUX_NUCLEO_FFI_REQUIRE_CARGO=0 CMUX_SKIP_ZIG_BUILD=1 ./scripts/test-unit.sh test -only-testing:cmuxTests/TitlebarLeadingInsetPassthroughViewTests -only-testing:cmuxTests/FolderWindowMoveSuppressionTests -only-testing:cmuxTests/WindowMoveSuppressionHitPathTestsCMUX_SKIP_ZIG_BUILD=1 CMUX_NUCLEO_FFI_REQUIRE_CARGO=0 ./scripts/reload.sh --tag window-movable-regressionDemo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
Checklist
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Restores OS-level movability for the main window in standard mode while keeping background drags off; minimal mode stays immovable and uses explicit chrome drags. Preserves suppression state across mode changes and aligns suppression helpers to the main thread for safe UI updates.
Bug Fixes
isMovableby presentation mode: enabled in standard, disabled in minimal; keepisMovableByWindowBackground = false.finishWindowMoveSuppressionSequence(...).@MainActor; used lightweight locks where needed to keep AppKit event paths synchronous and avoid cross-actor issues.New Features
InfoPlist.xcstrings(zh-Hans/zh-Hant/ko/de/es/fr/it/da/pl/ru/bs/ar/nb/pt-BR/th/tr).Written for commit 2ebd196. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests
Localization
Refactor