fix(BAR-32): Keeps windows maximised across restarts and fixes the full-screen title bar on Windows

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

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<AppMenuBar>>,
     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 Fn(&ClickEvent, &mut Window, &mut App) + 'static>,
+) -> 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<Cell<bool>>);
+
+    impl Render for Bar {
+        fn render(&mut self, window: &mut Window, _: &mut Context<Self>) -> 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<AppMenuBar>);
+
+    impl Render for Menus {
+        fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> 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);
+    }
+}

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<Pixels>, work: gpui_kit::Point<Pixels>) -> WindowState {
+// Windows parks a minimised window off every display, and stops reporting it maximised.
+fn parked(bounds: Bounds<Pixels>, work: Option<Bounds<Pixels>>) -> 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<Bounds<Pixels>>) -> 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<Pixels>, work: Option<Bounds<Pixels>>) -> 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"));
     }
 }

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<WindowState>, window: &mut Window, cx: &mut Context<Self>) {
         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<Self>) -> 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(