diff --git a/app/src/lib.rs b/app/src/lib.rs index 04dada41c..026b1d2c0 100644 --- a/app/src/lib.rs +++ b/app/src/lib.rs @@ -1912,19 +1912,6 @@ fn app_callbacks(is_integration_test: bool) -> warpui::platform::AppCallbacks { crash_reporting::uninit_sentry(); })), on_should_close_window: Some(Box::new(move |window_id, ctx| { - let general_settings = GeneralSettings::as_ref(ctx); - // On Linux or Windows, if we're about to close the final window, we should quit the app instead. - // On Mac, we do this conditionally based on a user setting. - let quit_on_last_window_closed = cfg!(any(target_os = "linux", windows)) - || *general_settings.quit_on_last_window_closed; - if ctx.window_ids().count() == 1 && quit_on_last_window_closed { - log::info!("No windows left, terminating app"); - ctx.terminate_app(TerminationMode::Cancellable, None); - return ApproveTerminateResult::Cancel; - } - - let summary = UnsavedStateSummary::for_window(window_id, ctx); - send_telemetry_from_app_ctx!( TelemetryEvent::UserInitiatedClose { initiated_on: CloseTarget::Window, @@ -1932,28 +1919,63 @@ fn app_callbacks(is_integration_test: bool) -> warpui::platform::AppCallbacks { ctx ); - // Don't show dialog on integration test. Machine can't press buttons. - if !is_integration_test && summary.should_display_warning(ctx) { - let shown = summary - .dialog() - .on_confirm(move |ctx| { - ctx.windows() - .close_window(window_id, TerminationMode::ForceTerminate); - }) - .on_cancel(move |ctx| { - on_close_window_cancelled(window_id, false, ctx); - }) - .on_show_processes(move |ctx| { - on_close_window_cancelled(window_id, true, ctx); - }) - .show(ctx); - if shown { - ApproveTerminateResult::Cancel + let general_settings = GeneralSettings::as_ref(ctx); + + #[cfg(target_os = "macos")] + { + let window_ids = ctx.window_ids().collect_vec(); + let window_visibility = window_ids + .iter() + .map(|id| (*id, ctx.windows().is_window_visible(*id))) + .collect_vec(); + let is_closing_last_visible_window = + is_closing_last_visible_window(window_id, &window_visibility); + + if is_closing_last_visible_window && *general_settings.quit_on_last_window_closed { + log::info!("No windows left, terminating app"); + ctx.terminate_app(TerminationMode::Cancellable, None); + } else { + ctx.windows().hide_window(window_id); + } + ApproveTerminateResult::Cancel + } + + #[cfg(not(target_os = "macos"))] + { + // On Linux or Windows, if we're about to close the final window, we should quit the app instead. + let quit_on_last_window_closed = cfg!(any(target_os = "linux", windows)) + || *general_settings.quit_on_last_window_closed; + if ctx.window_ids().count() == 1 && quit_on_last_window_closed { + log::info!("No windows left, terminating app"); + ctx.terminate_app(TerminationMode::Cancellable, None); + return ApproveTerminateResult::Cancel; + } + + let summary = UnsavedStateSummary::for_window(window_id, ctx); + + // Don't show dialog on integration test. Machine can't press buttons. + if !is_integration_test && summary.should_display_warning(ctx) { + let shown = summary + .dialog() + .on_confirm(move |ctx| { + ctx.windows() + .close_window(window_id, TerminationMode::ForceTerminate); + }) + .on_cancel(move |ctx| { + on_close_window_cancelled(window_id, false, ctx); + }) + .on_show_processes(move |ctx| { + on_close_window_cancelled(window_id, true, ctx); + }) + .show(ctx); + if shown { + ApproveTerminateResult::Cancel + } else { + ApproveTerminateResult::Terminate + } } else { ApproveTerminateResult::Terminate } - } else { - ApproveTerminateResult::Terminate } })), on_should_terminate_app: Some(Box::new(move |ctx| { @@ -2034,7 +2056,11 @@ fn app_callbacks(is_integration_test: bool) -> warpui::platform::AppCallbacks { // e.g. clicking on the Dock icon. It is NOT called from the New Window // menu item. App::record_last_active_timestamp(); - ctx.dispatch_global_action("root_view:open_new", &()); + if let Some(window_id) = ctx.windows().frontmost_window_id() { + ctx.windows().show_window_and_focus_app(window_id); + } else { + ctx.dispatch_global_action("root_view:open_new", &()); + } ctx.dispatch_global_action("workspace:save_app", &()); })), on_open_urls: Some(Box::new(move |urls, ctx| { @@ -2174,6 +2200,86 @@ fn on_close_app_cancelled(open_navigation_palette: bool, ctx: &mut AppContext) { } } +#[cfg(target_os = "macos")] +fn is_closing_last_visible_window( + window_id: WindowId, + window_visibility: &[(WindowId, Option)], +) -> bool { + if window_visibility + .iter() + .any(|(_, visible)| visible.is_none()) + { + return window_visibility.len() == 1 + && window_visibility + .first() + .is_some_and(|(id, _)| *id == window_id); + } + + let mut visible_window_ids = window_visibility + .iter() + .filter_map(|(id, visible)| visible.unwrap_or(false).then_some(*id)); + + visible_window_ids + .next() + .is_some_and(|visible_window_id| visible_window_id == window_id) + && visible_window_ids.next().is_none() +} + +#[cfg(test)] +#[cfg(target_os = "macos")] +mod window_close_tests { + use super::*; + + #[test] + fn test_closing_last_visible_window_ignores_hidden_windows() { + let visible_window = WindowId::from_usize(1); + + assert!(is_closing_last_visible_window( + visible_window, + &[ + (visible_window, Some(true)), + (WindowId::from_usize(2), Some(false)), + (WindowId::from_usize(3), Some(false)), + ], + )); + } + + #[test] + fn test_closing_last_visible_window_requires_no_other_visible_windows() { + let closing_window = WindowId::from_usize(1); + let other_visible_window = WindowId::from_usize(2); + + assert!(!is_closing_last_visible_window( + closing_window, + &[ + (closing_window, Some(true)), + (other_visible_window, Some(true)), + (WindowId::from_usize(3), Some(false)), + ], + )); + } + + #[test] + fn test_closing_last_visible_window_falls_back_when_visibility_unknown() { + assert!(is_closing_last_visible_window( + WindowId::from_usize(1), + &[(WindowId::from_usize(1), None)], + )); + assert!(!is_closing_last_visible_window( + WindowId::from_usize(1), + &[ + (WindowId::from_usize(1), None), + (WindowId::from_usize(2), Some(false)), + ], + )); + assert!(!is_closing_last_visible_window( + WindowId::from_usize(1), + &[(WindowId::from_usize(2), None)], + )); + } +} + +#[cfg_attr(target_os = "macos", allow(dead_code))] fn on_close_window_cancelled( window_id: WindowId, open_navigation_palette: bool, diff --git a/app/src/quit_warning/mod.rs b/app/src/quit_warning/mod.rs index 30ffec698..fc498389d 100644 --- a/app/src/quit_warning/mod.rs +++ b/app/src/quit_warning/mod.rs @@ -27,6 +27,7 @@ enum QuitScope<'a> { pane_id: PaneId, }, Tabs(Vec>), + #[cfg_attr(target_os = "macos", allow(dead_code))] Window(WindowId), App, #[allow(dead_code)] @@ -207,6 +208,7 @@ impl UnsavedStateSummary<'static> { Self::for_scope(QuitScope::App, ctx) } + #[cfg_attr(target_os = "macos", allow(dead_code))] pub fn for_window(window_id: WindowId, ctx: &mut AppContext) -> Self { Self::for_scope(QuitScope::Window(window_id), ctx) } diff --git a/app/src/session_management.rs b/app/src/session_management.rs index dc451ea87..4ad08d28a 100644 --- a/app/src/session_management.rs +++ b/app/src/session_management.rs @@ -197,6 +197,7 @@ impl<'a> RunningSessionSummary<'a> { .collect() } + #[cfg_attr(target_os = "macos", allow(dead_code))] pub fn processes_in_window(&self, window_id: &WindowId) -> Vec<&SessionNavigationData> { self.long_running_cmds .iter() diff --git a/app/src/util/bindings.rs b/app/src/util/bindings.rs index acef9f9b9..2221252de 100644 --- a/app/src/util/bindings.rs +++ b/app/src/util/bindings.rs @@ -245,7 +245,7 @@ pub fn trigger_to_keystroke(trigger: &Trigger) -> Option { Trigger::Custom(custom) => custom_tag_to_keystroke(*custom), // Similarly, Standard Actions have their keyboard shortcuts set when creating the menu Trigger::Standard(standard) => match standard { - StandardAction::Close => mac_only_keystroke("cmd-shift-W"), + StandardAction::Close => mac_only_keystroke("cmd-w"), // "cmd-q" to quit and "cmd-h" to hide are the standard bindings for these actions on // Mac. StandardAction::Quit => mac_only_keystroke("cmd-q"), @@ -397,8 +397,14 @@ pub fn custom_tag_to_keystroke(custom: CustomTag) -> Option { Keystroke::parse("ctrl-shift-|").ok() } } - CustomAction::CloseWindow => mac_only_keystroke("cmd-shift-W"), - CustomAction::CloseCurrentSession => Keystroke::parse(cmd_or_ctrl_shift("w")).ok(), + CustomAction::CloseWindow => mac_only_keystroke("cmd-w"), + CustomAction::CloseCurrentSession => { + if OperatingSystem::get().is_mac() { + None + } else { + Keystroke::parse(cmd_or_ctrl_shift("w")).ok() + } + } CustomAction::ViewChangelog => Keystroke::parse(cmd_or_ctrl_shift("alt-o")).ok(), CustomAction::NewAgentModePane => Keystroke::parse("ctrl-space").ok(), CustomAction::AttachSelectionAsAgentModeContext => { diff --git a/app/src/util/bindings_tests.rs b/app/src/util/bindings_tests.rs index 029d8c386..2a1af434f 100644 --- a/app/src/util/bindings_tests.rs +++ b/app/src/util/bindings_tests.rs @@ -4,7 +4,13 @@ use warpui::{ App, }; -use crate::{util::bindings::keybinding_name_to_display_string, workspace::WorkspaceAction}; +use crate::{ + util::bindings::{ + custom_tag_to_keystroke, keybinding_name_to_display_string, trigger_to_keystroke, + CustomAction, + }, + workspace::WorkspaceAction, +}; #[test] fn test_keybinding_name_to_display_string() { @@ -72,3 +78,28 @@ fn test_keybinding_name_to_display_string() { }); }); } + +#[cfg(target_os = "macos")] +#[test] +fn test_cmd_w_defaults_to_close_window_on_macos() { + use warpui::actions::StandardAction; + + assert_eq!( + Some("cmd-w"), + custom_tag_to_keystroke(CustomAction::CloseWindow.into()) + .as_ref() + .map(|keystroke| keystroke.normalized()) + .as_deref() + ); + assert_eq!( + None, + custom_tag_to_keystroke(CustomAction::CloseCurrentSession.into()) + ); + assert_eq!( + Some("cmd-w"), + trigger_to_keystroke(&Trigger::Standard(StandardAction::Close)) + .as_ref() + .map(|keystroke| keystroke.normalized()) + .as_deref() + ); +} diff --git a/app/src/workspace/mod.rs b/app/src/workspace/mod.rs index a4432dfb4..abe630dd5 100644 --- a/app/src/workspace/mod.rs +++ b/app/src/workspace/mod.rs @@ -927,7 +927,7 @@ pub fn init(app: &mut AppContext) { .with_custom_description(bindings::MAC_MENUS_CONTEXT, "Close Window"), WorkspaceAction::CloseWindow, ) - .with_mac_key_binding("cmd-shift-W") + .with_mac_key_binding("cmd-w") .with_context_predicate(id!("Workspace")) .with_group(bindings::BindingGroup::Close.as_str()) .with_custom_action(CustomAction::CloseWindow) diff --git a/crates/warpui/src/platform/headless/windowing.rs b/crates/warpui/src/platform/headless/windowing.rs index bfc532013..a496ca72f 100644 --- a/crates/warpui/src/platform/headless/windowing.rs +++ b/crates/warpui/src/platform/headless/windowing.rs @@ -91,20 +91,35 @@ impl warpui_core::platform::WindowManager for WindowManager { window_id: WindowId, _behavior: platform::WindowFocusBehavior, ) { + if let Some(window) = self.windows.get(&window_id) { + window.set_visible(true); + } self.set_active_window(Some(window_id)); } fn hide_app(&self) { - // No-op. + for window in self.windows.values() { + window.set_visible(false); + } + self.set_active_window(None); } fn hide_window(&self, window_id: WindowId) { + if let Some(window) = self.windows.get(&window_id) { + window.set_visible(false); + } // If hiding the active window, clear focus. if *self.active_window.borrow() == Some(window_id) { self.set_active_window(None); } } + fn is_window_visible(&self, window_id: WindowId) -> Option { + self.windows + .get(&window_id) + .map(|window| window.is_visible()) + } + fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { if let Some(window) = self.windows.get(&window_id) { window.set_bounds(bound); @@ -176,6 +191,7 @@ pub struct Window { callbacks: WindowCallbacks, bounds: RefCell, fullscreen_state: RefCell, + visible: RefCell, } impl Window { @@ -189,12 +205,21 @@ impl Window { callbacks, bounds: RefCell::new(bounds), fullscreen_state: RefCell::new(options.fullscreen_state), + visible: RefCell::new(true), } } fn set_bounds(&self, rect: RectF) { *self.bounds.borrow_mut() = rect; } + + fn set_visible(&self, visible: bool) { + *self.visible.borrow_mut() = visible; + } + + fn is_visible(&self) -> bool { + *self.visible.borrow() + } } impl platform::Window for Window { diff --git a/crates/warpui/src/platform/mac/objc/window.m b/crates/warpui/src/platform/mac/objc/window.m index 05db724e7..33ecac56c 100644 --- a/crates/warpui/src/platform/mac/objc/window.m +++ b/crates/warpui/src/platform/mac/objc/window.m @@ -1001,6 +1001,10 @@ void hide_window(WarpWindow *window) { [window orderOut:nil]; } +BOOL is_window_visible(WarpWindow *window) { + return [window isVisible]; +} + void set_window_title(id window, NSString *title) { if ([window isKindOfClass:[WarpPanel class]] && [window isVisible]) { // For the hotkey window (which is an NSPanel), we need to explicitly diff --git a/crates/warpui/src/platform/mac/window.rs b/crates/warpui/src/platform/mac/window.rs index b8f474e2b..daf8410f1 100644 --- a/crates/warpui/src/platform/mac/window.rs +++ b/crates/warpui/src/platform/mac/window.rs @@ -146,6 +146,10 @@ impl platform::WindowManager for WindowManager { Window::hide_window(window_id) } + fn is_window_visible(&self, window_id: WindowId) -> Option { + Window::is_window_visible(window_id) + } + fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { Window::set_window_bounds(window_id, bound) } @@ -323,6 +327,10 @@ impl platform::WindowManager for IntegrationTestWindowManager { self.window_manager.hide_window(window_id) } + fn is_window_visible(&self, window_id: WindowId) -> Option { + self.window_manager.is_window_visible(window_id) + } + fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { self.window_manager.set_window_bounds(window_id, bound) } @@ -448,6 +456,7 @@ extern "C" { fn activate_app(); fn show_window_and_focus_app(window: id, bringToFront: BOOL); fn hide_window(window: id); + fn is_window_visible(window: id) -> BOOL; fn position_and_order_front(window: id); fn position_at_given_location(window: id, origin: NSPoint); fn order_front_without_focus(window: id, origin: NSPoint); @@ -845,6 +854,12 @@ impl Window { } } + pub fn is_window_visible(window_id: WindowId) -> Option { + unsafe { + Self::find_window_with_id(window_id).map(|window| is_window_visible(window) == YES) + } + } + /// Returns a reference to a `WarpWindow` identified by `window_id`, if any. /// /// # Safety diff --git a/crates/warpui/src/windowing/winit/window.rs b/crates/warpui/src/windowing/winit/window.rs index 415a28aab..b3a2ef4af 100644 --- a/crates/warpui/src/windowing/winit/window.rs +++ b/crates/warpui/src/windowing/winit/window.rs @@ -238,6 +238,12 @@ impl platform::WindowManager for WindowManager { } } + fn is_window_visible(&self, window_id: WindowId) -> Option { + self.windows + .get(&window_id) + .map(|window| window.is_visible()) + } + fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { if let Some(window) = self.windows.get(&window_id) { window.set_bounds(bound); @@ -523,6 +529,10 @@ impl platform::WindowManager for IntegrationTestWindowManager { .retain(|id| *id != window_id); } + fn is_window_visible(&self, window_id: WindowId) -> Option { + self.window_manager.is_window_visible(window_id) + } + fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { self.window_manager.set_window_bounds(window_id, bound) } diff --git a/crates/warpui_core/src/platform/mod.rs b/crates/warpui_core/src/platform/mod.rs index 20d33b2b2..1499996a0 100644 --- a/crates/warpui_core/src/platform/mod.rs +++ b/crates/warpui_core/src/platform/mod.rs @@ -583,6 +583,9 @@ pub trait WindowManager { fn show_window_and_focus_app(&self, window_id: WindowId, behavior: WindowFocusBehavior); fn hide_app(&self); fn hide_window(&self, window_id: WindowId); + fn is_window_visible(&self, _window_id: WindowId) -> Option { + None + } fn set_window_bounds(&self, window_id: WindowId, bound: RectF); /// Sets the background blur radius for all windows to the given `blur_radius_pixels` value. diff --git a/crates/warpui_core/src/windowing/state.rs b/crates/warpui_core/src/windowing/state.rs index 6dbc4eaca..28cdd7e12 100644 --- a/crates/warpui_core/src/windowing/state.rs +++ b/crates/warpui_core/src/windowing/state.rs @@ -90,6 +90,10 @@ impl WindowManager { self.platform.hide_window(window_id) } + pub fn is_window_visible(&self, window_id: WindowId) -> Option { + self.platform.is_window_visible(window_id) + } + pub fn set_window_bounds(&self, window_id: WindowId, bound: RectF) { self.platform.set_window_bounds(window_id, bound) }