From c3a699781fd679f120de468b77a9ea3defc643ef Mon Sep 17 00:00:00 2001 From: Eridanus <45489268+Eridanus117@users.noreply.github.com> Date: Tue, 5 May 2026 05:20:59 -0700 Subject: [PATCH 1/4] Make UI zoom per-window instead of global When UIZoom is enabled, Cmd++/Cmd+-/Cmd+0 in one window currently propagates to every window because zoom_factor lives on AppContext and set_zoom_factor invalidates all views. The handlers also wrote through WindowSettings::zoom_level, which is an app-wide singleton. Add a per-window override on top of the existing app-wide default: effective_zoom = window.zoom_factor_override ?? AppContext.zoom_factor - core::Window: zoom_factor_override: Option - AppContext: window_zoom_factor / set_window_zoom_factor / reset_window_zoom_factor; the setters only invalidate the target window's views. - presenter.rs: build_scene + dispatch_event read window_zoom_factor. - Workspace adjust_zoom / reset_zoom now write the per-window override directly. WindowSettings.zoom_level remains the surface for the Settings dropdown ("Default Zoom"), which goes through the existing AppContext::set_zoom_factor path; windows without overrides follow it. Aligns with the prior art shape used by VS Code (window.zoomLevel + per-window override), Kitty (change_font_size affects current OS window), and iTerm2 (per-session font size on top of profile). --- app/src/workspace/view.rs | 28 ++++++++++---------- crates/warpui_core/src/core/app.rs | 37 +++++++++++++++++++++++++-- crates/warpui_core/src/core/window.rs | 6 ++++- crates/warpui_core/src/presenter.rs | 7 ++--- crates/warpui_core/src/zoom.rs | 4 +++ 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 56422006d..6e7554881 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -356,7 +356,7 @@ use crate::view_components::callout_bubble::{ use crate::view_components::{ AgentToast, AgentToastStack, DismissibleToast, DismissibleToastStack, ToastLink, }; -use crate::window_settings::{WindowSettings, WindowSettingsChangedEvent, ZoomLevel}; +use crate::window_settings::{WindowSettings, WindowSettingsChangedEvent}; use crate::workflows::{ manager::WorkflowOpenSource, AIWorkflowOrigin, CloudWorkflow, WorkflowSelectionSource, WorkflowSource, WorkflowType, WorkflowViewMode, @@ -15550,18 +15550,22 @@ impl Workspace { } fn reset_zoom(&mut self, ctx: &mut ViewContext) { - WindowSettings::handle(ctx).update(ctx, |window_settings, ctx| { - report_if_error!(window_settings - .zoom_level - .set_value(ZoomLevel::default_value(), ctx)); - }); + // Cmd+0: clear the per-window zoom override so this window follows + // the app-wide default again. + let window_id = ctx.window_id(); + ctx.reset_window_zoom_factor(window_id); } fn adjust_zoom(&mut self, increase: bool, ctx: &mut ViewContext) { - let current_zoom = *WindowSettings::as_ref(ctx).zoom_level.value(); + // Cmd++ / Cmd+-: step the focused window's zoom up or down within + // the discrete percentages defined by `ZoomLevel::VALUES`. Writes a + // per-window override; other windows are unaffected. + let window_id = ctx.window_id(); + let current_factor = ctx.window_zoom_factor(window_id); + let current_percent = (current_factor.as_f32() * 100.0).round() as u16; let Some(current_index) = crate::window_settings::ZoomLevel::VALUES .iter() - .position(|zoom| *zoom == current_zoom) + .position(|zoom| *zoom == current_percent) else { return; }; @@ -15572,11 +15576,9 @@ impl Workspace { current_index.saturating_sub(1) }; - WindowSettings::handle(ctx).update(ctx, |window_settings, ctx| { - report_if_error!(window_settings - .zoom_level - .set_value(crate::window_settings::ZoomLevel::VALUES[next_index], ctx)); - }); + let next_factor = + crate::window_settings::ZoomLevel::VALUES[next_index] as f32 / 100.0; + ctx.set_window_zoom_factor(window_id, next_factor); } fn adjust_terminal_font_size(&mut self, font_size_delta: f32, ctx: &mut ViewContext) { diff --git a/crates/warpui_core/src/core/app.rs b/crates/warpui_core/src/core/app.rs index 1f4d62c8a..91076a3da 100644 --- a/crates/warpui_core/src/core/app.rs +++ b/crates/warpui_core/src/core/app.rs @@ -1178,8 +1178,9 @@ impl AppContext { self.event_munger = Box::new(handler) } - /// Sets the zoom factor for the application. Changing the zoom factor adjusts - /// the magnification of every element rendered within the application. + /// Sets the app-wide default zoom factor. Per-window overrides set via + /// [`AppContext::set_window_zoom_factor`] are not affected; windows that + /// have no override will follow this new default. /// /// All views in every window are invalidated when this is invoked. /// @@ -1191,6 +1192,38 @@ impl AppContext { self.invalidate_all_views(); } + /// Returns the effective zoom factor for `window_id`: the per-window + /// override if set, otherwise the app-wide default. Falls back to the + /// default if `window_id` is not found. + pub fn window_zoom_factor(&self, window_id: WindowId) -> ZoomFactor { + self.windows + .get(&window_id) + .and_then(|w| w.zoom_factor_override) + .unwrap_or(self.zoom_factor) + } + + /// Sets a per-window zoom factor override. Only invalidates the views of + /// `window_id`; other windows are unaffected. + /// + /// ## Validation + /// The zoom factor is clamped to the range [0.5, 4.0]. + pub fn set_window_zoom_factor(&mut self, window_id: WindowId, zoom_factor: f32) { + let zoom_factor = ZoomFactor::new(zoom_factor.clamp(0.5, 4.0)); + if let Some(window) = self.windows.get_mut(&window_id) { + window.zoom_factor_override = Some(zoom_factor); + self.invalidate_all_views_for_window(window_id); + } + } + + /// Clears the per-window zoom factor override. The window will follow the + /// app-wide default returned by [`AppContext::zoom_factor`]. + pub fn reset_window_zoom_factor(&mut self, window_id: WindowId) { + if let Some(window) = self.windows.get_mut(&window_id) { + window.zoom_factor_override = None; + self.invalidate_all_views_for_window(window_id); + } + } + /// Sets the callback invoked before opening a URL. pub fn set_before_open_url(&mut self, handler: F) where diff --git a/crates/warpui_core/src/core/window.rs b/crates/warpui_core/src/core/window.rs index 1ac1c3e85..a6cb5b2a2 100644 --- a/crates/warpui_core/src/core/window.rs +++ b/crates/warpui_core/src/core/window.rs @@ -6,7 +6,7 @@ use std::{ use serde::{Deserialize, Serialize}; -use crate::{core::view::AnyViewHandle, AnyView, EntityId}; +use crate::{core::view::AnyViewHandle, AnyView, EntityId, ZoomFactor}; /// A unique identifier for a window. /// @@ -47,4 +47,8 @@ pub(super) struct Window { /// The ID of the currently focused view, if any. pub focused_view: Option, + + /// Per-window zoom factor override. `None` means the window follows the + /// app-wide default returned by [`AppContext::zoom_factor`]. + pub zoom_factor_override: Option, } diff --git a/crates/warpui_core/src/presenter.rs b/crates/warpui_core/src/presenter.rs index 7e6f2ec86..e193d615d 100644 --- a/crates/warpui_core/src/presenter.rs +++ b/crates/warpui_core/src/presenter.rs @@ -343,8 +343,9 @@ impl Presenter { // is proportionally smaller based on the current zoom factor. Once we build up a scene // with the fake window bounds, we then adjust the scale factor to include the zoom level // so every item in the scene is blown up to fit in the actual window bounds. - let zoomed_window_size = window_size.scale_down(ctx.zoom_factor()); - let zoomed_scale_factor = scale_factor.scale_up(ctx.zoom_factor()); + let zoom = ctx.window_zoom_factor(self.window_id); + let zoomed_window_size = window_size.scale_down(zoom); + let zoomed_scale_factor = scale_factor.scale_up(zoom); self.layout(zoomed_window_size, ctx); // In theory, after_layout would be a good place for Elements to update app state with the @@ -517,7 +518,7 @@ impl Presenter { pub fn dispatch_event(&mut self, event: Event, app: &AppContext) -> DispatchResult { // Translate all events to be in the coordinate space after factoring in the // zoom factor. - let event = event.scale_down(app.zoom_factor()); + let event = event.scale_down(app.window_zoom_factor(self.window_id)); let window_id = self.window_id; let mut event_ctx = self.create_event_context(app.font_cache()); let handled = app.root_view_id(window_id).is_some_and(|root_view_id| { diff --git a/crates/warpui_core/src/zoom.rs b/crates/warpui_core/src/zoom.rs index 9c7d47626..6fc1892b7 100644 --- a/crates/warpui_core/src/zoom.rs +++ b/crates/warpui_core/src/zoom.rs @@ -10,6 +10,10 @@ impl ZoomFactor { pub fn new(zoom_level: f32) -> Self { Self(zoom_level) } + + pub fn as_f32(&self) -> f32 { + self.0 + } } impl Default for ZoomFactor { From 6d38c6b0b0b0784b873d981414fffe4d15dda9ba Mon Sep 17 00:00:00 2001 From: Eridanus <45489268+Eridanus117@users.noreply.github.com> Date: Tue, 5 May 2026 05:49:57 -0700 Subject: [PATCH 2/4] Fix zoom-aware readers and chrome layout for per-window override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rev 2 addresses dual-reviewer block on Rev 1: the patch only routed build_scene + dispatch_event through window_zoom_factor, but four other zoom readers in the chrome/layout path still consulted the app-wide WindowSettings::zoom_level. Cmd++/Cmd+- on macOS would therefore scale the rendered tab bar but leave the native titlebar height and traffic-light reservation at the global zoom, producing a visibly broken titlebar. - view.rs:11990 update_titlebar_height - view.rs:17494 traffic-light reservation - view.rs:17516 compute_tab_bar_left_padding - theme_chooser.rs:593 traffic-light header margin All four now read ctx.window_zoom_factor(window_id).as_f32(). Workspace::adjust_zoom / reset_zoom now invoke update_titlebar_height explicitly because the per-window override path no longer mutates WindowSettings::zoom_level (so WindowSettingsChangedEvent::ZoomLevel no longer fires for Cmd++/Cmd+-/Cmd+0). set_window_zoom_factor clamp tightened to [0.5, 3.5] to match ZoomLevel::VALUES (50%–350%); the original [0.5, 4.0] left a 3.5–4.0 band that adjust_zoom's position lookup couldn't step within. Add unit tests (crates/warpui_core/src/core/zoom_test.rs) covering: - default fallback to AppContext.zoom_factor - per-window isolation under set - global default change leaves overrides untouched, updates non-overridden windows - reset clears override - missing window_id is silent noop - clamp [0.5, 3.5] enforced --- app/src/themes/theme_chooser.rs | 3 +- app/src/workspace/view.rs | 17 ++-- crates/warpui_core/src/core/app.rs | 8 +- crates/warpui_core/src/core/mod.rs | 4 + crates/warpui_core/src/core/zoom_test.rs | 105 +++++++++++++++++++++++ 5 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 crates/warpui_core/src/core/zoom_test.rs diff --git a/app/src/themes/theme_chooser.rs b/app/src/themes/theme_chooser.rs index 9fc34e00d..0c33ea2b3 100644 --- a/app/src/themes/theme_chooser.rs +++ b/app/src/themes/theme_chooser.rs @@ -36,7 +36,6 @@ use crate::{ themes::theme::SelectedSystemThemes, user_config::{load_theme_configs, themes_dir, WarpConfig, WarpConfigUpdateEvent}, util::traffic_lights::{TrafficLightData, TrafficLightSide}, - window_settings::WindowSettings, }; use crate::{appearance::AppearanceManager, send_telemetry_from_ctx}; use crate::{editor::EditorView, resource_center::TipsCompleted}; @@ -590,7 +589,7 @@ impl ThemeChooser { ) -> Box { let mut margin_left = 16.; - let zoom_factor = WindowSettings::as_ref(app).zoom_level.as_zoom_factor(); + let zoom_factor = app.window_zoom_factor(self.window_id).as_f32(); // Since this panel is always on the left, only account for left-side traffic lights. if let Some(width) = traffic_light_data .filter(|data| data.side == TrafficLightSide::Left) diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 6e7554881..bb04df0dc 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -11987,7 +11987,7 @@ impl Workspace { /// Updates the titlebar height to match the scaled tab bar height. pub fn update_titlebar_height(&self, ctx: &mut ViewContext) { - let zoom_factor = WindowSettings::as_ref(ctx).zoom_level.as_zoom_factor(); + let zoom_factor = ctx.window_zoom_factor(ctx.window_id()).as_f32(); let scaled_tab_bar_height = (TOTAL_TAB_BAR_HEIGHT * zoom_factor) as f64; if let Some(platform_window) = ctx.windows().platform_window(ctx.window_id()) { @@ -15551,15 +15551,21 @@ impl Workspace { fn reset_zoom(&mut self, ctx: &mut ViewContext) { // Cmd+0: clear the per-window zoom override so this window follows - // the app-wide default again. + // the app-wide default again. The native macOS titlebar height also + // needs to be recomputed for this window because we no longer route + // through `WindowSettingsChangedEvent::ZoomLevel`. let window_id = ctx.window_id(); ctx.reset_window_zoom_factor(window_id); + self.update_titlebar_height(ctx); } fn adjust_zoom(&mut self, increase: bool, ctx: &mut ViewContext) { // Cmd++ / Cmd+-: step the focused window's zoom up or down within // the discrete percentages defined by `ZoomLevel::VALUES`. Writes a - // per-window override; other windows are unaffected. + // per-window override; other windows are unaffected. The native macOS + // titlebar height is recomputed explicitly because Cmd++/Cmd+- no + // longer mutates `WindowSettings::zoom_level` (and therefore does not + // fire `WindowSettingsChangedEvent::ZoomLevel`). let window_id = ctx.window_id(); let current_factor = ctx.window_zoom_factor(window_id); let current_percent = (current_factor.as_f32() * 100.0).round() as u16; @@ -15579,6 +15585,7 @@ impl Workspace { let next_factor = crate::window_settings::ZoomLevel::VALUES[next_index] as f32 / 100.0; ctx.set_window_zoom_factor(window_id, next_factor); + self.update_titlebar_height(ctx); } fn adjust_terminal_font_size(&mut self, font_size_delta: f32, ctx: &mut ViewContext) { @@ -17491,7 +17498,7 @@ impl Workspace { } } - let zoom_factor = WindowSettings::as_ref(ctx).zoom_level.as_zoom_factor(); + let zoom_factor = ctx.window_zoom_factor(self.window_id).as_f32(); let traffic_light_data = traffic_light_data(ctx, self.window_id); if let Some(traffic_light_data) = traffic_light_data.as_ref() { let vertical_tabs_active = FeatureFlag::VerticalTabs.is_enabled() @@ -17513,7 +17520,7 @@ impl Workspace { } fn compute_tab_bar_left_padding(&self, ctx: &AppContext) -> f32 { - let zoom_factor = WindowSettings::as_ref(ctx).zoom_level.as_zoom_factor(); + let zoom_factor = ctx.window_zoom_factor(self.window_id).as_f32(); let traffic_light_data = traffic_light_data(ctx, self.window_id); let is_window_fullscreen = ctx .windows() diff --git a/crates/warpui_core/src/core/app.rs b/crates/warpui_core/src/core/app.rs index 91076a3da..6ef78d180 100644 --- a/crates/warpui_core/src/core/app.rs +++ b/crates/warpui_core/src/core/app.rs @@ -1206,9 +1206,13 @@ impl AppContext { /// `window_id`; other windows are unaffected. /// /// ## Validation - /// The zoom factor is clamped to the range [0.5, 4.0]. + /// The zoom factor is clamped to the range [0.5, 3.5], aligning with the + /// discrete percentages exposed by `WindowSettings::ZoomLevel::VALUES` + /// (50%–350%). Values outside this range are not reachable through the + /// keyboard shortcut, and would leave `adjust_zoom`'s position lookup + /// unable to step until `reset_window_zoom_factor` clears the override. pub fn set_window_zoom_factor(&mut self, window_id: WindowId, zoom_factor: f32) { - let zoom_factor = ZoomFactor::new(zoom_factor.clamp(0.5, 4.0)); + let zoom_factor = ZoomFactor::new(zoom_factor.clamp(0.5, 3.5)); if let Some(window) = self.windows.get_mut(&window_id) { window.zoom_factor_override = Some(zoom_factor); self.invalidate_all_views_for_window(window_id); diff --git a/crates/warpui_core/src/core/mod.rs b/crates/warpui_core/src/core/mod.rs index 352d4cff2..591ede49f 100644 --- a/crates/warpui_core/src/core/mod.rs +++ b/crates/warpui_core/src/core/mod.rs @@ -664,3 +664,7 @@ impl RequestState { #[cfg(test)] #[path = "mod_test.rs"] mod tests; + +#[cfg(test)] +#[path = "zoom_test.rs"] +mod zoom_tests; diff --git a/crates/warpui_core/src/core/zoom_test.rs b/crates/warpui_core/src/core/zoom_test.rs new file mode 100644 index 000000000..1f73d8e3f --- /dev/null +++ b/crates/warpui_core/src/core/zoom_test.rs @@ -0,0 +1,105 @@ +use super::*; +use crate::elements::*; +use crate::platform::WindowStyle; + +#[derive(Default)] +struct ZoomTestView; + +impl Entity for ZoomTestView { + type Event = (); +} + +impl super::View for ZoomTestView { + fn render(&self, _: &AppContext) -> Box { + Empty::new().finish() + } + + fn ui_name() -> &'static str { + "ZoomTestView" + } +} + +impl TypedActionView for ZoomTestView { + type Action = (); +} + +#[test] +fn test_per_window_zoom_factor_invariants() { + App::test((), |mut app| async move { + let app = &mut app; + + let (window_a, _) = app.add_window(WindowStyle::NotStealFocus, |_| { + ZoomTestView::default() + }); + let (window_b, _) = app.add_window(WindowStyle::NotStealFocus, |_| { + ZoomTestView::default() + }); + + // Invariant 1: no override → effective is the app-wide default (1.0). + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_a).as_f32(), 1.0); + assert_eq!(ctx.window_zoom_factor(window_b).as_f32(), 1.0); + }); + + // Invariant 2: per-window override only affects the target window. + app.update(|ctx| { + ctx.set_window_zoom_factor(window_a, 1.5); + }); + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_a).as_f32(), 1.5); + assert_eq!(ctx.window_zoom_factor(window_b).as_f32(), 1.0); + }); + + // Invariant 3: changing the global default leaves overridden windows + // alone but updates non-overridden ones. + app.update(|ctx| { + ctx.set_zoom_factor(1.3); + }); + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_a).as_f32(), 1.5); + assert_eq!(ctx.window_zoom_factor(window_b).as_f32(), 1.3); + }); + + // Invariant 4: reset clears the override so the window follows the + // global default again. + app.update(|ctx| { + ctx.reset_window_zoom_factor(window_a); + }); + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_a).as_f32(), 1.3); + assert_eq!(ctx.window_zoom_factor(window_b).as_f32(), 1.3); + }); + + // Invariant 5: a missing window_id is a silent noop on set/reset and + // window_zoom_factor falls back to the global default. + app.update(|ctx| { + let bogus = WindowId::from_usize(99_999); + ctx.set_window_zoom_factor(bogus, 1.7); + ctx.reset_window_zoom_factor(bogus); + assert_eq!(ctx.window_zoom_factor(bogus).as_f32(), 1.3); + }); + }); +} + +#[test] +fn test_set_window_zoom_factor_clamps_to_values_range() { + App::test((), |mut app| async move { + let app = &mut app; + + let (window_id, _) = app.add_window(WindowStyle::NotStealFocus, |_| { + ZoomTestView::default() + }); + + // Below the minimum: clamped up to 0.5. + app.update(|ctx| ctx.set_window_zoom_factor(window_id, 0.1)); + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_id).as_f32(), 0.5); + }); + + // Above the maximum: clamped down to 3.5 (matches ZoomLevel::VALUES). + app.update(|ctx| ctx.set_window_zoom_factor(window_id, 5.0)); + app.read(|ctx| { + assert_eq!(ctx.window_zoom_factor(window_id).as_f32(), 3.5); + }); + }); +} From 16cff28db50e6f84c972097dd30ea8f47365b259 Mon Sep 17 00:00:00 2001 From: Eridanus <45489268+Eridanus117@users.noreply.github.com> Date: Tue, 5 May 2026 06:21:57 -0700 Subject: [PATCH 3/4] Align AppContext::set_zoom_factor clamp to ZoomLevel::VALUES range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-wide setter clamped to [0.5, 4.0] while the new per-window setter clamps to [0.5, 3.5] (matching ZoomLevel::VALUES, 50%–350%). Asymmetry is observable: a caller that pushes the global default into the [3.5, 4.0] band would leave non-overridden windows at an effective zoom that adjust_zoom's discrete-step lookup cannot find in VALUES, making Cmd++/Cmd+- silently no-op until reset_window_zoom_factor. No internal caller in this PR drives set_zoom_factor outside the VALUES range (lib.rs startup and appearance_page.rs settings dropdown both read percentages straight from VALUES), so the immediate symptom is not user-reachable. Aligning the clamp now keeps both setters honest and removes a future-trap ahead of upstream review. --- crates/warpui_core/src/core/app.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/warpui_core/src/core/app.rs b/crates/warpui_core/src/core/app.rs index 6ef78d180..0ce3acdea 100644 --- a/crates/warpui_core/src/core/app.rs +++ b/crates/warpui_core/src/core/app.rs @@ -1185,9 +1185,15 @@ impl AppContext { /// All views in every window are invalidated when this is invoked. /// /// ## Validation - /// The zoom factor is clamped to the range [0.5, 4.0]. + /// The zoom factor is clamped to the range [0.5, 3.5], matching + /// `WindowSettings::ZoomLevel::VALUES` (50%–350%). This range is shared + /// with [`AppContext::set_window_zoom_factor`] so that + /// `Workspace::adjust_zoom`'s discrete-step lookup can always find the + /// effective zoom in the VALUES table; values outside the range would + /// dead-end the keyboard shortcut until `reset_window_zoom_factor` is + /// called. pub fn set_zoom_factor(&mut self, zoom_factor: f32) { - let zoom_factor = ZoomFactor::new(zoom_factor.clamp(0.5, 4.0)); + let zoom_factor = ZoomFactor::new(zoom_factor.clamp(0.5, 3.5)); self.zoom_factor = zoom_factor; self.invalidate_all_views(); } From e7a783f8879c8a0cc7fcbeb19aab44e4be16a939 Mon Sep 17 00:00:00 2001 From: Eridanus <45489268+Eridanus117@users.noreply.github.com> Date: Tue, 5 May 2026 09:07:06 -0700 Subject: [PATCH 4/4] adjust_zoom: snap to nearest VALUES entry instead of position lookup Addresses oz-agent review on PR #10145: keyboard zoom path could no-op when the effective zoom factor was a valid in-range value but not exactly one of `ZoomLevel::VALUES`. The previous implementation looked up the current percentage in VALUES and stepped by index; if the current value wasn't in VALUES (e.g. a caller set the global default to 105% or any non-discrete percentage in [0.5, 3.5]), `position()` returned `None` and `adjust_zoom` early-returned, leaving Cmd++/Cmd+- silently inert. Replace the index lookup with a directional search for the next/prev VALUES entry strictly above/below `current_percent`, falling back to the boundary value if there is no further step. Behaviour for values already in VALUES is unchanged; non-VALUES values now snap to the nearest VALUES entry in the chosen direction. Extract the step computation as a free `next_zoom_step` so the logic can be unit-tested in isolation. Add 8 tests covering: in-VALUES step up/down, non-VALUES snap up/down, boundary stays (at min/max), and out-of-range graceful fallback. --- app/src/workspace/view.rs | 112 +++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index bb04df0dc..31fc51ba7 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -1055,6 +1055,94 @@ pub struct Workspace { remove_tab_config_confirmation_dialog: ViewHandle, } +/// Returns the next zoom percentage from `ZoomLevel::VALUES` relative to +/// `current_percent`, snapping to the nearest VALUES entry in the chosen +/// direction even when `current_percent` itself is not a VALUES entry. +/// Stays at the boundary if there is no further step in that direction. +fn next_zoom_step(current_percent: u16, increase: bool) -> u16 { + use crate::window_settings::ZoomLevel; + if increase { + ZoomLevel::VALUES + .iter() + .find(|&&v| v > current_percent) + .copied() + .unwrap_or_else(|| { + *ZoomLevel::VALUES + .last() + .expect("ZoomLevel::VALUES is non-empty") + }) + } else { + ZoomLevel::VALUES + .iter() + .rev() + .find(|&&v| v < current_percent) + .copied() + .unwrap_or_else(|| { + *ZoomLevel::VALUES + .first() + .expect("ZoomLevel::VALUES is non-empty") + }) + } +} + +#[cfg(test)] +mod next_zoom_step_tests { + use super::next_zoom_step; + use crate::window_settings::ZoomLevel; + + #[test] + fn step_up_from_in_values() { + assert_eq!(next_zoom_step(100, true), 110); + assert_eq!(next_zoom_step(110, true), 125); + } + + #[test] + fn step_down_from_in_values() { + assert_eq!(next_zoom_step(100, false), 90); + assert_eq!(next_zoom_step(110, false), 100); + } + + #[test] + fn step_up_from_non_values_snaps_to_next_higher() { + // 105 is between 100 and 110 in VALUES; up → 110. + assert_eq!(next_zoom_step(105, true), 110); + // 230 is between 225 and 250; up → 250. + assert_eq!(next_zoom_step(230, true), 250); + } + + #[test] + fn step_down_from_non_values_snaps_to_next_lower() { + // 105 → down → 100. + assert_eq!(next_zoom_step(105, false), 100); + // 230 → down → 225. + assert_eq!(next_zoom_step(230, false), 225); + } + + #[test] + fn step_up_at_max_stays_at_max() { + let max = *ZoomLevel::VALUES.last().unwrap(); + assert_eq!(next_zoom_step(max, true), max); + } + + #[test] + fn step_down_at_min_stays_at_min() { + let min = *ZoomLevel::VALUES.first().unwrap(); + assert_eq!(next_zoom_step(min, false), min); + } + + #[test] + fn step_up_above_max_stays_at_max() { + // current_percent above all VALUES: no entry > current → stays at max. + assert_eq!(next_zoom_step(500, true), *ZoomLevel::VALUES.last().unwrap()); + } + + #[test] + fn step_down_below_min_stays_at_min() { + // current_percent below all VALUES: no entry < current → stays at min. + assert_eq!(next_zoom_step(10, false), *ZoomLevel::VALUES.first().unwrap()); + } +} + impl Workspace { pub fn is_tab_drag_preview(&self) -> bool { self.is_tab_drag_preview @@ -15566,24 +15654,18 @@ impl Workspace { // titlebar height is recomputed explicitly because Cmd++/Cmd+- no // longer mutates `WindowSettings::zoom_level` (and therefore does not // fire `WindowSettingsChangedEvent::ZoomLevel`). + // + // The current effective zoom is not guaranteed to be one of the + // discrete `ZoomLevel::VALUES` entries — e.g. a caller could set the + // app-wide default to a value that does not appear in VALUES — so we + // snap to the nearest VALUES entry in the chosen direction rather + // than indexing into VALUES directly. This keeps Cmd++/Cmd+- usable + // from any in-range starting factor. let window_id = ctx.window_id(); let current_factor = ctx.window_zoom_factor(window_id); let current_percent = (current_factor.as_f32() * 100.0).round() as u16; - let Some(current_index) = crate::window_settings::ZoomLevel::VALUES - .iter() - .position(|zoom| *zoom == current_percent) - else { - return; - }; - - let next_index = if increase { - (current_index + 1).min(crate::window_settings::ZoomLevel::VALUES.len() - 1) - } else { - current_index.saturating_sub(1) - }; - - let next_factor = - crate::window_settings::ZoomLevel::VALUES[next_index] as f32 / 100.0; + let next_percent = next_zoom_step(current_percent, increase); + let next_factor = next_percent as f32 / 100.0; ctx.set_window_zoom_factor(window_id, next_factor); self.update_titlebar_height(ctx); }