diff --git a/docs/features.md b/docs/features.md index f9879ca9..282aed19 100644 --- a/docs/features.md +++ b/docs/features.md @@ -106,6 +106,7 @@ Keys are shown in macOS notation — on Windows and Linux, read a | ⌘ F | search the scrollback | | ⌃ R | fuzzy-search shell history | | ⌘ + · ⌘ − · ⌘ 0 | font size up · down · reset | +| + wheel | zoom the font by scrolling over a terminal | **Settings → Keybindings** (⌘ ,) lists every shortcut. Click one, press the new keys (Esc cancels, Backspace resets to diff --git a/docs/features.zh-CN.md b/docs/features.zh-CN.md index 8c205e7a..a66b6293 100644 --- a/docs/features.zh-CN.md +++ b/docs/features.zh-CN.md @@ -103,6 +103,7 @@ Aider、Amp、OpenCode 等共 18 个)并在其外围加功能 —— 绝不包 | ⌘ F | 搜索 scrollback | | ⌃ R | 模糊搜索 shell 历史 | | ⌘ + · ⌘ − · ⌘ 0 | 字号增大 · 减小 · 重置 | +| + 滚轮 | 在终端上滚动缩放字号,演示时随手放大 | **设置 → 按键绑定**(⌘ ,)列出全部快捷键。点一行、按下新键即可 (Esc 取消,Backspace 恢复默认),改完立即生效。窗格缩放与 diff --git a/src/terminal/parked_cursor.rs b/src/terminal/parked_cursor.rs index 8b3fbd25..facad126 100644 --- a/src/terminal/parked_cursor.rs +++ b/src/terminal/parked_cursor.rs @@ -10,13 +10,19 @@ //! on every batch of pty output) draws that stray cursor for a frame, and a //! TUI that repaints on a spinner — Codex while it works — produces one every //! spinner tick, which reads as a second cursor blinking in the wrong place. -//! macOS never shows it: no ConPTY sits in between, and the TUI itself always -//! moves the cursor before it shows it. //! //! [`ParkedCursorScanner`] finds those hide/show pairs in the byte stream and //! [`ParkedCursorRepair`] restores the cell the cursor stood on when it went //! invisible, which is the cell the correcting frame would have moved it back //! to anyway. +//! +//! Only conhost parks a cursor, so the reader runs this on Windows alone +//! (`RemoteTerminal::REPAIR_PARKED_CURSOR`). Off it the pty is raw and the +//! application's cursor is the real one: a TUI is free to end a repaint on the +//! text it just wrote and then echo the next keystroke straight after it, with +//! no positioning of its own — vim opens its `:` command line exactly that way, +//! which a repair on a raw pty turns into `wq!` landing on the row being edited +//! (#430). use std::time::{Duration, Instant}; diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 31e64d31..7917d5cf 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -553,6 +553,17 @@ impl RemoteTerminal { term.set_options(terminal_config_from_user(user_config)); } + /// Whether this build puts back the cursor a repaint parked — see + /// [`crate::terminal::parked_cursor`]. + /// + /// Only conhost parks one, so like `conpty_resize` the repair is Windows' + /// alone. On a raw pty the application owns the cursor and is free to end a + /// repaint on the text it just wrote and then echo the next keystroke + /// straight after it, with no positioning of its own: vim opens its command + /// line that way, and putting the cursor back on the cell the repaint hid it + /// on drops the `wq!` typed next onto the row being edited (#430). + const REPAIR_PARKED_CURSOR: bool = cfg!(windows); + fn spawn_reader( term: Arc>>, proxy: EventProxy, @@ -637,7 +648,9 @@ impl RemoteTerminal { // emulator to the cut, act on the state that // sequence left behind, carry on. let mut cuts: Vec<(usize, CursorCut)> = Vec::new(); - cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); + if Self::REPAIR_PARKED_CURSOR { + cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); + } { let t0 = trace.then(std::time::Instant::now); let mut term = term.lock(); @@ -3067,6 +3080,7 @@ mod tests { /// Feeds one conhost-shaped repaint and reports the cell the cursor ends on, /// waiting for the `X` the frame paints so the reader is known to be done. fn cursor_after_conpty_frame(frame: &[u8]) -> (i32, usize) { + crate::core::config::pin_test_config_dir(); let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); @@ -3097,14 +3111,24 @@ mod tests { #[test] fn a_conpty_frame_that_shows_the_cursor_over_an_erase_keeps_the_cell_it_hid_on() { - assert_eq!( - cursor_after_conpty_frame( - b"\x1b[?25l\x1b[20;2HX\x1b[K\x1b[m\x1b[22;42H\x1b[K\x1b[?25h" - ), - (5, 3), - "conhost parked the cursor on the cell it erased last; the cursor \ - belongs where it was when the repaint hid it" + let got = cursor_after_conpty_frame( + b"\x1b[?25l\x1b[20;2HX\x1b[K\x1b[m\x1b[22;42H\x1b[K\x1b[?25h", ); + if RemoteTerminal::REPAIR_PARKED_CURSOR { + assert_eq!( + got, + (5, 3), + "conhost parked the cursor on the cell it erased last; the cursor \ + belongs where it was when the repaint hid it" + ); + } else { + assert_eq!( + got, + (21, 41), + "with no conhost in between the stream is the application's own, \ + and the cell it left the cursor on is the cell it meant" + ); + } } #[test] @@ -3118,6 +3142,70 @@ mod tests { ); } + /// Issue #430. Vim opens its command line with exactly the shape the parked + /// -cursor scanner calls parked — hide, move around to paint, end on the `:` + /// it wrote — and then echoes every following keystroke as a bare byte at + /// wherever that left the cursor. Putting the cursor back on a raw pty + /// therefore does not straighten out a stray caret, it drops `wq!` onto the + /// row vim was editing. Bytes below are a capture of vim 9 on a 20x11 pty. + #[test] + fn a_raw_pty_repaint_keeps_the_cursor_the_frame_left_so_the_echo_lands_on_it() { + crate::core::config::pin_test_config_dir(); + let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(20, 11)).unwrap(); + + let mut stream: Vec = Vec::new(); + // `vim test.md`: the alternate screen, the file, cursor home. + stream.extend_from_slice(b"\x1b[?1049h\x1b[H\x1b[2J\x1b[1;1H123456789\x1b[1;1H"); + // Esc, then `:` — two bracketed repaints, the second ending on the `:` + // vim wrote at the head of the command line. + stream.extend_from_slice(b"\x1b[?25l\x1b[m\x1b[11;10H^[\x1b[1;1H\x1b[?25h"); + stream.extend_from_slice(b"\x1b[?25l\x1b[11;10H \x1b[1;1H\x07\x1b[?25h"); + stream.extend_from_slice( + b"\x1b[?25l\x1b[11;10H:\x1b[1;1H\x1b[11;1H\x1b[K\x1b[11;1H:\x1b[?25h", + ); + // `w`, `q`, `!`: vim echoes them with no positioning of their own. + stream.extend_from_slice(b"wq!"); + DaemonMsg::Output(stream).encode(&mut daemon_side).unwrap(); + daemon_side.flush().unwrap(); + + let row = |t: &Term, line: i32| -> String { + (0..20) + .map(|col| { + t.grid()[alacritty_terminal::index::Line(line)] + [alacritty_terminal::index::Column(col)] + .c + }) + .collect::() + .trim_end() + .to_string() + }; + + // The whole batch is applied under one lock, so the `:` landing on the + // command line means every byte after it landed too. + let mut command_line = String::new(); + let mut edited = String::new(); + for _ in 0..600 { + { + let t = term.term.lock(); + command_line = row(&t, 10); + edited = row(&t, 0); + } + if command_line.starts_with(':') { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!( + command_line, ":wq!", + "the keystrokes belong after the `:` the repaint ended on" + ); + assert_eq!( + edited, "123456789", + "and nothing of them belongs on the row vim was editing" + ); + } + #[test] fn layout_resize_reasserts_geometry_after_a_late_size_frame() { use alacritty_terminal::grid::Dimensions as _; diff --git a/src/terminal/view.rs b/src/terminal/view.rs index da67d812..5e5a3933 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -23,8 +23,9 @@ use super::reverse_search::{self, ReverseSearch}; use super::search::{LinkTarget, SearchState}; use super::typeahead::{RawInput, Typeahead}; use crate::core::actions::{ - CloseActiveTab, ForkAgentSessionDown, ForkAgentSessionLeft, ForkAgentSessionRight, - ForkAgentSessionUp, NewTab, SendBackTab, SendTab, SplitDown, SplitRight, ToggleMaximizePane, + CloseActiveTab, DecreaseFontSize, ForkAgentSessionDown, ForkAgentSessionLeft, + ForkAgentSessionRight, ForkAgentSessionUp, IncreaseFontSize, NewTab, SendBackTab, SendTab, + SplitDown, SplitRight, ToggleMaximizePane, }; use crate::core::config::{BellMode, Config, NotifyMode}; use crate::daemon::protocol::{RemoteContext, ShellSpec}; @@ -113,6 +114,10 @@ const SCROLL_ANIM_MIN_JUMP: f32 = 1.0; /// to bridge the gaps in a momentum tail, short enough that reaching for the /// wheel right after a swipe is not mistaken for more of the swipe. const SCROLL_GESTURE_IDLE: std::time::Duration = std::time::Duration::from_millis(150); +/// How far a trackpad has to travel, in lines, to earn one font-size step while +/// the platform modifier is held. A wheel detent is one step on its own, so +/// this only ever applies to the continuous stream a gesture produces. +const ZOOM_SCROLL_LINES: f32 = 3.0; fn cwd_is_on_host(pane_runs_remotely: bool, host_is_local: bool) -> bool { match pane_runs_remotely { @@ -160,6 +165,11 @@ pub struct TerminalView { /// modifiers the user actually held. context_menu_allowed: bool, scroll_debt: f32, + /// Lines travelled under the zoom modifier that have not yet added up to a + /// font-size step. Kept apart from [`scroll_debt`](Self::scroll_debt) so + /// letting go of the modifier mid-gesture cannot hand the leftovers of one + /// to the other. + zoom_debt: f32, pub(super) scroll_frac: f32, pub search: Option, pub cursor_visible: bool, @@ -1095,6 +1105,7 @@ impl TerminalView { link_modifier_down: false, context_menu_allowed: true, scroll_debt: 0., + zoom_debt: 0., scroll_frac: 0., search: None, cursor_visible: true, @@ -4259,6 +4270,14 @@ impl TerminalView { } fn on_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + // The platform modifier alone turns the wheel into a zoom, the way it + // does in a browser. Any other modifier alongside it is somebody else's + // gesture — shift in particular is the escape hatch that scrolls the + // scrollback out from under a mouse-reporting program. + if ev.modifiers.secondary() && ev.modifiers.number_of_modifiers() == 1 { + self.zoom_scroll(ev, window, cx); + return; + } let mult = cx.global::().mouse_scroll_multiplier; let raw = match ev.delta { ScrollDelta::Lines(p) => p.y, @@ -4293,6 +4312,34 @@ impl TerminalView { } } + /// Resize the terminal font by whole steps under the platform modifier. + /// + /// The event never reaches the buffer, and never reaches the program + /// running in it either: zooming is chrome, and showing a pane to someone + /// standing behind you has to work the same whether or not what is running + /// asked for the wheel. Steps go out as the same actions the keyboard and + /// the View menu send, so the min/max clamp and the saved setting live in + /// one place — [`Tty7App::change_font_size`](crate::ui::app::Tty7App). + fn zoom_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + // Whatever the scrollback still had in flight is dropped: it was + // travelling in lines of a font that is about to change size. + self.cancel_scroll_anim(); + let lines = match ev.delta { + ScrollDelta::Lines(p) => p.y, + ScrollDelta::Pixels(p) => p.y.as_f32() / self.line_height.as_f32(), + }; + let gesturing = self.track_scroll_gesture(ev.touch_phase); + let (steps, debt) = zoom_scroll_steps(lines, self.zoom_debt, gesturing); + self.zoom_debt = debt; + for _ in 0..steps.unsigned_abs() { + if steps > 0 { + window.dispatch_action(Box::new(IncreaseFontSize), cx); + } else { + window.dispatch_action(Box::new(DecreaseFontSize), cx); + } + } + } + /// Track whether the pointing device is mid-gesture, which is what tells a /// trackpad apart from a wheel. /// @@ -5871,6 +5918,30 @@ fn wrapped_click_index( } } +/// How many font-size steps a zoom event is worth, and what is left over for +/// the next one. +/// +/// A wheel detent is a discrete click of intent, so it is one step whatever the +/// platform says it covers — macOS calls a single notch five lines, and five +/// points of font per notch would be unusable. A trackpad has no detents and +/// spends a flick over dozens of events, so those accumulate and only pay out +/// once the fingers have travelled [`ZOOM_SCROLL_LINES`]. +fn zoom_scroll_steps(lines: f32, debt: f32, gesturing: bool) -> (i32, f32) { + if !gesturing { + let step = if lines > 0. { + 1 + } else if lines < 0. { + -1 + } else { + 0 + }; + return (step, 0.); + } + let total = debt + lines; + let steps = (total / ZOOM_SCROLL_LINES).trunc(); + (steps as i32, total - steps * ZOOM_SCROLL_LINES) +} + fn smooth_scroll_step(offset: usize, frac: f32, delta: f32, max: usize) -> (i32, f32) { let pos = (offset as f32 + frac + delta).clamp(0., max as f32); let new_offset = pos.floor(); @@ -9003,6 +9074,46 @@ mod gpui_tests { .unwrap(); } + /// Zooming has to take the wheel away from the buffer entirely, or the + /// grid would slide under the pointer while the font changed size. + #[gpui::test] + fn the_zoom_modifier_takes_the_wheel_off_the_scrollback(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, w, cx| { + scroll_into_history(view, 10); + let mut ev = notch(view, -4.9); + ev.modifiers = Modifiers::secondary_key(); + view.on_scroll(&ev, w, cx); + assert_eq!(display_offset(view), 10, "the wheel reached the grid"); + assert!(view.scroll_anim.is_none(), "and queued more of it"); + }) + .unwrap(); + } + + /// A detent is one step however many lines the platform bills it as — + /// macOS calls a single notch five. + #[test] + fn a_wheel_detent_zooms_by_exactly_one_step() { + assert_eq!(zoom_scroll_steps(4.9, 0., false), (1, 0.)); + assert_eq!(zoom_scroll_steps(-4.9, 0., false), (-1, 0.)); + assert_eq!(zoom_scroll_steps(0., 0., false), (0, 0.)); + } + + /// A trackpad has no detents, so a flick arrives as a stream of slivers. + /// Paying out a step per sliver would run the font from end to end. + #[test] + fn a_trackpad_flick_adds_up_to_whole_steps() { + let (mut debt, mut steps) = (0., 0); + for _ in 0..20 { + let (s, d) = zoom_scroll_steps(1. / 3., debt, true); + steps += s; + debt = d; + } + assert_eq!(steps, 2, "twenty thirds of a line is two steps, not twenty"); + assert!(debt > 0., "and the remainder was dropped instead of kept"); + } + /// The one thing typing at a prompt must never do is land where the /// person typing cannot see it. #[gpui::test] diff --git a/src/ui/app.rs b/src/ui/app.rs index edc020c9..ff7d661f 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -413,6 +413,10 @@ pub struct Tty7App { _appearance_watch: Subscription, palette: Option>, palette_sub: Option, + /// Preset that was live when the palette's theme picker started previewing. + /// `Some` means the theme on screen is a preview that was never written to + /// disk, and closing the palette without confirming puts this one back. + theme_preview_restore: Option, pub(crate) closed: Vec, pub(crate) renaming: Option, pub(crate) worktree_prompt: Option, @@ -436,9 +440,13 @@ pub struct Tty7App { pub(crate) sidebar_dragging: Rc>, /// How much width a settings row will actually get, measured once per /// render. `settings_row` is called from page builders that never see the - /// window, and the answer differs per page — the SSH page spends 500px on - /// its two lists first. + /// window, and the answer differs per page — the SSH page spends a host + /// list on top of the nav before the row gets anything. pub(crate) settings_row_width: Cell, + /// The window width the settings chrome sized itself against, measured in + /// the same pass. The pages that render their own chrome — the SSH host + /// list, the theme panel — are as blind to the window as `settings_row` is. + pub(crate) settings_viewport_w: Cell, /// Cleared at the top of every settings render, then set by the first row /// the live search matched, so exactly one row per page carries the anchor /// the page scrolls to. @@ -936,6 +944,7 @@ impl Tty7App { _appearance_watch: appearance_watch, palette: None, palette_sub: None, + theme_preview_restore: None, closed: Vec::new(), renaming: None, worktree_prompt: None, @@ -966,6 +975,7 @@ impl Tty7App { sidebar_width: Rc::new(Cell::new(sidebar_width)), sidebar_dragging: Rc::new(Cell::new(false)), settings_row_width: Cell::new(f32::MAX), + settings_viewport_w: Cell::new(f32::MAX), settings_hit_anchored: Cell::new(false), right_panel_width: Rc::new(Cell::new(right_panel_width)), right_panel_dragging: Rc::new(Cell::new(false)), @@ -1533,6 +1543,14 @@ impl Tty7App { } pub(crate) fn set_preset(&mut self, id: &str, window: &mut Window, cx: &mut Context) { + // A confirmed pick ends any preview: there is nothing left to roll back. + self.theme_preview_restore = None; + self.write_preset(id, cx); + self.after_theme_change(window, cx); + } + + /// Points whichever preset slot is live at `id`, in memory only. + fn write_preset(&mut self, id: &str, cx: &mut Context) { let dark_now = crate::ui::theme::system_dark(cx); let cfg = cx.global_mut::(); if !cfg.theme_follow_system { @@ -1542,7 +1560,27 @@ impl Tty7App { } else { cfg.theme_preset_light = id.to_string(); } - self.after_theme_change(window, cx); + } + + /// Shows a preset for as long as the palette's theme picker is open, so + /// arrowing through the list is how you find out what a theme looks like. + /// Nothing is written to `config.json` until the pick is confirmed. + pub(crate) fn preview_preset(&mut self, id: &str, window: &mut Window, cx: &mut Context) { + if self.theme_preview_restore.is_none() { + self.theme_preview_restore = Some(crate::ui::theme::effective_preset_id(cx)); + } + self.write_preset(id, cx); + self.apply_theme_change(false, window, cx); + } + + /// Puts back the preset that was live before the preview started. A no-op + /// when nothing is being previewed. + pub(crate) fn cancel_preset_preview(&mut self, window: &mut Window, cx: &mut Context) { + let Some(id) = self.theme_preview_restore.take() else { + return; + }; + self.write_preset(&id, cx); + self.apply_theme_change(false, window, cx); } pub(crate) fn set_slot_preset( @@ -1609,9 +1647,18 @@ impl Tty7App { } fn after_theme_change(&mut self, window: &mut Window, cx: &mut Context) { + self.apply_theme_change(true, window, cx); + } + + /// Repaints everything a theme change touches. `persist` is false for a + /// palette preview, which repaints on every arrow key and must not turn + /// each of those keystrokes into a `config.json` write. + fn apply_theme_change(&mut self, persist: bool, window: &mut Window, cx: &mut Context) { apply_theme(Some(window), cx); set_menus(cx); - cx.global::().save(); + if persist { + cx.global::().save(); + } self.rebuild_theme_editor(window, cx); self.sync_window_opacity_slider(window, cx); cx.notify(); @@ -3749,16 +3796,30 @@ impl Tty7App { match ev { PaletteEvent::Confirm(kind) => { let kind = kind.clone(); + // The picker is already showing this theme; keep it through the + // close instead of reverting and re-applying it. + if matches!(kind, CommandKind::SetTheme(_)) { + self.theme_preview_restore = None; + } self.close_palette(window, cx); self.run_command(kind, window, cx); } PaletteEvent::Dismiss => self.close_palette(window, cx), + PaletteEvent::PreviewTheme(i) => { + if let Some(id) = crate::ui::presets::all(cx).get(*i).map(|t| t.id.clone()) { + self.preview_preset(&id, window, cx); + } + } + PaletteEvent::CancelThemePreview => self.cancel_preset_preview(window, cx), } } pub(crate) fn close_palette(&mut self, window: &mut Window, cx: &mut Context) { self.palette = None; self.palette_sub = None; + // A previewed theme was never persisted: closing the palette any way + // other than confirming the pick puts the old one back. + self.cancel_preset_preview(window, cx); self.focus_active(window, cx); cx.notify(); } diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 7379833b..5b5fb606 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1127,6 +1127,25 @@ pub fn t_fmt(key: L10nKey, args: &[(&str, &str)]) -> String { apply_template(t(key), args, None) } +/// The same key as every *other* locale words it. +/// +/// Fingers learn a command's name once. Someone running the UI in Chinese who +/// picked the habit up in English still types `theme`, and until this existed +/// the palette answered "no matching commands" — the label it was matching +/// against was the only one that existed. Callers keep these as hidden search +/// aliases; nothing renders them, so the wording on screen never changes. +pub fn alias_translations(key: L10nKey) -> Vec<&'static str> { + let shown = t(key); + let mut out = Vec::with_capacity(SUPPORTED_LANGUAGES.len() - 1); + for idx in 0..SUPPORTED_LANGUAGES.len() { + let text = translate(idx, key); + if text != shown && !out.contains(&text) { + out.push(text); + } + } + out +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PluralCategory { Zero, diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 9dbfd769..d11bb4c2 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -12,7 +12,7 @@ use uuid::Uuid; use crate::core::config::{Config, RightPanelTab, TabBarPosition}; use crate::core::ssh_profile::parse_quick_connect; -use crate::ui::i18n::{L10nKey, t, t_fmt}; +use crate::ui::i18n::{L10nKey, alias_translations, t, t_fmt}; #[derive(Clone, PartialEq, Eq)] pub enum CommandKind { @@ -317,6 +317,10 @@ pub struct ChromeState { pub struct Command { pub title: String, pub subtitle: Option, + /// Text the query may match but the row never shows: the same label as + /// every other locale words it, plus the stable command id. Built once, + /// with the entry — the filter runs on every keystroke. + pub aliases: Vec<&'static str>, pub kind: CommandKind, pub group: CommandGroup, } @@ -326,11 +330,22 @@ impl Command { Self { title: title.into(), subtitle: None, + // The id is English and hyphenated ("change-theme"), which is close + // enough to what someone reaching past a translated label types. + aliases: kind.id().into_iter().collect(), kind, group: CommandGroup::Application, } } + /// A command whose label comes out of the locale table, and which can + /// therefore also be found by the wording any other locale would show. + pub fn localized(key: L10nKey, kind: CommandKind) -> Self { + let mut cmd = Self::new(t(key), kind); + cmd.aliases.extend(alias_translations(key)); + cmd + } + pub fn with_subtitle(mut self, subtitle: impl Into) -> Self { self.subtitle = Some(subtitle.into()); self @@ -349,129 +364,129 @@ impl Command { let right_panel_open = chrome.right_panel_visible; let tabs = [ - Command::new(t(L10nKey::CmdNewTab), NewTab), - Command::new(t(L10nKey::CmdNewWorktreeTab), NewWorktreeTab) + Command::localized(L10nKey::CmdNewTab, NewTab), + Command::localized(L10nKey::CmdNewWorktreeTab, NewWorktreeTab) .with_subtitle(t(L10nKey::CmdNewWorktreeTabSubtitle)), - Command::new(t(L10nKey::CmdRenameTab), RenameTab), - Command::new(t(L10nKey::CmdSplitRight), SplitRight), - Command::new(t(L10nKey::CmdSplitDown), SplitDown), - Command::new(t(L10nKey::CmdZoomPane), ToggleMaximizePane), - Command::new(t(L10nKey::CmdNextPane), NextPane), - Command::new(t(L10nKey::CmdPreviousPane), PrevPane), - Command::new(t(L10nKey::CmdFocusPaneLeft), FocusPaneLeft), - Command::new(t(L10nKey::CmdFocusPaneRight), FocusPaneRight), - Command::new(t(L10nKey::CmdFocusPaneUp), FocusPaneUp), - Command::new(t(L10nKey::CmdFocusPaneDown), FocusPaneDown), - Command::new(t(L10nKey::CmdResizePaneLeft), ResizePaneLeft), - Command::new(t(L10nKey::CmdResizePaneRight), ResizePaneRight), - Command::new(t(L10nKey::CmdResizePaneUp), ResizePaneUp), - Command::new(t(L10nKey::CmdResizePaneDown), ResizePaneDown), - Command::new(t(L10nKey::CmdSwapPaneNext), SwapPaneNext), - Command::new(t(L10nKey::CmdSwapPanePrevious), SwapPanePrev), - Command::new(t(L10nKey::CmdNextTab), NextTab), - Command::new(t(L10nKey::CmdPreviousTab), PrevTab), - Command::new(t(L10nKey::CmdCopyWorkingDirectory), CopyWorkingDirectory), - Command::new(t(L10nKey::CmdCopySessionId), CopyAgentSessionId) + Command::localized(L10nKey::CmdRenameTab, RenameTab), + Command::localized(L10nKey::CmdSplitRight, SplitRight), + Command::localized(L10nKey::CmdSplitDown, SplitDown), + Command::localized(L10nKey::CmdZoomPane, ToggleMaximizePane), + Command::localized(L10nKey::CmdNextPane, NextPane), + Command::localized(L10nKey::CmdPreviousPane, PrevPane), + Command::localized(L10nKey::CmdFocusPaneLeft, FocusPaneLeft), + Command::localized(L10nKey::CmdFocusPaneRight, FocusPaneRight), + Command::localized(L10nKey::CmdFocusPaneUp, FocusPaneUp), + Command::localized(L10nKey::CmdFocusPaneDown, FocusPaneDown), + Command::localized(L10nKey::CmdResizePaneLeft, ResizePaneLeft), + Command::localized(L10nKey::CmdResizePaneRight, ResizePaneRight), + Command::localized(L10nKey::CmdResizePaneUp, ResizePaneUp), + Command::localized(L10nKey::CmdResizePaneDown, ResizePaneDown), + Command::localized(L10nKey::CmdSwapPaneNext, SwapPaneNext), + Command::localized(L10nKey::CmdSwapPanePrevious, SwapPanePrev), + Command::localized(L10nKey::CmdNextTab, NextTab), + Command::localized(L10nKey::CmdPreviousTab, PrevTab), + Command::localized(L10nKey::CmdCopyWorkingDirectory, CopyWorkingDirectory), + Command::localized(L10nKey::CmdCopySessionId, CopyAgentSessionId) .with_subtitle(t(L10nKey::CmdCopySessionIdSubtitle)), - Command::new(t(L10nKey::CmdForkSession), ForkAgentSession) + Command::localized(L10nKey::CmdForkSession, ForkAgentSession) .with_subtitle(t(L10nKey::CmdForkSessionSubtitle)), - Command::new(t(L10nKey::CmdMarkTabAsUnread), MarkTabUnread), - Command::new(t(L10nKey::CmdClosePaneTab), ClosePane), - Command::new(t(L10nKey::CmdCloseOtherTabs), CloseOtherTabs), - Command::new(t(L10nKey::CmdCloseTabsToTheRight), CloseTabsToTheRight), - Command::new(t(L10nKey::CmdReopenClosedTab), ReopenClosedTab), + Command::localized(L10nKey::CmdMarkTabAsUnread, MarkTabUnread), + Command::localized(L10nKey::CmdClosePaneTab, ClosePane), + Command::localized(L10nKey::CmdCloseOtherTabs, CloseOtherTabs), + Command::localized(L10nKey::CmdCloseTabsToTheRight, CloseTabsToTheRight), + Command::localized(L10nKey::CmdReopenClosedTab, ReopenClosedTab), ]; let workspaces = [ - Command::new(t(L10nKey::CmdNewWorkspace), NewWorkspace), - Command::new(t(L10nKey::CmdSwitchWorkspace), OpenWorkspacePicker), - Command::new(t(L10nKey::CmdRenameWorkspace), RenameWorkspace), - Command::new(t(L10nKey::CmdStopWorkspace), StopWorkspace) + Command::localized(L10nKey::CmdNewWorkspace, NewWorkspace), + Command::localized(L10nKey::CmdSwitchWorkspace, OpenWorkspacePicker), + Command::localized(L10nKey::CmdRenameWorkspace, RenameWorkspace), + Command::localized(L10nKey::CmdStopWorkspace, StopWorkspace) .with_subtitle(t(L10nKey::CmdStopWorkspaceSubtitle)), - Command::new(t(L10nKey::CmdDeleteWorkspace), DeleteWorkspace) + Command::localized(L10nKey::CmdDeleteWorkspace, DeleteWorkspace) .with_subtitle(t(L10nKey::CmdDeleteWorkspaceSubtitle)), ]; let view = [ - Command::new( + Command::localized( if sidebar_hidden { - t(L10nKey::CmdShowLeftSidebar) + L10nKey::CmdShowLeftSidebar } else { - t(L10nKey::CmdHideLeftSidebar) + L10nKey::CmdHideLeftSidebar }, ToggleLeftPanel, ), - Command::new( + Command::localized( if right_panel_open { - t(L10nKey::CmdHideRightPanel) + L10nKey::CmdHideRightPanel } else { - t(L10nKey::CmdShowRightPanel) + L10nKey::CmdShowRightPanel }, ToggleRightPanel, ), - Command::new(t(L10nKey::CmdShowCodePanel), ToggleCodePanel), - Command::new( + Command::localized(L10nKey::CmdShowCodePanel, ToggleCodePanel), + Command::localized( if tab_bar_left { - t(L10nKey::CmdTabBarMoveToTop) + L10nKey::CmdTabBarMoveToTop } else { - t(L10nKey::CmdTabBarMoveToLeftSidebar) + L10nKey::CmdTabBarMoveToLeftSidebar }, ToggleTabSidebar, ), - Command::new( - t(L10nKey::CmdRightPanelInfo), + Command::localized( + L10nKey::CmdRightPanelInfo, ShowRightPanel(RightPanelTab::Info), ), - Command::new( - t(L10nKey::CmdRightPanelChanges), + Command::localized( + L10nKey::CmdRightPanelChanges, ShowRightPanel(RightPanelTab::Changes), ), - Command::new( - t(L10nKey::CmdRightPanelFiles), + Command::localized( + L10nKey::CmdRightPanelFiles, ShowRightPanel(RightPanelTab::Files), ), - Command::new(t(L10nKey::CmdChangeTheme), OpenThemePicker), - Command::new(t(L10nKey::CmdResetFontSize), ResetFontSize), - Command::new(t(L10nKey::CmdEnterFullScreen), ToggleFullscreen), + Command::localized(L10nKey::CmdChangeTheme, OpenThemePicker), + Command::localized(L10nKey::CmdResetFontSize, ResetFontSize), + Command::localized(L10nKey::CmdEnterFullScreen, ToggleFullscreen), ]; let terminal = [ - Command::new(t(L10nKey::CmdClearScrollback), ClearTerminal), - Command::new(t(L10nKey::CmdFindInTerminal), FindInTerminal), - Command::new(t(L10nKey::CmdFindNext), FindNext), - Command::new(t(L10nKey::CmdFindPrevious), FindPrevious), - Command::new(t(L10nKey::CmdCopy), CopyText), - Command::new(t(L10nKey::CmdCut), CutText), - Command::new(t(L10nKey::CmdPaste), PasteText), - Command::new(t(L10nKey::CmdSelectAll), SelectAllText), + Command::localized(L10nKey::CmdClearScrollback, ClearTerminal), + Command::localized(L10nKey::CmdFindInTerminal, FindInTerminal), + Command::localized(L10nKey::CmdFindNext, FindNext), + Command::localized(L10nKey::CmdFindPrevious, FindPrevious), + Command::localized(L10nKey::CmdCopy, CopyText), + Command::localized(L10nKey::CmdCut, CutText), + Command::localized(L10nKey::CmdPaste, PasteText), + Command::localized(L10nKey::CmdSelectAll, SelectAllText), ]; let ssh = [ - Command::new(t(L10nKey::CmdSshAddConnection), OpenSshConnectInput), - Command::new(t(L10nKey::CmdSshManageProfiles), OpenSshProfiles), - Command::new(t(L10nKey::CmdSshReconnect), RestartSshSession), - Command::new(t(L10nKey::CmdSshRemoteFiles), ToggleSftp), - Command::new(t(L10nKey::CmdSshPortForwarding), ShowSshForwards), + Command::localized(L10nKey::CmdSshAddConnection, OpenSshConnectInput), + Command::localized(L10nKey::CmdSshManageProfiles, OpenSshProfiles), + Command::localized(L10nKey::CmdSshReconnect, RestartSshSession), + Command::localized(L10nKey::CmdSshRemoteFiles, ToggleSftp), + Command::localized(L10nKey::CmdSshPortForwarding, ShowSshForwards), ]; let agents = [ - Command::new(t(L10nKey::CmdAgentSendSelection), SendSelectionToAgent) + Command::localized(L10nKey::CmdAgentSendSelection, SendSelectionToAgent) .with_subtitle(t(L10nKey::CmdAgentSendSelectionSubtitle)), - Command::new(t(L10nKey::CmdAgentSendGitDiffForReview), SendGitDiffToAgent) + Command::localized(L10nKey::CmdAgentSendGitDiffForReview, SendGitDiffToAgent) .with_subtitle(t(L10nKey::CmdAgentSendGitDiffSubtitle)), ]; let application = [ - Command::new(t(L10nKey::CmdSettings), OpenSettings), - Command::new(t(L10nKey::CmdKeyboardShortcuts), ShowKeyboardShortcuts), - Command::new(t(L10nKey::CmdAboutTty7), About), - Command::new(t(L10nKey::CmdCheckForUpdates), CheckForUpdates), - Command::new(t(L10nKey::CmdDocumentation), OpenDocumentation), - Command::new(t(L10nKey::CmdJoinDiscord), OpenDiscord), - Command::new(t(L10nKey::CmdReportIssue), ReportIssue), - Command::new(t(L10nKey::CmdRestartServer), RestartDaemon) + Command::localized(L10nKey::CmdSettings, OpenSettings), + Command::localized(L10nKey::CmdKeyboardShortcuts, ShowKeyboardShortcuts), + Command::localized(L10nKey::CmdAboutTty7, About), + Command::localized(L10nKey::CmdCheckForUpdates, CheckForUpdates), + Command::localized(L10nKey::CmdDocumentation, OpenDocumentation), + Command::localized(L10nKey::CmdJoinDiscord, OpenDiscord), + Command::localized(L10nKey::CmdReportIssue, ReportIssue), + Command::localized(L10nKey::CmdRestartServer, RestartDaemon) .with_subtitle(t(L10nKey::CmdRestartServerSubtitle)), - Command::new(t(L10nKey::CmdQuitTty7), Quit) + Command::localized(L10nKey::CmdQuitTty7, Quit) .with_subtitle(t(L10nKey::CmdQuitTty7Subtitle)), ]; @@ -505,6 +520,15 @@ impl Command { .collect() } + /// Row of the preset already in use, so the theme picker can open on it + /// instead of previewing something else the moment it is opened. + pub fn active_theme_index(cx: &App) -> Option { + let active = crate::ui::theme::effective_preset_id(cx); + crate::ui::presets::all(cx) + .iter() + .position(|p| p.id == active) + } + fn ssh_connect_command(input: &str) -> Command { let trimmed = input.trim(); let title = if trimmed.is_empty() { @@ -573,6 +597,11 @@ pub fn fuzzy_score(query: &str, text: &str) -> Option { Some(score) } +/// How far behind the visible label an alias hit lands. An alias *is* the +/// command's own name, only in another language, so the gap is small — but two +/// commands that both match must still be ordered by the text on screen. +const ALIAS_PENALTY: i32 = 10; + fn command_score(query: &str, cmd: &Command) -> Option { let title = fuzzy_score(query, &cmd.title); let subtitle = cmd @@ -580,10 +609,16 @@ fn command_score(query: &str, cmd: &Command) -> Option { .as_deref() .and_then(|s| fuzzy_score(query, s)) .map(|s| s / 2 - 25); - match (title, subtitle) { - (Some(a), Some(b)) => Some(a.max(b)), - (a, b) => a.or(b), - } + // Aliases only ever decide whether a row is in the list and where it sits. + // The row keeps rendering `title` untouched, so a hit on wording the user + // cannot see can never end up underlined against the wrong characters. + let alias = cmd + .aliases + .iter() + .filter_map(|a| fuzzy_score(query, a)) + .max() + .map(|s| s - ALIAS_PENALTY); + [title, subtitle, alias].into_iter().flatten().max() } /// A bounded nudge, not a re-ranking. Two commands that match the query about @@ -756,7 +791,7 @@ impl ListDelegate for PaletteDelegate { fn perform_search( &mut self, query: &str, - _window: &mut Window, + window: &mut Window, cx: &mut Context>, ) -> Task<()> { if let Some(PaletteInput::SshConnect) = self.input { @@ -805,7 +840,12 @@ impl ListDelegate for PaletteDelegate { commands, }]; } - self.selected = self.first_row(); + // Through `set_selected_index`, not by hand: the row index may not have + // moved, but the command under it has, and the theme picker previews + // the command — not the index. + self.selected = None; + let first = self.first_row(); + self.set_selected_index(first, window, cx); Task::ready(()) } @@ -923,7 +963,14 @@ impl ListDelegate for PaletteDelegate { _window: &mut Window, cx: &mut Context>, ) { + let moved = self.selected != ix; self.selected = ix; + // The list only emits `Select` for the arrow keys; a query that re-arms + // the first row moves the highlight silently. The theme picker previews + // whatever is highlighted, so it has to hear about both. + if moved && let Some(ix) = ix { + cx.emit(ListEvent::Select(ix)); + } cx.notify(); } } @@ -931,6 +978,11 @@ impl ListDelegate for PaletteDelegate { pub enum PaletteEvent { Confirm(CommandKind), Dismiss, + /// Show the theme at this preset index without persisting it: the theme + /// picker previews the highlighted row while it stays open. + PreviewTheme(usize), + /// Put back the theme that was live before the preview started. + CancelThemePreview, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -944,6 +996,9 @@ pub struct PaletteView { list: Entity>, root: Vec, menu: PaletteMenu, + /// Preset index the theme picker is currently previewing, so the same + /// theme is not re-applied on every redundant selection event. + previewing: Option, _sub: Subscription, } @@ -955,6 +1010,7 @@ impl PaletteView { list, root: commands, menu: PaletteMenu::Root, + previewing: None, _sub, } } @@ -995,8 +1051,20 @@ impl PaletteView { list } - fn show(&mut self, commands: Vec, window: &mut Window, cx: &mut Context) { + fn show( + &mut self, + commands: Vec, + selected: Option, + window: &mut Window, + cx: &mut Context, + ) { let list = Self::build_list(commands, window, cx); + if let Some(row) = selected { + list.update(cx, |state, cx| { + state.set_selected_index(Some(IndexPath::new(row)), window, cx); + state.scroll_to_selected_item(window, cx); + }); + } self._sub = cx.subscribe_in(&list, window, Self::on_list_event); self.list = list; cx.notify(); @@ -1039,7 +1107,11 @@ impl PaletteView { Some(CommandKind::OpenThemePicker) => { self.menu = PaletteMenu::Theme; let themes = Command::theme_commands(cx); - self.show(themes, window, cx); + // Open on the theme already in use: the picker previews + // the highlighted row, and merely opening it must not + // change what the window looks like. + self.previewing = Command::active_theme_index(cx); + self.show(themes, self.previewing, window, cx); } Some(CommandKind::OpenSshConnectInput) => { self.menu = PaletteMenu::SshConnect; @@ -1055,6 +1127,12 @@ impl PaletteView { } ListEvent::Cancel => { if self.menu != PaletteMenu::Root { + if self.menu == PaletteMenu::Theme { + // Backing out of the picker is not a choice: whatever + // was previewed goes back to what it was. + self.previewing = None; + cx.emit(PaletteEvent::CancelThemePreview); + } self.menu = PaletteMenu::Root; let root = self.root.clone(); let list = Self::build_root_list(root, window, cx); @@ -1065,7 +1143,15 @@ impl PaletteView { cx.emit(PaletteEvent::Dismiss); } } - ListEvent::Select(_) => {} + ListEvent::Select(ix) => { + if self.menu == PaletteMenu::Theme + && let Some(CommandKind::SetTheme(i)) = list.read(cx).delegate().command_at(*ix) + && self.previewing != Some(i) + { + self.previewing = Some(i); + cx.emit(PaletteEvent::PreviewTheme(i)); + } + } } } } @@ -1259,6 +1345,35 @@ mod tests { assert!(title_hit > subtitle_hit); } + #[test] + fn a_translated_command_is_still_found_by_its_english_name() { + crate::ui::i18n::set_locale("zh-CN"); + let cmd = Command::localized(L10nKey::CmdChangeTheme, CommandKind::OpenThemePicker); + assert!( + !cmd.title.to_lowercase().contains("theme"), + "the row shows the Chinese label: {:?}", + cmd.title + ); + assert!( + command_score("theme", &cmd).is_some(), + "an English query must find the Chinese-labelled row" + ); + assert!( + command_score("テーマ", &cmd).is_some(), + "so must the Japanese one" + ); + // The displayed label still works, and still wins: the same query + // against the locale that shows it scores higher. + let zh = command_score("更改主题", &cmd).expect("the shown label matches"); + crate::ui::i18n::set_locale("en"); + let en = Command::localized(L10nKey::CmdChangeTheme, CommandKind::OpenThemePicker); + assert!(zh > 0 && en.title.contains("Theme")); + assert!( + command_score("theme", &en).unwrap() > command_score("theme", &cmd).unwrap(), + "a hit on the visible label must outrank the same hit on an alias" + ); + } + #[test] fn stable_ids_are_unique() { let mut seen = std::collections::HashSet::new(); @@ -1285,6 +1400,30 @@ mod tests { } } + /// The picker previews whatever row is highlighted, so it has to open on + /// the row of the theme already in use — the one carrying the check mark. + #[gpui::test] + fn the_theme_picker_opens_on_the_theme_in_use(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + let themes = crate::ui::presets::all(cx); + let last = themes.last().expect("built-in themes").id.clone(); + cx.set_global(Config(crate::core::config::CoreConfig { + theme_follow_system: false, + theme_preset: last, + ..Default::default() + })); + + let ix = Command::active_theme_index(cx).expect("the live preset is listed"); + assert_eq!(ix, themes.len() - 1); + let rows = Command::theme_commands(cx); + assert!( + rows[ix].title.ends_with('✓'), + "row {ix} ({:?}) should be the checked one", + rows[ix].title + ); + }); + } + #[test] fn dynamic_commands_have_no_id() { assert!(CommandKind::ActivateTab(2).id().is_none()); diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 7ba7cf42..f294375c 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -22,8 +22,8 @@ use std::sync::Arc; use uuid::Uuid; use crate::core::config::{ - BellMode, Config, CursorStyle, NewTabPosition, NotifyMode, TabBarPosition, UpdateChannel, - WindowBackdrop, + BellMode, Config, CursorStyle, NewTabPosition, NotifyMode, TabBarPosition, + UI_FONT_SIZE_DEFAULT, UpdateChannel, WindowBackdrop, }; use crate::core::keychain::CredentialRef; use crate::core::ssh_profile::{ @@ -39,29 +39,192 @@ use crate::ui::presets; use crate::ui::rounding; use crate::ui::rounding::RoundedCorners as _; -/// The settings nav, the SSH host list, and the padding each page sets — the -/// chrome a row has to share the window with. +/// The settings nav, the SSH host list, the theme panel, and the padding each +/// page sets — the chrome a row has to share the window with. const NAV_W: f32 = 220.; const SSH_LIST_W: f32 = 280.; +const THEME_PANEL_W: f32 = 300.; const SSH_DETAIL_PAD: f32 = 64.; const PAGE_PAD: f32 = 80.; +/// The narrowest window these numbers have to hold for. +/// +/// `ui::windows::MIN_SIZE` declares 720, but the settings window in the report +/// this file was fixed for measured 641pt: `window_min_size` governs dragging, +/// not the bounds a window opens with, so a remembered bound walked straight +/// under it. `ui::windows::at_least_min_size` now closes that path, and this +/// stays below it deliberately — a declared minimum is a claim about the code, +/// and this file would rather be laid out for the window that turns up. +const NARROWEST_WINDOW: f32 = 640.; + +/// The narrowest each list is still itself: a nav item that still shows a label +/// beside its icon (40 of icon, gap and padding, then the longest label), a +/// host row that still shows a name, a theme card that is still a recognisable +/// picture of a theme. +/// +/// The nav floor is sized for the longest nav label in *any* locale, not the +/// one the developer happens to be reading. `SidebarMenuItem` clips its label +/// rather than eliding it, so a floor that fits English cuts a glyph in half in +/// Chinese and Japanese: at 140 the zh-CN "窗口与标签页" lost the right half of +/// its last character. The widest is ja-JP "ウィンドウとタブ" — 8 full-width +/// kana beside the icon, which is 36 more than the 6-glyph Chinese label needs. +const NAV_W_MIN: f32 = 176.; +const SSH_LIST_W_MIN: f32 = 180.; +const THEME_PANEL_W_MIN: f32 = 240.; + +/// The reading column every scrolled page caps itself at. Wider than this and +/// a description stops being a paragraph and becomes a line to scan across. +const READING_COLUMN: f32 = 640.; + +/// What the page gets before any list does, and the floor it may be pushed to +/// when even that cannot be had — the numbers this file did not have. +/// +/// Every list beside the page was a fixed width that never gave anything back, +/// so the page absorbed the entire shortfall. On a half-width window with the +/// theme panel open that ran all the way down: a Chinese description came out +/// one character per line, twenty-five lines tall, and the theme cards under it +/// were slivers clipped by the window edge. +/// +/// `CONTENT_W` holds a stacked row's widest control — the 260px text fields — +/// with a description beside it that still reads as a paragraph. `CONTENT_MIN_W` +/// is not chosen at all: it is what the narrowest window in the wild leaves the +/// SSH page, the one that spends a second list, once both lists are standing on +/// their own floors. A control wider than it has to be able to shrink, which is +/// what `max_w_full` on the wrappers below is for. +const CONTENT_W: f32 = 420.; +const CONTENT_MIN_W: f32 = NARROWEST_WINDOW - NAV_W_MIN - SSH_LIST_W_MIN - SSH_DETAIL_PAD; + +/// What the settings page is made of at a given window width. +#[derive(Clone, Copy, PartialEq, Debug)] +struct SettingsColumns { + nav: f32, + /// Zero off the SSH page. + ssh_list: f32, + /// The width to *draw* the theme panel at, whether it is taking a column or + /// covering one — zero only when it is closed. + theme_panel: f32, + /// The panel no longer fits beside the page, so it lays itself over the + /// page instead of taking width from it. The panel is a temporary layer + /// over one choice; the page underneath is what the window is for. + panel_overlays: bool, +} + +/// Hand every list its full width, then take the shortfall back from all of +/// them at once — each in proportion to what it has to spare — until the page +/// between them reaches `CONTENT_W`. Once every list is standing on its own +/// floor the page takes whatever is left, which from `NARROWEST_WINDOW` up is +/// never less than `CONTENT_MIN_W`. The theme panel leaves the row altogether +/// rather than let it come to that. +/// +/// Every width here is one this row can actually have, so the columns always +/// add up to the window. That is deliberate: the fix for a page squeezed to +/// nothing is not a `min_w` the row cannot honour — a floor a flex row cannot +/// meet does not push back, it overflows, and overflow here means content +/// painted off the edge of the window, which is the other half of this bug. +fn settings_columns( + section: SettingsSection, + theme_panel_open: bool, + viewport: f32, +) -> SettingsColumns { + let ssh = matches!(section, SettingsSection::Ssh); + // The panel belongs to Appearance; a stale open flag on any other page is + // not a column, the same way `render_settings` does not draw one. + let theme_panel_open = theme_panel_open && matches!(section, SettingsSection::Appearance); + let pad = if ssh { SSH_DETAIL_PAD } else { PAGE_PAD }; + let page = CONTENT_W + pad; + + // Even with the nav and the panel both at their floors there has to be a + // readable page left between them. Below that width the panel stops being a + // column — this is the one place the *floor* is the test, because the panel + // leaving the row is what buys the page its preferred width back. + let panel_overlays = + theme_panel_open && viewport - NAV_W_MIN - THEME_PANEL_W_MIN - PAGE_PAD < CONTENT_MIN_W; + let beside = theme_panel_open && !panel_overlays; + + let mut nav = NAV_W; + let mut ssh_list = only_when(ssh, SSH_LIST_W); + let mut theme_panel = only_when(beside, THEME_PANEL_W); + let (nav_slack, list_slack, panel_slack) = ( + NAV_W - NAV_W_MIN, + only_when(ssh, SSH_LIST_W - SSH_LIST_W_MIN), + only_when(beside, THEME_PANEL_W - THEME_PANEL_W_MIN), + ); + let slack = nav_slack + list_slack + panel_slack; + let short = (nav + ssh_list + theme_panel + page - viewport).max(0.); + if short > 0. && slack > 0. { + let give = (short / slack).min(1.); + nav -= nav_slack * give; + ssh_list -= list_slack * give; + theme_panel -= panel_slack * give; + } + if panel_overlays { + // Covering the page, not replacing it: leave a strip of the page in + // view so the panel reads as something laid on top and dismissible. + theme_panel = THEME_PANEL_W + .min(viewport - nav - CONTENT_MIN_W / 2.) + .max(THEME_PANEL_W_MIN); + } + SettingsColumns { + nav: nav.round(), + ssh_list: ssh_list.round(), + theme_panel: theme_panel.round(), + panel_overlays, + } +} + /// What a row on this page really has to lay out in. /// /// The nav is always in front of it; the SSH page puts its host list there too, -/// and only the scrolled pages cap the reading column at 640. -fn settings_row_width(section: SettingsSection, viewport: f32) -> f32 { +/// the theme panel takes another slice of Appearance for as long as it is open +/// *and* still fits beside it, and only the scrolled pages cap the reading +/// column. +fn settings_row_width( + section: SettingsSection, + theme_panel_open: bool, + viewport: f32, + ui_scale: f32, +) -> f32 { + let cols = settings_columns(section, theme_panel_open, viewport); + let panel = only_when(!cols.panel_overlays, cols.theme_panel); match section { - SettingsSection::Ssh => (viewport - NAV_W - SSH_LIST_W - SSH_DETAIL_PAD).max(0.), - _ => (viewport - NAV_W - PAGE_PAD).clamp(0., 640.), + SettingsSection::Ssh => (viewport - cols.nav - cols.ssh_list - SSH_DETAIL_PAD).max(0.), + _ => (viewport - cols.nav - panel - PAGE_PAD).clamp(0., READING_COLUMN * ui_scale), } } +fn only_when(on: bool, w: f32) -> f32 { + if on { w } else { 0. } +} + +/// How much wider every piece of text on this page is than the px thresholds +/// below assume. +/// +/// Those thresholds are widths a *label* needs, measured at the default +/// interface font. The interface has a font size of its own and it goes up to +/// 24 — half as wide again — while a slider or a text field beside that label +/// stays the px width it was built at. Without this the row that has to stack +/// first is the one that never does. +fn ui_scale(cx: &App) -> f32 { + cx.global::().ui_font_size / UI_FONT_SIZE_DEFAULT +} + /// Width a settings row needs before its label and its control fit side by /// side: a 260px control, the `gap_8` between them, and enough left for a /// description to read as prose rather than as a column of words. const STACK_ROW_BELOW: f32 = 500.; +/// Width a port-forwarding rule needs before its kind switch, its two host:port +/// pairs, its description and its remove button all fit on one line: about 580 +/// with every field at its floor, rounded up. Past this the rule takes two +/// lines instead of running off the page. +const SPLIT_FORWARD_ROW_BELOW: f32 = 620.; + +/// And the width below which even `bind → target` is more than one line holds: +/// two host fields at their narrow floor, two ports and the arrow come to about +/// 310, which is more than `CONTENT_MIN_W`. The SSH page reaches this on the +/// window the report came from, so the two ends take a line each. +const STACK_FORWARD_ENDS_BELOW: f32 = 340.; + fn settings_row_id(label: &str, _desc: &str) -> SharedString { SharedString::from(format!("settings-row-{label}")) } @@ -873,9 +1036,15 @@ impl Tty7App { let query = search.read(cx).value().trim().to_lowercase(); let show_theme_panel = theme_panel_open && section == SettingsSection::Appearance; + let viewport_w = window.viewport_size().width.as_f32(); + let ui_scale = ui_scale(cx); + self.settings_viewport_w.set(viewport_w); + let cols = settings_columns(section, show_theme_panel, viewport_w); self.settings_row_width.set(settings_row_width( section, - window.viewport_size().width.as_f32(), + show_theme_panel, + viewport_w, + ui_scale, )); self.settings_hit_anchored.set(false); @@ -971,7 +1140,7 @@ impl Tty7App { let sidebar = Sidebar::new("settings-sidebar") .collapsible(SidebarCollapsible::None) - .w(px(220.)) + .w(px(cols.nav)) .header( v_flex() .w_full() @@ -1040,6 +1209,12 @@ impl Tty7App { // No fill of its own: the root already paints the opaque surface and // the background image behind it, and repainting here would hide the // image again in the one pane that fills most of the panel. + // + // `min_w_0` and not a `CONTENT_MIN_W` floor: `settings_columns` sized + // the chrome so this pane clears it, and a floor a flex row cannot + // honour does not push the nav back — it overflows, and overflow here + // means content painted off the edge of the window, which is the other + // half of the bug this file is fixing. let content_pane = if section == SettingsSection::Ssh { v_flex() .id("settings-content") @@ -1067,7 +1242,7 @@ impl Tty7App { div().w_full().px_10().py_8().child( div() .w_full() - .max_w(px(640.)) + .max_w(px(READING_COLUMN * ui_scale)) .children(no_match_note) .child(content), ), @@ -1129,7 +1304,25 @@ impl Tty7App { ) .on_double_click(|_, window, _| window.titlebar_double_click()), ) - .when(show_theme_panel, |r| r.child(self.render_theme_panel(cx))) + // Beside the page while there is room for both, over it when there + // is not. As a column it was taking its 300px from the page and + // from nothing else, which is how a half-width window ended up + // rendering a description one character wide. + .when(show_theme_panel && !cols.panel_overlays, |r| { + r.child(self.render_theme_panel(cx)) + }) + .when(show_theme_panel && cols.panel_overlays, |r| { + r.child( + div() + .absolute() + .top_0() + .right_0() + .bottom_0() + .occlude() + .shadow_lg() + .child(self.render_theme_panel(cx)), + ) + }) .when(!show_theme_panel, |r| { r.child( div() @@ -1161,6 +1354,27 @@ impl Tty7App { root } + /// The column widths this render settled on. `settings_columns` is pure and + /// cheap, so the two pages that draw chrome of their own work them out + /// again rather than have the answer threaded through every builder. + fn settings_columns_now(&self) -> SettingsColumns { + let (section, panel_open) = match self.active_settings() { + Some(s) => ( + s.section, + s.theme_panel_open && s.section == SettingsSection::Appearance, + ), + None => (SettingsSection::Appearance, false), + }; + settings_columns(section, panel_open, self.settings_viewport_w.get()) + } + + /// Whether the row measured this render came out narrower than a threshold + /// quoted at the default interface font — the only way those px thresholds + /// mean anything to a reader who scaled the interface up. + fn settings_row_under(&self, at_default_font: f32, cx: &App) -> bool { + self.settings_row_width.get() < at_default_font * ui_scale(cx) + } + fn header_text(&self, title: &str, cx: &Context) -> Div { div() .text_base() @@ -1270,7 +1484,7 @@ impl Tty7App { // where both still fit, put the control on its own line instead. // Measured, not `flex_wrap`: wrapping made the label column size to its // description, which then ran out past the row on every wide page. - let stacked = self.settings_row_width.get() < STACK_ROW_BELOW; + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); let labels = v_flex() .gap_0p5() .min_w_0() @@ -1309,7 +1523,16 @@ impl Tty7App { .hover(|h| h.bg(gpui::rgb(cx.global::().window.hover))) .on_hover(cx.listener(|_this, _hovered, _window, cx| cx.notify())) .child(labels) - .child(h_flex().flex_shrink_0().child(control)) + // Stacked, the control column takes the row: that is what gives a + // `max_w_full` control a definite width to shrink against, and on + // the SSH page the widest of them is 260 in a column that can be + // `CONTENT_MIN_W`. + .child( + h_flex() + .when(stacked, |c| c.w_full()) + .when(!stacked, |c| c.flex_shrink_0()) + .child(control), + ) } pub(crate) fn segmented( @@ -1643,6 +1866,7 @@ impl Tty7App { .items_center() .gap_3() .w(px(240.)) + .max_w_full() .child(div().flex_1().child(Slider::new(&slider))) .child( div() @@ -1951,7 +2175,7 @@ impl Tty7App { .child( v_flex() .flex_shrink_0() - .w(px(280.)) + .w(px(self.settings_columns_now().ssh_list)) .h_full() .border_r_1() .border_color(border) @@ -2455,7 +2679,12 @@ impl Tty7App { h_flex() .mt_3() .gap_2() - .child(div().w(px(320.)).child(Input::new(&input).small())) + .child( + div() + .flex_1() + .max_w(px(320.)) + .child(Input::new(&input).small()), + ) .child( Button::new("ssh-quick-connect") .label(t(L10nKey::Connect)) @@ -3172,6 +3401,7 @@ impl Tty7App { t(L10nKey::SettingsNameDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.name).small()) .into_any_element(), cx, @@ -3183,8 +3413,19 @@ impl Tty7App { t(L10nKey::SettingsHostDesc), h_flex() .gap_2() - .child(div().w(px(172.)).child(Input::new(&form.host).small())) - .child(div().w(px(80.)).child(Input::new(&form.port).small())) + .max_w_full() + .child( + div() + .w(px(172.)) + .min_w_0() + .child(Input::new(&form.host).small()), + ) + .child( + div() + .w(px(80.)) + .flex_shrink_0() + .child(Input::new(&form.port).small()), + ) .into_any_element(), cx, ), @@ -3195,6 +3436,7 @@ impl Tty7App { t(L10nKey::SettingsUserDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.user).small()) .into_any_element(), cx, @@ -3314,6 +3556,7 @@ impl Tty7App { t(L10nKey::SettingsJumpHostDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.jump).small()) .into_any_element(), cx, @@ -3373,6 +3616,7 @@ impl Tty7App { ) .child( h_flex() + .flex_wrap() .gap_3() .pt_1() .text_xs() @@ -3400,6 +3644,14 @@ impl Tty7App { }; let incomplete = row.collect(cx).is_none() && !row.is_blank(cx); + // Below `SPLIT_FORWARD_ROW_BELOW` the five controls stop fitting on one + // line. The kind switch, the description and the remove button keep the + // first line; the mapping the rule is actually about takes the second, + // where two host fields, two ports and an arrow are what has to fit + // inside `CONTENT_MIN_W` — hence the lower floor on the host field. + let split = self.settings_row_under(SPLIT_FORWARD_ROW_BELOW, cx); + let stack_ends = self.settings_row_under(STACK_FORWARD_ENDS_BELOW, cx); + let host_min = if split { 80. } else { 104. }; let endpoint = |host: &Entity, port: &Entity| { h_flex() .gap_1() @@ -3411,74 +3663,89 @@ impl Tty7App { .child( div() .flex_1() - .min_w(px(104.)) + .min_w(px(host_min)) .child(Input::new(host).xsmall()), ) .child(div().text_xs().text_color(muted).child(":")) .child(div().w(px(58.)).child(Input::new(port).xsmall())) }; + let mapping = |line: Div| { + line.child( + div() + .flex_1() + .when(stack_ends, |end| end.w_full()) + .child(endpoint(&row.bind_host, &row.bind_port)), + ) + .child(div().flex_shrink_0().text_xs().text_color(muted).child("→")) + .child( + div() + .flex_1() + .opacity(if needs_target { + 1.0 + } else { + crate::ui::forwards::NO_TARGET_FADE + }) + .when(stack_ends, |end| end.w_full()) + .child(endpoint(&row.target_host, &row.target_port)), + ) + }; + + let kind_switch = div().flex_shrink_0().child(self.segmented( + format!("ssh-fwd-kind-{idx}"), + &["L", "R", "D"], + kind_idx, + cx, + move |this, ix, _w, cx| { + let kind = match ix { + 1 => ForwardKind::Remote, + 2 => ForwardKind::Dynamic, + _ => ForwardKind::Local, + }; + if let Some(f) = this.ssh_form_mut() + && let Some(r) = f.forwards.get_mut(idx) + { + r.kind = kind; + cx.notify(); + } + }, + )); + let description = div() + .flex_1() + .min_w(px(80.)) + .child(Input::new(&row.description).xsmall()); + let remove = crate::ui::tab_strip::hit_target( + Button::new(("ssh-fwd-remove", idx)) + .icon(Icon::new(IconName::Close)) + .ghost() + .xsmall(), + ) + .tooltip(t(L10nKey::SettingsRemoveRule)) + .on_click(cx.listener(move |this, _, _w, cx| this.remove_forward_rule(idx, cx))); + + let rule = match split { + true => v_flex() + .gap_1() + .child( + h_flex() + .gap_2() + .items_center() + .child(kind_switch) + .child(description) + .child(remove), + ) + .child(match stack_ends { + true => mapping(v_flex().gap_1().items_start()), + false => mapping(h_flex().gap_2().items_center()), + }), + false => mapping(h_flex().gap_2().items_center().child(kind_switch)) + .child(description) + .child(remove), + }; v_flex() .gap_0p5() .py_1() - .child( - h_flex() - .gap_2() - .items_center() - .child(self.segmented( - format!("ssh-fwd-kind-{idx}"), - &["L", "R", "D"], - kind_idx, - cx, - move |this, ix, _w, cx| { - let kind = match ix { - 1 => ForwardKind::Remote, - 2 => ForwardKind::Dynamic, - _ => ForwardKind::Local, - }; - if let Some(f) = this.ssh_form_mut() - && let Some(r) = f.forwards.get_mut(idx) - { - r.kind = kind; - cx.notify(); - } - }, - )) - .child( - div() - .flex_1() - .child(endpoint(&row.bind_host, &row.bind_port)), - ) - .child(div().flex_shrink_0().text_xs().text_color(muted).child("→")) - .child( - div() - .flex_1() - .opacity(if needs_target { - 1.0 - } else { - crate::ui::forwards::NO_TARGET_FADE - }) - .child(endpoint(&row.target_host, &row.target_port)), - ) - .child( - div() - .flex_1() - .min_w(px(80.)) - .child(Input::new(&row.description).xsmall()), - ) - .child( - crate::ui::tab_strip::hit_target( - Button::new(("ssh-fwd-remove", idx)) - .icon(Icon::new(IconName::Close)) - .ghost() - .xsmall(), - ) - .tooltip(t(L10nKey::SettingsRemoveRule)) - .on_click( - cx.listener(move |this, _, _w, cx| this.remove_forward_rule(idx, cx)), - ), - ), - ) + .child(rule) .when(incomplete, |col| { col.child(div().text_xs().text_color(danger).child(if needs_target { t(L10nKey::SettingsFwdNeedsBoth) @@ -3550,6 +3817,7 @@ impl Tty7App { desc.to_string(), div() .w(px(260.)) + .max_w_full() .child(Input::new(input).small()) .into_any_element(), cx, @@ -3874,10 +4142,12 @@ impl Tty7App { }); let program_control = div() .w(px(260.)) + .max_w_full() .child(Input::new(&program_input).small().suffix(program_picker)) .into_any_element(); let args_control = div() .w(px(260.)) + .max_w_full() .child(Input::new(&args_input).small()) .into_any_element(); @@ -3908,6 +4178,7 @@ impl Tty7App { let wd_path_control = if wd_strategy == WdStrategy::Custom { div() .w(px(260.)) + .max_w_full() .child(Input::new(&wd_path_input).small()) .into_any_element() } else { @@ -3997,6 +4268,7 @@ impl Tty7App { .into_any_element(); let link_file_command_control = div() .w(px(300.)) + .max_w_full() .child(Input::new(&link_file_command_input).small()) .into_any_element(); let scrollback_radio = self.segmented( @@ -4059,6 +4331,7 @@ impl Tty7App { .items_center() .gap_3() .w(px(240.)) + .max_w_full() .child(div().flex_1().child(Slider::new(&scroll_slider))) .child( div() @@ -4264,6 +4537,7 @@ impl Tty7App { ), None => (AgentHooksView::Loading, None, HostId::LOCAL), }; + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); let mut page = v_flex().child(self.section_intro( t(L10nKey::SettingsAgentsIntro), t(L10nKey::SettingsAgentsIntroDesc), @@ -4308,9 +4582,13 @@ impl Tty7App { .filter(|(for_agent, _)| *for_agent == agent) .map(|(_, text)| text.clone()); + // Right-aligned beside its label, left-aligned under it — + // `settings_row` gives the control column the whole row + // once it stacks, and buttons flush to the far edge of a + // row whose label starts at the near one read as unrelated. let control = v_flex() .gap_2() - .items_end() + .when(!stacked, |c| c.items_end()) .child( h_flex() .gap_2() @@ -4344,8 +4622,9 @@ impl Tty7App { col.child( div() .max_w_80() + .max_w_full() .text_xs() - .text_right() + .when(!stacked, |note| note.text_right()) .text_color(muted_fg) .child(text), ) @@ -4797,6 +5076,9 @@ impl Tty7App { let open = self .active_settings() .is_some_and(|s| s.theme_panel_open && s.theme_panel_slot == slot); + // The same width a row stacks at: the card is a row too, just one whose + // control happens to be a whole preview. + let narrow = self.settings_row_under(STACK_ROW_BELOW, cx); div() .id(card_id) @@ -4822,9 +5104,17 @@ impl Tty7App { }) .bg(surface) .hover(|h| h.bg(hover_bg)) - .child(div().w(px(150.)).flex_shrink_0().child(preview)) + // The preview is the first thing to go: it is a picture of a + // choice the two lines beside it already name, and at the + // width where it stops fitting it was pushing the "change + // theme" affordance off the card. + .when(!narrow, |card| { + card.child(div().w(px(150.)).flex_shrink_0().child(preview)) + }) .child( v_flex() + .flex_1() + .min_w_0() .gap_0p5() .child(div().text_xs().text_color(muted_fg).child(caption)) .child( @@ -4836,9 +5126,9 @@ impl Tty7App { ) .child(swatches), ) - .child(div().flex_1()) .child( h_flex() + .flex_shrink_0() .items_center() .gap_1() .text_sm() @@ -4922,7 +5212,7 @@ impl Tty7App { }); let search_box = div().px_4().pb_3().child( - div().w(px(268.)).child( + div().w_full().child( Input::new(&search).small().prefix( Icon::empty() .path("stock/icons/search.svg") @@ -4986,7 +5276,7 @@ impl Tty7App { .cursor_pointer() .child( div() - .w(px(268.)) + .w_full() .rounded(rounding::TRACK_RADIUS) .overflow_hidden() .border_1() @@ -5005,8 +5295,10 @@ impl Tty7App { h_flex() .items_center() .gap_1p5() + .w_full() .child( div() + .truncate() .text_sm() .font_weight(if is_active { FontWeight::SEMIBOLD @@ -5017,7 +5309,12 @@ impl Tty7App { .child(p.name.clone()), ) .when(is_active, |s| { - s.child(Icon::new(IconName::Check).small().text_color(foreground)) + s.child( + Icon::new(IconName::Check) + .small() + .flex_shrink_0() + .text_color(foreground), + ) }), ) .on_click(cx.listener(move |this, _, window, cx| match slot { @@ -5036,7 +5333,7 @@ impl Tty7App { } v_flex() - .w(px(300.)) + .w(px(self.settings_columns_now().theme_panel)) .h_full() .flex_shrink_0() .bg(bg) @@ -5134,13 +5431,18 @@ impl Tty7App { // The label column has to be allowed to shrink, or its description sets // the row's width and the control it belongs to is pushed off the page. // `settings_row` does this for every other row in Settings; these two - // are hand-rolled and were missing it. - let preset_row = h_flex() - .w_full() - .items_center() - .justify_between() - .gap_8() - .py_2() + // are hand-rolled and were missing it. They take its breakpoint too: + // the segmented controls beside them are the widest on the page. + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); + let hand_rolled_row = |row: Div| { + row.w_full() + .flex() + .when(stacked, |r| r.flex_col().items_start().gap_2()) + .when(!stacked, |r| { + r.flex_row().items_center().justify_between().gap_8() + }) + }; + let preset_row = hand_rolled_row(div().py_2()) .child( v_flex() .min_w_0() @@ -5161,12 +5463,7 @@ impl Tty7App { ) .child(h_flex().flex_shrink_0().child(preset_control)); - let prefix_row = h_flex() - .w_full() - .items_center() - .justify_between() - .gap_8() - .py_2() + let prefix_row = hand_rolled_row(div().py_2()) .child( div() .min_w_0() @@ -5221,8 +5518,11 @@ impl Tty7App { let is_recording = recording.as_ref().is_some_and(|(a, _)| a == &action); let is_overridden = overridden.contains(&action); + // Wrapping, because a four-chord binding is wider than the column a + // narrow window leaves for it, and the alternative to a second line + // is a first one that runs off the page. let keycaps = |spec: &str| { - h_flex().gap_2().children( + h_flex().flex_wrap().gap_2().children( crate::ui::keymap::key_chords(spec) .into_iter() .map(|chord| h_flex().gap_1().children(chord.into_iter().map(&keycap))), @@ -5332,13 +5632,16 @@ impl Tty7App { } let last_in_group = heading_at.contains_key(&(i + 1)) || i + 1 == count; list = list.child( - h_flex() - .items_center() - .justify_between() - .py_1p5() + hand_rolled_row(div().py_1p5()) .when(!last_in_group, |s| s.border_b_1().border_color(border)) - .child(div().text_sm().text_color(foreground).child(label)) - .child(right), + .child( + div() + .min_w_0() + .text_sm() + .text_color(foreground) + .child(label), + ) + .child(right.flex_shrink_0()), ); } @@ -5454,6 +5757,7 @@ impl Tty7App { let http_proxy_control = v_flex() .gap_1() .w(px(260.)) + .max_w_full() .child(Input::new(&http_proxy_input).small()) .when(http_proxy_invalid, |this| { this.child( @@ -5809,21 +6113,177 @@ mod tests { use super::*; /// The row keeps its side-by-side shape while both halves fit, and stacks - /// once they do not. The SSH page reaches that point first — it spends 500px - /// on two lists before the row gets any. + /// once they do not. The SSH page reaches that point first — it spends its + /// host list before the row gets anything. #[test] fn a_row_stacks_once_its_label_and_control_stop_fitting() { use SettingsSection::*; - assert!(settings_row_width(Terminal, 1440.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Terminal, 900.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Terminal, 700.) < STACK_ROW_BELOW); + assert!(settings_row_width(Terminal, false, 1440., 1.) >= STACK_ROW_BELOW); + assert!(settings_row_width(Terminal, false, 900., 1.) >= STACK_ROW_BELOW); + // At the narrowest window that turns up the page is 420 wide, which is + // under the width where a label and a control still share a line. + assert!(settings_row_width(Terminal, false, NARROWEST_WINDOW, 1.) < STACK_ROW_BELOW); // Capped at the reading column, so a wider window never widens the row. - assert_eq!(settings_row_width(Terminal, 4000.), 640.); - // SSH crosses over while the window is still wide. - assert!(settings_row_width(Ssh, 1440.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Ssh, 1000.) < STACK_ROW_BELOW); + assert_eq!( + settings_row_width(Terminal, false, 4000., 1.), + READING_COLUMN + ); + // SSH crosses over while the window is still wide — it is the page that + // spends a host list before its rows get anything. + assert!(settings_row_width(Ssh, false, 1440., 1.) >= STACK_ROW_BELOW); + assert!(settings_row_width(Ssh, false, 900., 1.) < STACK_ROW_BELOW); // And never goes negative on a window narrower than its own chrome. - assert_eq!(settings_row_width(Ssh, 100.), 0.); + assert_eq!(settings_row_width(Ssh, false, 100., 1.), 0.); + } + + /// Every list gives width back before the page does, and no combination of + /// page and panel leaves the page below the width it is derived to keep. + /// 641pt is the window the report came from — under the declared 720 —- so + /// that is the case that has to hold, not the one the manifest promises. + #[test] + fn the_page_keeps_a_readable_width_at_every_window_that_turns_up() { + use SettingsSection::*; + for section in SettingsSection::ALL { + for panel_open in [false, true] { + for viewport in [NARROWEST_WINDOW, 641., 720., 900., 1100., 1440., 2560.] { + let cols = settings_columns(section, panel_open, viewport); + let pad = match section { + Ssh => SSH_DETAIL_PAD, + _ => PAGE_PAD, + }; + let panel = only_when(!cols.panel_overlays, cols.theme_panel); + let page = viewport - cols.nav - cols.ssh_list - panel - pad; + assert!( + page >= CONTENT_MIN_W, + "{viewport}px, {panel_open}: page got {page}, floor is {CONTENT_MIN_W}" + ); + // And the columns add up to the window rather than past it, + // which is what keeps the rightmost one on screen. + assert!(cols.nav + cols.ssh_list + panel + pad + page <= viewport + 1.); + assert!(cols.nav >= NAV_W_MIN && cols.nav <= NAV_W); + } + } + } + } + + /// The three screenshots in the report, by the numbers measured off them. + /// Every one of them is the same 641pt window. + #[test] + fn the_reported_window_lays_out_without_running_off_the_screen() { + use SettingsSection::*; + const REPORTED: f32 = 641.; + + // Shot 1 — SSH. Nav 220 and host list 280 left the detail 141pt and its + // empty state painted ~270 past the window edge. Both lists now stand + // on their floors and the detail keeps the rest. + let ssh = settings_columns(Ssh, false, REPORTED); + assert_eq!((ssh.nav, ssh.ssh_list), (NAV_W_MIN, SSH_LIST_W_MIN)); + let detail = settings_row_width(Ssh, false, REPORTED, 1.); + assert!(detail >= CONTENT_MIN_W, "SSH detail got {detail}"); + + // Shot 2 — Appearance, panel closed. The opacity row measured a 78pt + // label beside a 240pt slider; at this width the row has to stack. + // The page does not reach its preferred `CONTENT_W` here: the nav floor + // is sized to show a Japanese nav label whole, and at 641 that costs the + // page the difference. Readable and stacked is the property that matters. + let page = settings_row_width(Appearance, false, REPORTED, 1.); + assert_eq!(page, REPORTED - NAV_W_MIN - PAGE_PAD); + assert!(page >= CONTENT_MIN_W, "the page got {page}"); + assert!( + page < STACK_ROW_BELOW, + "the opacity row has to stack at 641" + ); + + // Shot 3 — Appearance with the theme panel. 220 + 300 of chrome left + // the page ~125pt, one Chinese character per line. The panel now lifts + // off the row entirely and the page is back to shot 2's width. + assert!(settings_columns(Appearance, true, REPORTED).panel_overlays); + assert_eq!(settings_row_width(Appearance, true, REPORTED, 1.), page); + } + + /// The list that has to give the most is the one the window has the least + /// room for, and no list is ever asked for more than it has to spare. + #[test] + fn the_lists_shrink_together_and_stop_at_their_floors() { + use SettingsSection::*; + // Wide enough for everyone: nothing moves. + let wide = settings_columns(Ssh, false, 1440.); + assert_eq!((wide.nav, wide.ssh_list), (NAV_W, SSH_LIST_W)); + // The reported window — half of a 1440pt screen, three columns on SSH. + // Both lists give, neither past its floor, and the detail comes out at + // its preferred width instead of the 336 it used to be left with. + let half = settings_columns(Ssh, false, 900.); + assert!(half.nav < NAV_W && half.ssh_list < SSH_LIST_W); + assert!(half.nav >= NAV_W_MIN && half.ssh_list >= SSH_LIST_W_MIN); + assert_eq!( + settings_row_width(Ssh, false, 900., 1.).round(), + CONTENT_W, + "the SSH detail should get its preferred width at 900pt" + ); + // The narrowest window that turns up: both at the floor, page readable. + let tiny = settings_columns(Ssh, false, NARROWEST_WINDOW); + assert_eq!((tiny.nav, tiny.ssh_list), (NAV_W_MIN, SSH_LIST_W_MIN)); + assert_eq!( + settings_row_width(Ssh, false, NARROWEST_WINDOW, 1.), + CONTENT_MIN_W + ); + } + + /// The thresholds are widths a *label* needs, and a reader who scaled the + /// interface up scaled every label with it while the slider beside it kept + /// the px width it was built at. A window that reads fine at the default + /// font is a starved label column at the largest one. + #[test] + fn the_stacking_width_follows_the_interface_font() { + use crate::core::config::UI_FONT_SIZE_MAX; + use SettingsSection::*; + let large = UI_FONT_SIZE_MAX / UI_FONT_SIZE_DEFAULT; + // Side by side at the default font... + assert!(settings_row_width(Appearance, false, 900., 1.) >= STACK_ROW_BELOW); + // ...and stacked at the largest, where the same row holds half as much. + assert!( + settings_row_width(Appearance, false, 900., large) < STACK_ROW_BELOW * large, + "a 900pt window at the largest interface font has to stack" + ); + // The reading column grows with the font, so a wide window does not. + assert!(settings_row_width(Appearance, false, 1600., large) >= STACK_ROW_BELOW * large); + } + + /// The theme panel took its 300px from the page and from nothing else, so + /// a half-width window with it open rendered a description one character + /// wide. It now shrinks with everything else, and stops being a column at + /// all once even that is not enough. + #[test] + fn the_theme_panel_yields_before_the_page_does() { + use SettingsSection::*; + assert!(settings_row_width(Appearance, true, 900., 1.) < STACK_ROW_BELOW); + assert!( + settings_row_width(Appearance, true, 900., 1.) + < settings_row_width(Appearance, false, 900., 1.) + ); + // Beside the page while both fit — which, with the panel and the nav + // both allowed down to their floors, still holds at 720. + assert!(!settings_columns(Appearance, true, 900.).panel_overlays); + assert!(!settings_columns(Appearance, true, 720.).panel_overlays); + // ...and over it once they do not, at which point the page is back to + // the width it has with the panel closed. + assert!(settings_columns(Appearance, true, 641.).panel_overlays); + assert_eq!( + settings_row_width(Appearance, true, 641., 1.), + settings_row_width(Appearance, false, 641., 1.) + ); + // The panel is the only chrome that can leave, so it has to leave in + // time: at 641 there is no arrangement in which it and a readable page + // both fit in a row. + assert!( + NARROWEST_WINDOW - NAV_W_MIN - THEME_PANEL_W_MIN - PAGE_PAD < CONTENT_MIN_W, + "the overlay threshold has to fire at the narrowest window" + ); + // Wide enough and the cap is the reading column either way. + assert_eq!( + settings_row_width(Appearance, true, 1600., 1.), + READING_COLUMN + ); } #[test] diff --git a/src/ui/windows.rs b/src/ui/windows.rs index 8603c795..9d7800e7 100644 --- a/src/ui/windows.rs +++ b/src/ui/windows.rs @@ -549,7 +549,7 @@ fn window_options(cx: &mut App, workspace: Option) -> WindowOptions let existing = WindowRegistry::count(cx); let bounds = match remembered { Some(state) => { - let bounds = state.bounds(); + let bounds = at_least_min_size(state.bounds()); if cx.displays().iter().any(|d| d.bounds().intersects(&bounds)) { bounds } else { @@ -582,6 +582,23 @@ fn window_options(cx: &mut App, workspace: Option) -> WindowOptions } } +/// Grow a remembered bound back up to `MIN_SIZE`. +/// +/// `window_min_size` only governs what a *drag* may do to a window; the bounds +/// we open with are taken as given. A window that got under the minimum — an +/// older build that had no minimum, a hand-edited `views.json`, a display that +/// went away — was reopened at whatever it had been saved at, and the settings +/// page it opened onto had never been laid out for that width. +fn at_least_min_size(bounds: Bounds) -> Bounds { + Bounds { + origin: bounds.origin, + size: size( + bounds.size.width.max(px(MIN_SIZE.0)), + bounds.size.height.max(px(MIN_SIZE.1)), + ), + } +} + fn cascade(bounds: Bounds, existing: usize) -> Bounds { if existing == 0 { return bounds; @@ -626,6 +643,37 @@ mod tests { assert_eq!(cascade(b, 3).size, b.size); } + #[test] + fn a_remembered_bound_under_the_minimum_is_grown_back_to_it() { + // The window in the report this was fixed for: 641x830, saved and + // reopened at a width no settings page is laid out for. + let undersized = Bounds { + origin: point(px(700.), px(60.)), + size: size(px(641.), px(830.)), + }; + let grown = at_least_min_size(undersized); + assert_eq!(grown.size.width, px(MIN_SIZE.0)); + assert_eq!(grown.size.height, px(830.), "a tall enough height is kept"); + assert_eq!(grown.origin, undersized.origin, "the corner does not move"); + + let short = at_least_min_size(Bounds { + origin: point(px(0.), px(0.)), + size: size(px(1200.), px(300.)), + }); + assert_eq!(short.size, size(px(1200.), px(MIN_SIZE.1))); + } + + #[test] + fn a_remembered_bound_at_or_over_the_minimum_is_left_alone() { + let b = bounds_at(100., 100.); + assert_eq!(at_least_min_size(b).size, b.size); + let exact = Bounds { + origin: point(px(0.), px(0.)), + size: size(px(MIN_SIZE.0), px(MIN_SIZE.1)), + }; + assert_eq!(at_least_min_size(exact).size, exact.size); + } + #[test] fn cascade_wraps_so_windows_never_march_off_screen() { let b = bounds_at(100., 100.);