diff --git a/Cargo.lock b/Cargo.lock index 647bb6a..0978b83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -641,7 +641,7 @@ dependencies = [ [[package]] name = "barsql" -version = "1.2.4" +version = "1.2.5" dependencies = [ "anyhow", "barsql-app", @@ -656,7 +656,7 @@ dependencies = [ [[package]] name = "barsql-app" -version = "1.2.4" +version = "1.2.5" dependencies = [ "async-channel", "barsql-core", @@ -684,7 +684,7 @@ dependencies = [ [[package]] name = "barsql-core" -version = "1.2.4" +version = "1.2.5" dependencies = [ "dirs 7.0.0", "hex", @@ -697,7 +697,7 @@ dependencies = [ [[package]] name = "barsql-db" -version = "1.2.4" +version = "1.2.5" dependencies = [ "anyhow", "async-channel", @@ -734,7 +734,7 @@ dependencies = [ [[package]] name = "barsql-io" -version = "1.2.4" +version = "1.2.5" dependencies = [ "barsql-core", "barsql-db", @@ -749,7 +749,7 @@ dependencies = [ [[package]] name = "barsql-sql" -version = "1.2.4" +version = "1.2.5" dependencies = [ "barsql-core", "libc", @@ -761,7 +761,7 @@ dependencies = [ [[package]] name = "barsql-storage" -version = "1.2.4" +version = "1.2.5" dependencies = [ "barsql-core", "serde", @@ -772,7 +772,7 @@ dependencies = [ [[package]] name = "barsql-ui" -version = "1.2.4" +version = "1.2.5" dependencies = [ "anyhow", "async-channel", diff --git a/Cargo.toml b/Cargo.toml index 49f5280..989b8fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "3" members = ["crates/*", "xtask"] [workspace.package] -version = "1.2.4" +version = "1.2.5" edition = "2024" license = "GPL-3.0-only" publish = false diff --git a/crates/barsql-ui/src/title_bar.rs b/crates/barsql-ui/src/title_bar.rs index d073204..262fa28 100644 --- a/crates/barsql-ui/src/title_bar.rs +++ b/crates/barsql-ui/src/title_bar.rs @@ -1,3 +1,5 @@ +use std::rc::Rc; + use gpui_kit::component::menu::AppMenuBar; use gpui_kit::component::{ActiveTheme, Icon, IconName, Sizable, TITLE_BAR_HEIGHT, TitleBar, h_flex}; use gpui_kit::prelude::FluentBuilder as _; @@ -8,21 +10,25 @@ use crate::tokens::TEXT_SM; // macOS has traffic lights and the system menu bar, so its bar only shows the title. // README screenshots draw the Windows bar on macOS too, where GPUI Kit leaves out the window controls. -// On Linux, GPUI Kit's close button calls `on_close` instead of removing the window. Elsewhere closing goes through -// on_window_should_close. -// With the menu bar, 9px instead of GPUI Kit's 12px centres the mark over the connection switcher's icon below it. +// On Linux, and in fullscreen, the close button calls `on_close` instead of removing the window. +// Elsewhere closing goes through on_window_should_close. +// With the menu bar, 9px instead of GPUI Kit's 12px centres the mark over the connection switcher's icon below it. In +// fullscreen GPUI Kit pads the bar's content by another 0.75rem, which the menu bar's row takes back. pub fn render( menu_bar: Option<&Entity>, title: SharedString, on_close: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static, + window: &Window, cx: &App, ) -> impl IntoElement { let theme = cx.theme(); let mac_screenshot = cfg!(target_os = "macos") && menu_bar.is_some(); let content = match menu_bar { Some(menu_bar) => h_flex() + .debug_selector(|| "title-bar-menus".into()) .size_full() .gap_1() + .when(window.is_fullscreen(), |bar| bar.ml(rems(-0.75))) .child(vector_img(if theme.is_dark() { MARK_DARK } else { MARK_LIGHT }, px(16.)).flex_none()) .child(menu_bar.clone()) .child(div().flex_1()) @@ -33,7 +39,52 @@ pub fn render( .pr(px(80.)) .child(div().text_size(TEXT_SM).text_color(theme.muted_foreground).child(title)), }; - TitleBar::new().on_close_window(on_close).when(menu_bar.is_some(), |bar| bar.pl(px(9.))).child(content) + let on_close = Rc::new(on_close); + let bar = TitleBar::new() + .on_close_window({ + let on_close = on_close.clone(); + move |event, window, cx| on_close(event, window, cx) + }) + .when(menu_bar.is_some(), |bar| bar.pl(px(9.))) + .child(content); + let fullscreen = window.is_fullscreen() && draws_window_controls(window); + div().flex_shrink_0().child(bar).when(fullscreen, |bar| bar.child(fullscreen_controls(window, on_close))) +} + +fn draws_window_controls(window: &Window) -> bool { + cfg!(target_os = "windows") + || (cfg!(target_os = "linux") && matches!(window.window_decorations(), Decorations::Client { .. })) +} + +// In fullscreen GPUI stops hit testing the window controls on Windows, which leaves GPUI Kit's buttons without a click +// handler, and on Linux its maximise can't leave fullscreen. These sit over its buttons and take their clicks. +fn fullscreen_controls( + window: &Window, + on_close: Rc, +) -> impl IntoElement { + let supported = window.window_controls(); + let control = |id: &'static str| div().id(id).debug_selector(move || id.into()).w(TITLE_BAR_HEIGHT).h_full(); + h_flex() + .absolute() + .top_0() + .right_0() + .h(TITLE_BAR_HEIGHT) + .when(supported.minimize, |controls| { + controls.child(control("fullscreen-minimize").on_click(|_, window, cx| { + cx.stop_propagation(); + window.minimize_window(); + })) + }) + .when(supported.maximize, |controls| { + controls.child(control("fullscreen-exit").on_click(|_, window, cx| { + cx.stop_propagation(); + window.toggle_fullscreen(); + })) + }) + .child(control("fullscreen-close").on_click(move |event, window, cx| { + cx.stop_propagation(); + on_close(event, window, cx); + })) } fn window_controls(cx: &App) -> impl IntoElement { @@ -52,3 +103,67 @@ fn window_controls(cx: &App) -> impl IntoElement { .flex_none() .children([IconName::WindowMinimize, IconName::WindowMaximize, IconName::WindowClose].map(control)) } + +#[cfg(test)] +mod tests { + use std::cell::Cell; + use std::rc::Rc; + + use gpui_kit::component::menu::AppMenuBar; + use gpui_kit::{ + App, ClickEvent, Context, Entity, IntoElement, Modifiers, ParentElement as _, Render, Styled as _, + TestAppContext, VisualTestContext, Window, div, + }; + + use crate::test_support::Env; + + struct Bar(Rc>); + + impl Render for Bar { + fn render(&mut self, window: &mut Window, _: &mut Context) -> impl IntoElement { + let closed = self.0.clone(); + let on_close = move |_: &ClickEvent, _: &mut Window, _: &mut App| closed.set(true); + div().size_full().child(super::fullscreen_controls(window, Rc::new(on_close))) + } + } + + fn click(selector: &'static str, cx: &mut VisualTestContext) { + cx.update(|window, cx| window.draw(cx).clear(cx)); + let at = cx.debug_bounds(selector).unwrap_or_else(|| panic!("{selector} is not drawn")).center(); + cx.simulate_click(at, Modifiers::none()); + cx.run_until_parked(); + } + + #[gpui_kit::test] + fn in_fullscreen_maximise_leaves_it_and_close_asks_the_workspace(cx: &mut TestAppContext) { + let _env = Env::new(cx); + let closed = Rc::new(Cell::new(false)); + let (_, cx) = cx.add_window_view(|_, _| Bar(closed.clone())); + cx.update(|window, _| window.toggle_fullscreen()); + click("fullscreen-exit", cx); + assert!(!cx.update(|window, _| window.is_fullscreen())); + click("fullscreen-close", cx); + assert!(closed.get()); + } + + struct Menus(Entity); + + impl Render for Menus { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + super::render(Some(&self.0), "BarSQL".into(), |_, _, _| {}, window, cx) + } + } + + #[gpui_kit::test] + fn the_menu_bar_keeps_its_place_in_fullscreen(cx: &mut TestAppContext) { + let _env = Env::new(cx); + let (_, cx) = cx.add_window_view(|_, cx| Menus(AppMenuBar::new(cx))); + let left = |cx: &mut VisualTestContext| { + cx.update(|window, cx| window.draw(cx).clear(cx)); + cx.debug_bounds("title-bar-menus").expect("the menu bar is drawn").left() + }; + let windowed = left(cx); + cx.update(|window, _| window.toggle_fullscreen()); + assert_eq!(left(cx), windowed); + } +} diff --git a/crates/barsql-ui/src/window_state.rs b/crates/barsql-ui/src/window_state.rs index ca98282..3e1eff0 100644 --- a/crates/barsql-ui/src/window_state.rs +++ b/crates/barsql-ui/src/window_state.rs @@ -66,6 +66,13 @@ pub fn initial_bounds(saved: Option<&WindowState>, cx: &App) -> WindowBounds { // Maximised and fullscreen windows keep their last normal bounds. pub fn capture(last: &WindowState, window: &Window, cx: &App) -> WindowState { + let work = window.display(cx).map(|display| display.visible_bounds()); + if parked(window.bounds(), work) { + return last.clone(); + } + if cfg!(target_os = "windows") { + return placement(last, window.window_bounds(), work); + } let mode = if window.is_fullscreen() { FULLSCREEN } else if window.is_maximized() { @@ -76,23 +83,38 @@ pub fn capture(last: &WindowState, window: &Window, cx: &App) -> WindowState { if mode != NORMAL { return WindowState { mode: mode.into(), ..last.clone() }; } - let bounds = window.bounds(); - let work = window.display(cx).map(|display| display.visible_bounds().origin).unwrap_or_default(); - normal(last, bounds, work) + normal(last, window.bounds(), work) } -fn normal(last: &WindowState, bounds: Bounds, work: gpui_kit::Point) -> WindowState { +// Windows parks a minimised window off every display, and stops reporting it maximised. +fn parked(bounds: Bounds, work: Option>) -> bool { + work.is_some_and(|work| !work.intersects(&bounds)) +} + +// Windows' own record of the window, so the mode doesn't hang on the order of its move and size messages, and a +// maximised or fullscreen window still gives its normal bounds. +fn placement(last: &WindowState, bounds: WindowBounds, work: Option>) -> WindowState { + let (mode, bounds) = match bounds { + WindowBounds::Fullscreen(bounds) => (FULLSCREEN, bounds), + WindowBounds::Maximized(bounds) => (MAXIMISED, bounds), + WindowBounds::Windowed(bounds) => (NORMAL, bounds), + }; + WindowState { mode: mode.into(), ..normal(last, bounds, work) } +} + +fn normal(last: &WindowState, bounds: Bounds, work: Option>) -> WindowState { let round = |value: Pixels| value.as_f32().round() as i64; let (width, height) = (round(bounds.size.width), round(bounds.size.height)); if width <= 0 || height <= 0 { return last.clone(); } + let origin = work.map(|work| work.origin).unwrap_or_default(); WindowState { mode: NORMAL.into(), width, height, - x: round(bounds.origin.x - work.x), - y: round(bounds.origin.y - work.y), + x: round(bounds.origin.x - origin.x), + y: round(bounds.origin.y - origin.y), } } @@ -106,9 +128,9 @@ pub fn stored_width(cx: &App, key: &str, default: f32, min: f32, max: f32) -> Pi #[cfg(test)] mod tests { - use gpui_kit::{Bounds, point, px, size}; + use gpui_kit::{Bounds, WindowBounds, point, px, size}; - use super::{WindowState, normal, parse}; + use super::{WindowState, normal, parked, parse, placement}; #[test] fn reads_a_stored_record() { @@ -127,10 +149,25 @@ mod tests { #[test] fn normal_bounds_are_kept_relative_to_the_work_area() { let last = WindowState { mode: "maximised".into(), width: 1000, height: 700, x: 5, y: 6 }; + let work = Some(Bounds::new(point(px(0.), px(25.)), size(px(1920.), px(1055.)))); let bounds = Bounds::new(point(px(100.4), px(60.)), size(px(1200.), px(800.))); - let captured = normal(&last, bounds, point(px(0.), px(25.))); + let captured = normal(&last, bounds, work); assert_eq!(captured, WindowState { mode: "normal".into(), width: 1200, height: 800, x: 100, y: 35 }); let empty = Bounds::new(point(px(0.), px(0.)), size(px(0.), px(0.))); - assert_eq!(normal(&last, empty, point(px(0.), px(0.))), last, "a zero size keeps the last record"); + assert_eq!(normal(&last, empty, work), last, "a zero size keeps the last record"); + let minimised = Bounds::new(point(px(-32000.), px(-32000.)), size(px(1920.), px(1055.))); + assert!(parked(minimised, work)); + assert!(!parked(bounds, work)); + } + + #[test] + fn windows_placement_gives_the_mode_and_the_normal_bounds() { + let last = WindowState { mode: "normal".into(), width: 1000, height: 700, x: 5, y: 6 }; + let work = Some(Bounds::new(point(px(0.), px(0.)), size(px(1920.), px(1040.)))); + let restore = Bounds::new(point(px(200.), px(100.)), size(px(1200.), px(800.))); + let record = |mode: &str| WindowState { mode: mode.into(), width: 1200, height: 800, x: 200, y: 100 }; + assert_eq!(placement(&last, WindowBounds::Maximized(restore), work), record("maximised")); + assert_eq!(placement(&last, WindowBounds::Fullscreen(restore), work), record("fullscreen")); + assert_eq!(placement(&last, WindowBounds::Windowed(restore), work), record("normal")); } } diff --git a/crates/barsql-ui/src/workspace.rs b/crates/barsql-ui/src/workspace.rs index 6abaf25..b7e28ac 100644 --- a/crates/barsql-ui/src/workspace.rs +++ b/crates/barsql-ui/src/workspace.rs @@ -428,6 +428,13 @@ pub fn open(options: LaunchOptions, cx: &mut App) { } cx.new(|cx| Root::new(view, window, cx)) }); + // On Windows GPUI maximises the window inside open_window, before it listens for resizes, so its layout would + // stay at the restore size. + if cfg!(target_os = "windows") + && let Ok(window) = window + { + let _ = window.update(cx, |_, window, cx| window.bounds_changed(cx)); + } if track && let Ok(window) = window { let _ = window.update(cx, |root, window, cx| { if let Some((file_path, name)) = state::bar(cx).take_pending_file() @@ -703,7 +710,9 @@ impl Workspace { fn track_window(&mut self, saved: Option, window: &mut Window, cx: &mut Context) { self.window_state = Some(saved.unwrap_or_default()); let subscription = cx.observe_window_bounds(window, |this, window, cx| { - let last = this.window_state.clone().unwrap_or_default(); + let Some(last) = this.window_state.clone() else { + return; + }; let next = window_state::capture(&last, window, cx); if next == last { return; @@ -1542,11 +1551,17 @@ impl Workspace { // AccessKit adapter, which resets the class of the content view while the Touch Bar observes it through KVO, and // AppKit aborts on the Touch Bar's next update. fn close_window(&mut self, window: &mut Window, cx: &mut Context) -> bool { + // On Windows and Linux the closed window takes this view with it before on_app_quit runs. + self.save_session(cx); + self.save_window_state(cx); let drafts = self.tabs.iter().any(|tab| tab.view.holds_draft(cx)); if cfg!(target_os = "macos") || drafts || state::bar(cx).any_transaction_open() { self.quit(window, cx); return false; } + // GPUI reports the deactivation as a bounds change, and on Windows that comes after the window is destroyed, + // which then reads as not maximised. + self.window_state = None; true } @@ -2403,6 +2418,7 @@ impl Render for Workspace { window.remove_window(); } }), + window, cx, )) .child( diff --git a/crates/barsql-ui/src/workspace/tests.rs b/crates/barsql-ui/src/workspace/tests.rs index 64ad874..d5d3f4c 100644 --- a/crates/barsql-ui/src/workspace/tests.rs +++ b/crates/barsql-ui/src/workspace/tests.rs @@ -367,6 +367,41 @@ fn window_moves_and_resizes_are_saved_after_a_pause(cx: &mut TestAppContext) { assert_eq!((saved.mode.as_str(), saved.width, saved.height), ("normal", 1000, 700)); } +#[gpui_kit::test] +fn closing_the_window_saves_what_waits_on_a_pause(cx: &mut TestAppContext) { + let env = Env::new(cx); + let session = EditorSession { tabs: vec![tab("a", &env.connection.id, "SELECT 1")], active_tab: "a".into() }; + env.bar.save_editor_session(session).unwrap(); + let (workspace, cx) = open(cx); + workspace.update_in(cx, |workspace, window, cx| workspace.track_window(None, window, cx)); + cx.simulate_keystrokes(end_of_text()); + cx.simulate_input(" + 1"); + cx.simulate_resize(size(px(1000.), px(700.))); + cx.run_until_parked(); + assert_eq!(env.bar.editor_session().tabs[0].sql, "SELECT 1", "not before the pause"); + + cx.simulate_close(); + assert_eq!(env.bar.editor_session().tabs[0].sql, "SELECT 1 + 1"); + let saved = env.bar.settings().get(window_state::KEY).and_then(|raw| window_state::parse(raw)).unwrap(); + assert_eq!((saved.width, saved.height), (1000, 700)); +} + +// The deactivation that follows a close reaches the window state as a bounds change. On macOS closing quits instead. +#[cfg(not(target_os = "macos"))] +#[gpui_kit::test] +fn a_closed_window_keeps_the_state_it_closed_in(cx: &mut TestAppContext) { + let env = Env::new(cx); + let (workspace, cx) = open(cx); + cx.update(|window, _| window.activate_window()); + cx.run_until_parked(); + let maximised = window_state::WindowState { mode: "maximised".into(), width: 1200, height: 800, x: 10, y: 20 }; + workspace.update_in(cx, |workspace, window, cx| workspace.track_window(Some(maximised.clone()), window, cx)); + assert!(cx.simulate_close()); + cx.deactivate_window(); + let saved = env.bar.settings().get(window_state::KEY).and_then(|raw| window_state::parse(raw)); + assert_eq!(saved, Some(maximised)); +} + fn dialog_open(cx: &mut VisualTestContext) -> bool { cx.update(|window, cx| window.has_active_dialog(cx)) }