feat(settings): open settings in a window of its own

Settings used to cover the workspace it was opened from, hiding the
terminal a setting was being tried on. It now opens a separate window.

- The state and page stay on the workspace's Tty7App; a thin
  SettingsWindow view borrows it to draw, observes it to repaint, and
  sets its own rem size. The state is built against the new window, so
  its inputs and subscriptions belong to it.
- Opening again raises the window instead of closing it. Esc, Cmd-W and
  the close button go through close_settings_checked, so unsaved forms
  and theme drafts still prompt; closing hands focus back to the
  workspace window. The window goes when its owner is released.
- open_settings_then / close_settings_then run follow-ups in the right
  window: loading an SSH form happens in the settings window, and
  Connect opens its tab in the workspace window.
- Window opacity, blur and backdrop are pushed to every window, since
  they are now changed from a window that is not the workspace.
- Tests keep the in-window overlay so they can drive settings through
  their single test window.

src/ui/app.rs

@@ -217,6 +217,9 @@ pub(crate) fn document_column_px(body: f32, ratio: f32) -> Option<f32> {
 
 pub(crate) const TITLE_BAR_HEIGHT: f32 = 40.;
 
+/// Work to run against the settings window once it has been built.
+type SettingsFollowUp = Box<dyn FnOnce(&mut Tty7App, &mut Window, &mut Context<Tty7App>)>;
+
 pub(crate) const TILE_SIZE: f32 = 32.;
 pub(crate) const TILE_GLYPH: f32 = 16.;
 /// A tile that sits in a body row rather than in chrome: the box shrinks to
@@ -922,6 +925,11 @@ pub struct Tty7App {
     _sidebar_search_sub: Subscription,
     _file_search_sub: Subscription,
     settings: Option<SettingsState>,
+    /// The window settings are drawn in, and the workspace window that opened
+    /// it (where focus goes back to on close). `None` while settings is shut —
+    /// and always in tests, which keep drawing settings over the workspace so
+    /// they can drive it through the one test window they have.
+    settings_window: Option<(gpui::AnyWindowHandle, gpui::AnyWindowHandle)>,
     pub(crate) ssh_prompt: crate::ui::ssh_prompt::SshPromptState,
     /// A close question is on screen. It carries no target: the answer acts on
     /// the tab or pane captured when the question was raised, not on whatever
@@ -1525,6 +1533,7 @@ impl Tty7App {
             file_search,
             _file_search_sub: file_search_sub,
             settings: None,
+            settings_window: None,
             ssh_prompt: crate::ui::ssh_prompt::SshPromptState::new(cx),
             close_prompt_open: false,
             window_bounds: window_bounds_to_remember(window),
@@ -1963,7 +1972,8 @@ impl Tty7App {
             TrayAction::SetNotifyMode(mode) => self.set_notify_mode(mode, cx),
             TrayAction::OpenSettings => {
                 surface_window(window, cx);
-                if self.settings.is_none() {
+                // Opens it, or brings its window forward — never shuts it.
+                if self.settings.is_none() || self.settings_window.is_some() {
                     self.toggle_settings(window, cx);
                 }
             }
@@ -6008,12 +6018,75 @@ impl Tty7App {
         cx.notify();
     }
 
+    /// Settings opens in a window of its own, beside the workspace rather
+    /// than over it. Asked for again while that window is up, it comes to the
+    /// front — a second ⌘, is someone looking for the window, not asking to
+    /// close it.
     fn toggle_settings(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+        if let Some((handle, _)) = self.settings_window {
+            if handle
+                .update(cx, |_, window, _| window.activate_window())
+                .is_ok()
+            {
+                return;
+            }
+            // The window went away without telling us.
+            self.settings_window = None;
+            self.settings = None;
+        }
         if self.settings.is_some() {
             self.close_settings_checked(window, cx);
             return;
         }
         self.remember_active_pane(window, cx);
+        if cfg!(test) {
+            self.build_settings_state(window, cx);
+            return;
+        }
+        self.open_settings_window_then(window, cx, None);
+    }
+
+    fn open_settings_window_then(
+        &mut self,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+        then: Option<SettingsFollowUp>,
+    ) {
+        let owner = window.window_handle();
+        let app = cx.entity();
+        // Deferred: the new window's state is built by updating this entity,
+        // which cannot happen while this call still holds it.
+        cx.defer(move |cx| {
+            let options = crate::ui::settings_window::window_options(cx);
+            let opened = cx.open_window(options, |window, cx| {
+                app.update(cx, |this, cx| {
+                    this.build_settings_state(window, cx);
+                    if let Some(then) = then {
+                        then(this, window, cx);
+                    }
+                });
+                let view =
+                    cx.new(|cx| crate::ui::settings_window::SettingsWindow::new(&app, window, cx));
+                cx.new(|cx| gpui_component::Root::new(view, window, cx))
+            });
+            match opened {
+                Ok(handle) => app.update(cx, |this, _| {
+                    this.settings_window = Some((handle.into(), owner));
+                }),
+                Err(e) => {
+                    log::error!("failed to open the settings window: {e}");
+                    app.update(cx, |this, cx| {
+                        this.settings = None;
+                        cx.notify();
+                    });
+                }
+            }
+        });
+    }
+
+    /// Build the settings page's state against `window` — the window it will
+    /// be drawn in, which owns its inputs' focus.
+    fn build_settings_state(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         let focus_handle = cx.focus_handle();
         let mut subs = Vec::new();
         let (font_select, font_bold_select, font_italic_select, ui_font_select) =
@@ -6680,7 +6753,36 @@ impl Tty7App {
                     s.content_scroll.offset()
                 },
             );
-            self.focus_active(window, cx);
+            match self.settings_window.take() {
+                // `window` is the settings window here, so the workspace is
+                // reached through its handle — after this update lets go of
+                // both it and this entity.
+                Some((settings, owner)) => {
+                    let app = cx.entity().downgrade();
+                    cx.defer(move |cx| {
+                        let _ = settings.update(cx, |_, window, _| window.remove_window());
+                        let _ = owner.update(cx, |_, window, cx| {
+                            window.activate_window();
+                            let _ = app.update(cx, |this, cx| this.focus_active(window, cx));
+                        });
+                    });
+                }
+                None => self.focus_active(window, cx),
+            }
+            cx.notify();
+        }
+    }
+
+    pub(crate) fn has_settings(&self) -> bool {
+        self.settings.is_some()
+    }
+
+    /// The settings window was closed out from under its state — its owner
+    /// is going away, or the window was removed without passing through
+    /// `close_settings`. Drop the state without trying to hand focus back.
+    pub(crate) fn forget_settings_window(&mut self, cx: &mut Context<Self>) {
+        if self.settings_window.take().is_some() {
+            self.settings = None;
             cx.notify();
         }
     }
@@ -6691,10 +6793,63 @@ impl Tty7App {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
+        self.open_settings_then(window, cx, move |this, window, cx| {
+            this.navigate_settings(section, None, window, cx)
+        });
+    }
+
+    /// Open settings (or bring its window forward) and then run `then` against
+    /// the window the page is drawn in, once its state exists. Anything that
+    /// builds inputs for the page — loading a form — has to run there: an
+    /// input belongs to the window it was made in.
+    pub(crate) fn open_settings_then(
+        &mut self,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+        then: impl FnOnce(&mut Self, &mut Window, &mut Context<Self>) + 'static,
+    ) {
+        if let Some((handle, _)) = self.settings_window {
+            let app = cx.entity().downgrade();
+            cx.defer(move |cx| {
+                let _ = handle.update(cx, |_, window, cx| {
+                    window.activate_window();
+                    let _ = app.update(cx, |this, cx| then(this, window, cx));
+                });
+            });
+            return;
+        }
         if self.settings.is_none() {
-            self.toggle_settings(window, cx);
+            self.remember_active_pane(window, cx);
+            if !cfg!(test) {
+                self.open_settings_window_then(window, cx, Some(Box::new(then)));
+                return;
+            }
+            self.build_settings_state(window, cx);
+        }
+        then(self, window, cx);
+    }
+
+    /// Close settings, then run `then` in the workspace window — where a tab it
+    /// opens belongs, whichever window the request came from.
+    pub(crate) fn close_settings_then(
+        &mut self,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+        then: impl FnOnce(&mut Self, &mut Window, &mut Context<Self>) + 'static,
+    ) {
+        let owner = self.settings_window.map(|(_, owner)| owner);
+        self.close_settings(window, cx);
+        match owner {
+            None => then(self, window, cx),
+            Some(owner) => {
+                let app = cx.entity().downgrade();
+                cx.defer(move |cx| {
+                    let _ = owner.update(cx, |_, window, cx| {
+                        let _ = app.update(cx, |this, cx| then(this, window, cx));
+                    });
+                });
+            }
         }
-        self.navigate_settings(section, None, window, cx);
     }
 
     /// Resolve a pending form before performing both parts of an external
@@ -6706,8 +6861,10 @@ impl Tty7App {
         cx: &mut Context<Self>,
     ) {
         self.with_settings_edits_resolved(window, cx, move |this, window, cx| {
-            this.open_settings_section(SettingsSection::Ssh, window, cx);
-            this.ssh_form_load(&profile, window, cx);
+            this.open_settings_then(window, cx, move |this, window, cx| {
+                this.navigate_settings(SettingsSection::Ssh, None, window, cx);
+                this.ssh_form_load(&profile, window, cx);
+            });
         });
     }
 
@@ -8434,19 +8591,20 @@ impl Render for Tty7App {
         let bg_image = window_background_image_layer(cx);
         let settings_bg = crate::ui::theme::overlay_background(cx);
 
-        let settings_overlay = self.settings.is_some().then(|| {
-            div()
-                .absolute()
-                .inset_0()
-                .occlude()
-                // Opaque on purpose: the settings panel must never let the
-                // workspace translucency (window opacity / backdrop material)
-                // show through, even at window edges during a resize. The
-                // preset's gradient fill is preserved, just with alpha 1;
-                // `render_settings` repaints the theme image over it.
-                .bg(settings_bg)
-                .child(self.render_settings(window, cx))
-        });
+        let settings_overlay =
+            (self.settings.is_some() && self.settings_window.is_none()).then(|| {
+                div()
+                    .absolute()
+                    .inset_0()
+                    .occlude()
+                    // Opaque on purpose: the settings panel must never let the
+                    // workspace translucency (window opacity / backdrop material)
+                    // show through, even at window edges during a resize. The
+                    // preset's gradient fill is preserved, just with alpha 1;
+                    // `render_settings` repaints the theme image over it.
+                    .bg(settings_bg)
+                    .child(self.render_settings(window, cx))
+            });
 
         let root =
             div()

src/ui/mod.rs

@@ -35,6 +35,7 @@ pub mod rounding;
 pub mod scm;
 pub mod scrollbar;
 pub mod settings;
+pub mod settings_window;
 pub mod sftp;
 pub mod sftp_host;
 pub mod ssh_connect;

src/ui/settings.rs

@@ -4636,8 +4636,9 @@ impl Tty7App {
         let Some(qc) = crate::core::ssh_profile::parse_quick_connect(&target) else {
             return;
         };
-        self.close_settings(window, cx);
-        self.quick_connect(qc, window, cx);
+        self.close_settings_then(window, cx, move |this, window, cx| {
+            this.quick_connect(qc, window, cx)
+        });
     }
 
     fn render_ssh_defaults_detail(&self, cx: &mut Context<Self>) -> AnyElement {
@@ -4701,8 +4702,9 @@ impl Tty7App {
                 let app = app.clone();
                 move |_, window, cx| {
                     let _ = app.update(cx, |this, cx| {
-                        this.close_settings(window, cx);
-                        this.connect_ssh_profile(id, window, cx);
+                        this.close_settings_then(window, cx, move |this, window, cx| {
+                            this.connect_ssh_profile(id, window, cx)
+                        });
                     });
                 }
             }))
@@ -5421,8 +5423,9 @@ impl Tty7App {
 
     pub(crate) fn save_and_connect_profile(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         if let Some(id) = self.save_editing_profile(window, cx) {
-            self.close_settings(window, cx);
-            self.connect_ssh_profile(id, window, cx);
+            self.close_settings_then(window, cx, move |this, window, cx| {
+                this.connect_ssh_profile(id, window, cx)
+            });
         }
     }
 

src/ui/settings_window.rs

@@ -0,0 +1,111 @@
+//! Settings in a window of its own.
+//!
+//! The settings page used to cover the workspace it was opened from, which hid
+//! the terminal a setting was being tried out on. It now opens beside it. The
+//! state and the page itself still belong to the workspace's `Tty7App` — this
+//! window only borrows it to draw — so every setting keeps the one code path it
+//! had, and a change repaints both windows.
+
+use gpui::{
+    App, Context, Entity, InteractiveElement as _, IntoElement, ParentElement as _, Render,
+    Styled as _, Subscription, TitlebarOptions, WeakEntity, Window, WindowBounds,
+    WindowDecorations, WindowOptions, prelude::FluentBuilder as _, px, size,
+};
+use gpui_component::{ActiveTheme as _, TitleBar};
+
+use crate::core::actions::{CloseActiveTab, OpenSettings};
+use crate::core::config::Config;
+use crate::ui::app::Tty7App;
+
+/// Big enough for the navigation column and a comfortable page beside it.
+const DEFAULT_SIZE: (f32, f32) = (960., 700.);
+const MIN_SIZE: (f32, f32) = (720., 480.);
+
+pub(crate) fn window_options(cx: &mut App) -> WindowOptions {
+    WindowOptions {
+        window_bounds: Some(WindowBounds::Windowed(gpui::Bounds::centered(
+            None,
+            size(px(DEFAULT_SIZE.0), px(DEFAULT_SIZE.1)),
+            cx,
+        ))),
+        app_id: Some("tty7".to_owned()),
+        titlebar: Some(TitlebarOptions {
+            traffic_light_position: Some(crate::ui::theme::traffic_light_position()),
+            ..TitleBar::title_bar_options()
+        }),
+        window_decorations: Some(WindowDecorations::Client),
+        window_background: crate::ui::theme::background_appearance(cx),
+        window_min_size: Some(size(px(MIN_SIZE.0), px(MIN_SIZE.1))),
+        ..Default::default()
+    }
+}
+
+pub(crate) struct SettingsWindow {
+    app: WeakEntity<Tty7App>,
+    _observe: Subscription,
+    _release: Subscription,
+}
+
+impl SettingsWindow {
+    pub(crate) fn new(app: &Entity<Tty7App>, window: &mut Window, cx: &mut Context<Self>) -> Self {
+        // Every settings change notifies the app, not this view; without this
+        // the page would change underneath and never repaint here.
+        let observe = cx.observe(app, |_, _, cx| cx.notify());
+        // The workspace that owns the page is gone, so the page is too.
+        let release = cx.observe_release_in(app, window, |_, _, window, _| window.remove_window());
+        // The close button asks the same question Escape does: a half-edited
+        // form or theme draft gets its prompt, and the window only goes once
+        // it is answered.
+        let weak = app.downgrade();
+        window.on_window_should_close(cx, move |window, cx| match weak.upgrade() {
+            Some(app) => {
+                app.update(cx, |this, cx| this.close_settings_checked(window, cx));
+                false
+            }
+            None => true,
+        });
+        Self {
+            app: app.downgrade(),
+            _observe: observe,
+            _release: release,
+        }
+    }
+
+    fn close(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+        if let Some(app) = self.app.upgrade() {
+            app.update(cx, |this, cx| this.close_settings_checked(window, cx));
+        }
+    }
+}
+
+impl Render for SettingsWindow {
+    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        // The workspace window sets this in its own render; a second window
+        // has to, or the page lays out against the default 16px rem.
+        window.set_rem_size(px(cx.global::<Config>().ui_font_size));
+        let page = self.app.upgrade().map(|app| {
+            let page = app.update(cx, |this, cx| {
+                // The state can be gone for a frame between closing and the
+                // window being removed.
+                this.has_settings()
+                    .then(|| this.render_settings(window, cx).into_any_element())
+            });
+            if page.is_none() {
+                // Closed without passing through `close_settings` — nothing
+                // is left to draw, so the window goes too.
+                app.update(cx, |this, cx| this.forget_settings_window(cx));
+                window.remove_window();
+            }
+            page
+        });
+        gpui::div()
+            .size_full()
+            .bg(crate::ui::theme::overlay_background(cx))
+            .text_color(cx.theme().foreground)
+            // ⌘W closes this window the way it closes a tab in a workspace.
+            .on_action(cx.listener(|this, _: &CloseActiveTab, window, cx| this.close(window, cx)))
+            // ⌘, lands here while this window has focus; it is already open.
+            .on_action(cx.listener(|_, _: &OpenSettings, window, _| window.activate_window()))
+            .when_some(page.flatten(), |root, page| root.child(page))
+    }
+}

src/ui/theme.rs

@@ -597,6 +597,17 @@ pub(crate) fn apply_theme(mut window: Option<&mut Window>, cx: &mut App) {
         if take_appearance_change(window, appearance, cx) {
             window.set_background_appearance(appearance);
         }
+        // Opacity, blur and backdrop are app-wide settings, but they are
+        // changed from whichever window the settings page is in — its own,
+        // now. Hand the same appearance to every other window too. The one
+        // being updated refuses a nested update, and was handled above.
+        for other in cx.windows() {
+            let _ = other.update(cx, |_, window, cx| {
+                if take_appearance_change(window, appearance, cx) {
+                    window.set_background_appearance(appearance);
+                }
+            });
+        }
     }
 
     Theme::change(mode, window.as_deref_mut(), cx);