Continue improving font adjustment settings (#24908)

Kirill Bulatov created

Follow-up of https://github.com/zed-industries/zed/pull/24857

Based on the feedback,

* made non-persisting font size change as a default in Zed keymaps
JetBrains IDEs seem to persist font size changes by default, hence left
to do so in Zed keymaps too

* fixed a bug with holding a binding to change the font size caused
flickering

Release Notes:

- N/A

Change summary

assets/keymaps/default-linux.json |  8 ++--
assets/keymaps/default-macos.json |  8 ++--
crates/theme/src/settings.rs      | 26 +++++++++------
crates/zed/src/zed.rs             | 54 ++++++++++++++++++++------------
4 files changed, 58 insertions(+), 38 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -24,10 +24,10 @@
       "shift-escape": "workspace::ToggleZoom",
       "open": "workspace::Open",
       "ctrl-o": "workspace::Open",
-      "ctrl-=": ["zed::IncreaseBufferFontSize", { "persist": true }],
-      "ctrl-+": ["zed::IncreaseBufferFontSize", { "persist": true }],
-      "ctrl--": ["zed::DecreaseBufferFontSize", { "persist": true }],
-      "ctrl-0": ["zed::ResetBufferFontSize", { "persist": true }],
+      "ctrl-=": ["zed::IncreaseBufferFontSize", { "persist": false }],
+      "ctrl-+": ["zed::IncreaseBufferFontSize", { "persist": false }],
+      "ctrl--": ["zed::DecreaseBufferFontSize", { "persist": false }],
+      "ctrl-0": ["zed::ResetBufferFontSize", { "persist": false }],
       "ctrl-,": "zed::OpenSettings",
       "ctrl-q": "zed::Quit",
       "f11": "zed::ToggleFullScreen",

assets/keymaps/default-macos.json 🔗

@@ -28,10 +28,10 @@
       "cmd-shift-w": "workspace::CloseWindow",
       "shift-escape": "workspace::ToggleZoom",
       "cmd-o": "workspace::Open",
-      "cmd-=": ["zed::IncreaseBufferFontSize", { "persist": true }],
-      "cmd-+": ["zed::IncreaseBufferFontSize", { "persist": true }],
-      "cmd--": ["zed::DecreaseBufferFontSize", { "persist": true }],
-      "cmd-0": ["zed::ResetBufferFontSize", { "persist": true }],
+      "cmd-=": ["zed::IncreaseBufferFontSize", { "persist": false }],
+      "cmd-+": ["zed::IncreaseBufferFontSize", { "persist": false }],
+      "cmd--": ["zed::DecreaseBufferFontSize", { "persist": false }],
+      "cmd-0": ["zed::ResetBufferFontSize", { "persist": false }],
       "cmd-,": "zed::OpenSettings",
       "cmd-q": "zed::Quit",
       "cmd-h": "zed::Hide",

crates/theme/src/settings.rs 🔗

@@ -557,9 +557,10 @@ impl BufferLineHeight {
 impl ThemeSettings {
     /// Returns the buffer font size.
     pub fn buffer_font_size(&self, cx: &App) -> Pixels {
-        cx.try_global::<AdjustedBufferFontSize>()
-            .map_or(self.buffer_font_size, |size| size.0)
-            .max(MIN_FONT_SIZE)
+        let font_size = cx
+            .try_global::<AdjustedBufferFontSize>()
+            .map_or(self.buffer_font_size, |size| size.0);
+        clamp_font_size(font_size)
     }
 
     // TODO: Rename: `line_height` -> `buffer_line_height`
@@ -644,14 +645,16 @@ pub fn observe_buffer_font_size_adjustment<V: 'static>(
 
 /// Sets the adjusted buffer font size.
 pub fn adjusted_font_size(size: Pixels, cx: &App) -> Pixels {
-    if let Some(AdjustedBufferFontSize(adjusted_size)) = cx.try_global::<AdjustedBufferFontSize>() {
+    let adjusted_font_size = if let Some(AdjustedBufferFontSize(adjusted_size)) =
+        cx.try_global::<AdjustedBufferFontSize>()
+    {
         let buffer_font_size = ThemeSettings::get_global(cx).buffer_font_size;
         let delta = *adjusted_size - buffer_font_size;
         size + delta
     } else {
         size
-    }
-    .max(MIN_FONT_SIZE)
+    };
+    clamp_font_size(adjusted_font_size)
 }
 
 /// Returns the adjusted buffer font size.
@@ -669,8 +672,7 @@ pub fn adjust_buffer_font_size(cx: &mut App, mut f: impl FnMut(&mut Pixels)) {
         .map_or(buffer_font_size, |adjusted_size| adjusted_size.0);
 
     f(&mut adjusted_size);
-    adjusted_size = adjusted_size.max(MIN_FONT_SIZE);
-    cx.set_global(AdjustedBufferFontSize(adjusted_size));
+    cx.set_global(AdjustedBufferFontSize(clamp_font_size(adjusted_size)));
     cx.refresh_windows();
 }
 
@@ -715,8 +717,7 @@ pub fn adjust_ui_font_size(cx: &mut App, mut f: impl FnMut(&mut Pixels)) {
         .map_or(ui_font_size, |adjusted_size| adjusted_size.0);
 
     f(&mut adjusted_size);
-    adjusted_size = adjusted_size.max(MIN_FONT_SIZE);
-    cx.set_global(AdjustedUiFontSize(adjusted_size));
+    cx.set_global(AdjustedUiFontSize(clamp_font_size(adjusted_size)));
     cx.refresh_windows();
 }
 
@@ -733,6 +734,11 @@ pub fn reset_ui_font_size(cx: &mut App) {
     }
 }
 
+/// Ensures font size is within the valid range.
+pub fn clamp_font_size(size: Pixels) -> Pixels {
+    size.max(MIN_FONT_SIZE)
+}
+
 fn clamp_font_weight(weight: f32) -> FontWeight {
     FontWeight(weight.clamp(100., 950.))
 }

crates/zed/src/zed.rs 🔗

@@ -541,12 +541,16 @@ fn register_actions(
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::IncreaseUiFontSize, _window, cx| {
-                theme::adjust_ui_font_size(cx, |size| {
-                    *size += px(1.0);
-                });
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, cx| {
-                        let _ = settings.ui_font_size.insert(theme::get_ui_font_size(cx).0);
+                        let ui_font_size = theme::get_ui_font_size(cx) + px(1.0);
+                        let _ = settings
+                            .ui_font_size
+                            .insert(theme::clamp_font_size(ui_font_size).0);
+                    });
+                } else {
+                    theme::adjust_ui_font_size(cx, |size| {
+                        *size += px(1.0);
                     });
                 }
             }
@@ -554,12 +558,16 @@ fn register_actions(
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::DecreaseUiFontSize, _window, cx| {
-                theme::adjust_ui_font_size(cx, |size| {
-                    *size -= px(1.0);
-                });
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, cx| {
-                        let _ = settings.ui_font_size.insert(theme::get_ui_font_size(cx).0);
+                        let ui_font_size = theme::get_ui_font_size(cx) - px(1.0);
+                        let _ = settings
+                            .ui_font_size
+                            .insert(theme::clamp_font_size(ui_font_size).0);
+                    });
+                } else {
+                    theme::adjust_ui_font_size(cx, |size| {
+                        *size -= px(1.0);
                     });
                 }
             }
@@ -567,25 +575,28 @@ fn register_actions(
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::ResetUiFontSize, _window, cx| {
-                theme::reset_ui_font_size(cx);
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, _| {
-                        let _ = settings.ui_font_size.take();
+                        settings.ui_font_size = None;
                     });
+                } else {
+                    theme::reset_ui_font_size(cx);
                 }
             }
         })
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::IncreaseBufferFontSize, _window, cx| {
-                theme::adjust_buffer_font_size(cx, |size| {
-                    *size += px(1.0);
-                });
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, cx| {
+                        let buffer_font_size = theme::get_buffer_font_size(cx) + px(1.0);
                         let _ = settings
                             .buffer_font_size
-                            .insert(theme::get_buffer_font_size(cx).0);
+                            .insert(theme::clamp_font_size(buffer_font_size).0);
+                    });
+                } else {
+                    theme::adjust_buffer_font_size(cx, |size| {
+                        *size += px(1.0);
                     });
                 }
             }
@@ -593,14 +604,16 @@ fn register_actions(
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::DecreaseBufferFontSize, _window, cx| {
-                theme::adjust_buffer_font_size(cx, |size| {
-                    *size -= px(1.0);
-                });
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, cx| {
+                        let buffer_font_size = theme::get_buffer_font_size(cx) - px(1.0);
                         let _ = settings
                             .buffer_font_size
-                            .insert(theme::get_buffer_font_size(cx).0);
+                            .insert(theme::clamp_font_size(buffer_font_size).0);
+                    });
+                } else {
+                    theme::adjust_buffer_font_size(cx, |size| {
+                        *size -= px(1.0);
                     });
                 }
             }
@@ -608,11 +621,12 @@ fn register_actions(
         .register_action({
             let fs = app_state.fs.clone();
             move |_, action: &zed_actions::ResetBufferFontSize, _window, cx| {
-                theme::reset_buffer_font_size(cx);
                 if action.persist {
                     update_settings_file::<ThemeSettings>(fs.clone(), cx, move |settings, _| {
-                        let _ = settings.buffer_font_size.take();
+                        settings.buffer_font_size = None;
                     });
+                } else {
+                    theme::reset_buffer_font_size(cx);
                 }
             }
         })