Hide macOS windows instead of closing sessions#9371
Hide macOS windows instead of closing sessions#9371creeveliu wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. I requested changes on this pull request and posted feedback. Comment You can view the conversation on Warp. I approved this pull request. No matching stakeholder was found for the changed files, so no human reviewers were requested. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes macOS window close actions and the native close callback to hide windows instead of terminating Warp sessions, and updates the Dock reopen path to restore an existing hidden window.
Concerns
- The macOS native close callback now bypasses the existing
quit_on_last_window_closedpreference, so users who opted into quitting when closing the final window will instead keep Warp running with the window hidden. - Security pass: no security-specific issues identified in the inlined diff.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| ApproveTerminateResult::Cancel | ||
| #[cfg(target_os = "macos")] | ||
| { | ||
| ctx.windows().hide_window(window_id); |
There was a problem hiding this comment.
quit_on_last_window_closed on macOS, so users who enabled the existing setting to quit when closing the final window will now get a hidden running app instead; preserve the last-window quit path before hiding.
0bf005a to
63119af
Compare
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR changes macOS close-window paths so Cmd-W, close-window actions, and the native close callback hide windows instead of closing sessions, and attempts to restore a hidden window when the Dock requests a reopen.
Concerns
- The macOS
quit_on_last_window_closedbehavior is broken for multiple windows because hidden windows remain inctx.window_ids(), so the app can end up with no visible windows and still not quit. - Supplemental security pass: no security issues found in the changed lines.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| #[cfg(target_os = "macos")] | ||
| { | ||
| if ctx.window_ids().count() == 1 && *general_settings.quit_on_last_window_closed { |
There was a problem hiding this comment.
ctx.window_ids().count() includes windows hidden by this new path, so with quit_on_last_window_closed enabled a two-window workflow can hide the first window, then hide the second without ever terminating; base this check on visible windows or track hidden state before deciding to hide.
Co-Authored-By: Warp <agent@warp.dev>
63119af to
c4bd213
Compare
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR changes macOS window close behavior so Cmd-W, the Close Window action, and the native red close button hide windows instead of closing their sessions, while adding visibility tracking and Dock reopen behavior to restore an existing window.
Concerns
- No blocking correctness or security concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Description
Make macOS window close actions hide the window instead of closing the underlying Warp session. This updates Cmd-W / Close Window bindings, the explicit close window actions, and the native macOS window close callback so clicking the red close button also hides the window. Reopening Warp from the Dock restores an existing hidden window before creating a new one.
Testing
Co-Authored-By: Warp agent@warp.dev