settings ui: Use font picker element from onboarding instead of editor for font components (#39593)

Anthony Eid created

The font picker from onboarding is a lot friendlier to interact with and
makes it impossible for a user to select an invalid font from the
settings ui.

I also moved the font picker from the onboarding crate to the ui_input
crate

## New Look
<img width="1136" height="812" alt="image"
src="https://github.com/user-attachments/assets/7436682c-6a41-4860-a18b-13e15b8f3f31"
/>

Release Notes:

- N/A

Change summary

Cargo.lock                                    |   2 
crates/onboarding/src/editing_page.rs         | 183 --------------------
crates/onboarding/src/onboarding.rs           |   1 
crates/settings/src/settings_content/theme.rs |  14 +
crates/settings_ui/src/settings_ui.rs         |  75 ++++++++
crates/ui_input/Cargo.toml                    |   2 
crates/ui_input/src/font_picker.rs            | 176 ++++++++++++++++++++
crates/ui_input/src/ui_input.rs               |   2 
8 files changed, 272 insertions(+), 183 deletions(-)

Detailed changes

Cargo.lock πŸ”—

@@ -17134,8 +17134,10 @@ version = "0.1.0"
 dependencies = [
  "component",
  "editor",
+ "fuzzy",
  "gpui",
  "menu",
+ "picker",
  "settings",
  "theme",
  "ui",

crates/onboarding/src/editing_page.rs πŸ”—

@@ -2,20 +2,16 @@ use std::sync::Arc;
 
 use editor::{EditorSettings, ShowMinimap};
 use fs::Fs;
-use fuzzy::{StringMatch, StringMatchCandidate};
-use gpui::{
-    Action, AnyElement, App, Context, FontFeatures, IntoElement, Pixels, SharedString, Task, Window,
-};
+use gpui::{Action, App, FontFeatures, IntoElement, Pixels, SharedString, Window};
 use language::language_settings::{AllLanguageSettings, FormatOnSave};
-use picker::{Picker, PickerDelegate};
 use project::project_settings::ProjectSettings;
 use settings::{Settings as _, update_settings_file};
-use theme::{FontFamilyCache, FontFamilyName, ThemeSettings};
+use theme::{FontFamilyName, ThemeSettings};
 use ui::{
-    ButtonLike, ListItem, ListItemSpacing, PopoverMenu, SwitchField, ToggleButtonGroup,
-    ToggleButtonGroupStyle, ToggleButtonSimple, ToggleState, Tooltip, prelude::*,
+    ButtonLike, PopoverMenu, SwitchField, ToggleButtonGroup, ToggleButtonGroupStyle,
+    ToggleButtonSimple, ToggleState, Tooltip, prelude::*,
 };
-use ui_input::NumericStepper;
+use ui_input::{NumericStepper, font_picker};
 
 use crate::{ImportCursorSettings, ImportVsCodeSettings, SettingsImportState};
 
@@ -451,175 +447,6 @@ fn font_picker_stepper(
     })
 }
 
-type FontPicker = Picker<FontPickerDelegate>;
-
-pub struct FontPickerDelegate {
-    fonts: Vec<SharedString>,
-    filtered_fonts: Vec<StringMatch>,
-    selected_index: usize,
-    current_font: SharedString,
-    on_font_changed: Arc<dyn Fn(SharedString, &mut App) + 'static>,
-}
-
-impl FontPickerDelegate {
-    fn new(
-        current_font: SharedString,
-        on_font_changed: impl Fn(SharedString, &mut App) + 'static,
-        cx: &mut Context<FontPicker>,
-    ) -> Self {
-        let font_family_cache = FontFamilyCache::global(cx);
-
-        let fonts = font_family_cache
-            .try_list_font_families()
-            .unwrap_or_else(|| vec![current_font.clone()]);
-        let selected_index = fonts
-            .iter()
-            .position(|font| *font == current_font)
-            .unwrap_or(0);
-
-        let filtered_fonts = fonts
-            .iter()
-            .enumerate()
-            .map(|(index, font)| StringMatch {
-                candidate_id: index,
-                string: font.to_string(),
-                positions: Vec::new(),
-                score: 0.0,
-            })
-            .collect();
-
-        Self {
-            fonts,
-            filtered_fonts,
-            selected_index,
-            current_font,
-            on_font_changed: Arc::new(on_font_changed),
-        }
-    }
-}
-
-impl PickerDelegate for FontPickerDelegate {
-    type ListItem = AnyElement;
-
-    fn match_count(&self) -> usize {
-        self.filtered_fonts.len()
-    }
-
-    fn selected_index(&self) -> usize {
-        self.selected_index
-    }
-
-    fn set_selected_index(&mut self, ix: usize, _: &mut Window, cx: &mut Context<FontPicker>) {
-        self.selected_index = ix.min(self.filtered_fonts.len().saturating_sub(1));
-        cx.notify();
-    }
-
-    fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc<str> {
-        "Search fonts…".into()
-    }
-
-    fn update_matches(
-        &mut self,
-        query: String,
-        _window: &mut Window,
-        cx: &mut Context<FontPicker>,
-    ) -> Task<()> {
-        let fonts = self.fonts.clone();
-        let current_font = self.current_font.clone();
-
-        let matches: Vec<StringMatch> = if query.is_empty() {
-            fonts
-                .iter()
-                .enumerate()
-                .map(|(index, font)| StringMatch {
-                    candidate_id: index,
-                    string: font.to_string(),
-                    positions: Vec::new(),
-                    score: 0.0,
-                })
-                .collect()
-        } else {
-            let _candidates: Vec<StringMatchCandidate> = fonts
-                .iter()
-                .enumerate()
-                .map(|(id, font)| StringMatchCandidate::new(id, font.as_ref()))
-                .collect();
-
-            fonts
-                .iter()
-                .enumerate()
-                .filter(|(_, font)| font.to_lowercase().contains(&query.to_lowercase()))
-                .map(|(index, font)| StringMatch {
-                    candidate_id: index,
-                    string: font.to_string(),
-                    positions: Vec::new(),
-                    score: 0.0,
-                })
-                .collect()
-        };
-
-        let selected_index = if query.is_empty() {
-            fonts
-                .iter()
-                .position(|font| *font == current_font)
-                .unwrap_or(0)
-        } else {
-            matches
-                .iter()
-                .position(|m| fonts[m.candidate_id] == current_font)
-                .unwrap_or(0)
-        };
-
-        self.filtered_fonts = matches;
-        self.selected_index = selected_index;
-        cx.notify();
-
-        Task::ready(())
-    }
-
-    fn confirm(&mut self, _secondary: bool, _window: &mut Window, cx: &mut Context<FontPicker>) {
-        if let Some(font_match) = self.filtered_fonts.get(self.selected_index) {
-            let font = font_match.string.clone();
-            (self.on_font_changed)(font.into(), cx);
-        }
-    }
-
-    fn dismissed(&mut self, _window: &mut Window, _cx: &mut Context<FontPicker>) {}
-
-    fn render_match(
-        &self,
-        ix: usize,
-        selected: bool,
-        _window: &mut Window,
-        _cx: &mut Context<FontPicker>,
-    ) -> Option<Self::ListItem> {
-        let font_match = self.filtered_fonts.get(ix)?;
-
-        Some(
-            ListItem::new(ix)
-                .inset(true)
-                .spacing(ListItemSpacing::Sparse)
-                .toggle_state(selected)
-                .child(Label::new(font_match.string.clone()))
-                .into_any_element(),
-        )
-    }
-}
-
-fn font_picker(
-    current_font: SharedString,
-    on_font_changed: impl Fn(SharedString, &mut App) + 'static,
-    window: &mut Window,
-    cx: &mut Context<FontPicker>,
-) -> FontPicker {
-    let delegate = FontPickerDelegate::new(current_font, on_font_changed, cx);
-
-    Picker::uniform_list(delegate, window, cx)
-        .show_scrollbar(true)
-        .width(rems_from_px(210.))
-        .max_height(Some(rems(20.).into()))
-}
-
 fn render_popular_settings_section(
     tab_index: &mut isize,
     window: &mut Window,

crates/onboarding/src/onboarding.rs πŸ”—

@@ -17,6 +17,7 @@ use ui::{
     Avatar, ButtonLike, FluentBuilder, Headline, KeyBinding, ParentElement as _,
     StatefulInteractiveElement, Vector, VectorName, WithScrollbar, prelude::*, rems_from_px,
 };
+pub use ui_input::font_picker;
 use workspace::{
     AppState, Workspace, WorkspaceId,
     dock::DockPosition,

crates/settings/src/settings_content/theme.rs πŸ”—

@@ -1,5 +1,5 @@
 use collections::{HashMap, IndexMap};
-use gpui::{FontFallbacks, FontFeatures, FontStyle, FontWeight};
+use gpui::{FontFallbacks, FontFeatures, FontStyle, FontWeight, SharedString};
 use schemars::{JsonSchema, JsonSchema_repr};
 use serde::{Deserialize, Deserializer, Serialize};
 use serde_json::Value;
@@ -237,6 +237,18 @@ impl AsRef<str> for FontFamilyName {
     }
 }
 
+impl From<SharedString> for FontFamilyName {
+    fn from(value: SharedString) -> Self {
+        Self(Arc::from(value))
+    }
+}
+
+impl From<FontFamilyName> for SharedString {
+    fn from(value: FontFamilyName) -> Self {
+        SharedString::new(value.0)
+    }
+}
+
 impl From<String> for FontFamilyName {
     fn from(value: String) -> Self {
         Self(Arc::from(value))

crates/settings_ui/src/settings_ui.rs πŸ”—

@@ -24,8 +24,8 @@ use std::{
     sync::{Arc, atomic::AtomicBool},
 };
 use ui::{
-    ContextMenu, Divider, DropdownMenu, DropdownStyle, IconButtonShape, Switch, SwitchColor,
-    TreeViewItem, WithScrollbar, prelude::*,
+    ButtonLike, ContextMenu, Divider, DropdownMenu, DropdownStyle, IconButtonShape, PopoverMenu,
+    Switch, SwitchColor, TreeViewItem, WithScrollbar, prelude::*,
 };
 use ui_input::{NumericStepper, NumericStepperStyle, NumericStepperType};
 use util::{ResultExt as _, paths::PathStyle, rel_path::RelPath};
@@ -2758,9 +2758,9 @@ fn init_renderers(cx: &mut App) {
         .add_renderer::<CloseWindowWhenNoItems>(|settings_field, file, _, window, cx| {
             render_dropdown(*settings_field, file, window, cx)
         })
-        .add_renderer::<settings::FontFamilyName>(|settings_field, file, metadata, _, cx| {
+        .add_renderer::<settings::FontFamilyName>(|settings_field, file, _, window, cx| {
             // todo(settings_ui): We need to pass in a validator for this to ensure that users that type in invalid font names
-            render_text_field(settings_field.clone(), file, metadata, cx)
+            render_font_picker(settings_field.clone(), file, window, cx)
         })
         .add_renderer::<settings::BufferLineHeight>(|settings_field, file, _, window, cx| {
             // todo(settings_ui): Do we want to expose the custom variant of buffer line height?
@@ -3127,6 +3127,16 @@ impl SettingsUiFile {
 
 impl SettingsWindow {
     pub fn new(window: &mut Window, cx: &mut Context<Self>) -> Self {
+        let font_family_cache = theme::FontFamilyCache::global(cx);
+
+        cx.spawn(async move |this, cx| {
+            font_family_cache.prefetch(cx).await;
+            this.update(cx, |_, cx| {
+                cx.notify();
+            })
+        })
+        .detach();
+
         let current_file = SettingsUiFile::User;
         let search_bar = cx.new(|cx| {
             let mut editor = Editor::single_line(window, cx);
@@ -3770,6 +3780,63 @@ fn render_toggle_button<B: Into<bool> + From<bool> + Copy>(
         .into_any_element()
 }
 
+fn render_font_picker(
+    field: SettingField<settings::FontFamilyName>,
+    file: SettingsUiFile,
+    window: &mut Window,
+    cx: &mut App,
+) -> AnyElement {
+    let current_value = SettingsStore::global(cx)
+        .get_value_from_file(file.to_settings(), field.pick)
+        .1
+        .clone();
+
+    let font_picker = cx.new(|cx| {
+        ui_input::font_picker(
+            current_value.clone().into(),
+            move |font_name, cx| {
+                update_settings_file(file.clone(), cx, move |settings, _cx| {
+                    *(field.pick_mut)(settings) = Some(font_name.into());
+                })
+                .log_err(); // todo(settings_ui) don't log err
+            },
+            window,
+            cx,
+        )
+    });
+
+    div()
+        .child(
+            PopoverMenu::new("font-picker")
+                .menu(move |_window, _cx| Some(font_picker.clone()))
+                .trigger(
+                    ButtonLike::new("font-family-button")
+                        .style(ButtonStyle::Outlined)
+                        .size(ButtonSize::Medium)
+                        .full_width()
+                        .child(
+                            h_flex()
+                                .w_full()
+                                .justify_between()
+                                .child(Label::new(current_value))
+                                .child(
+                                    Icon::new(IconName::ChevronUpDown)
+                                        .color(Color::Muted)
+                                        .size(IconSize::XSmall),
+                                ),
+                        ),
+                )
+                .full_width(true)
+                .anchor(gpui::Corner::TopLeft)
+                .offset(gpui::Point {
+                    x: px(0.0),
+                    y: px(4.0),
+                })
+                .with_handle(ui::PopoverMenuHandle::default()),
+        )
+        .into_any_element()
+}
+
 fn render_numeric_stepper<T: NumericStepperType + Send + Sync>(
     field: SettingField<T>,
     file: SettingsUiFile,

crates/ui_input/Cargo.toml πŸ”—

@@ -14,8 +14,10 @@ path = "src/ui_input.rs"
 [dependencies]
 component.workspace = true
 editor.workspace = true
+fuzzy.workspace = true
 gpui.workspace = true
 menu.workspace = true
+picker.workspace = true
 settings.workspace = true
 theme.workspace = true
 ui.workspace = true

crates/ui_input/src/font_picker.rs πŸ”—

@@ -0,0 +1,176 @@
+use std::sync::Arc;
+
+use fuzzy::{StringMatch, StringMatchCandidate};
+use gpui::{AnyElement, App, Context, SharedString, Task, Window};
+use picker::{Picker, PickerDelegate};
+use theme::FontFamilyCache;
+use ui::{ListItem, ListItemSpacing, prelude::*};
+
+type FontPicker = Picker<FontPickerDelegate>;
+
+pub struct FontPickerDelegate {
+    fonts: Vec<SharedString>,
+    filtered_fonts: Vec<StringMatch>,
+    selected_index: usize,
+    current_font: SharedString,
+    on_font_changed: Arc<dyn Fn(SharedString, &mut App) + 'static>,
+}
+
+impl FontPickerDelegate {
+    fn new(
+        current_font: SharedString,
+        on_font_changed: impl Fn(SharedString, &mut App) + 'static,
+        cx: &mut Context<FontPicker>,
+    ) -> Self {
+        let font_family_cache = FontFamilyCache::global(cx);
+
+        let fonts = font_family_cache
+            .try_list_font_families()
+            .unwrap_or_else(|| vec![current_font.clone()]);
+        let selected_index = fonts
+            .iter()
+            .position(|font| *font == current_font)
+            .unwrap_or(0);
+
+        let filtered_fonts = fonts
+            .iter()
+            .enumerate()
+            .map(|(index, font)| StringMatch {
+                candidate_id: index,
+                string: font.to_string(),
+                positions: Vec::new(),
+                score: 0.0,
+            })
+            .collect();
+
+        Self {
+            fonts,
+            filtered_fonts,
+            selected_index,
+            current_font,
+            on_font_changed: Arc::new(on_font_changed),
+        }
+    }
+}
+
+impl PickerDelegate for FontPickerDelegate {
+    type ListItem = AnyElement;
+
+    fn match_count(&self) -> usize {
+        self.filtered_fonts.len()
+    }
+
+    fn selected_index(&self) -> usize {
+        self.selected_index
+    }
+
+    fn set_selected_index(&mut self, ix: usize, _: &mut Window, cx: &mut Context<FontPicker>) {
+        self.selected_index = ix.min(self.filtered_fonts.len().saturating_sub(1));
+        cx.notify();
+    }
+
+    fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc<str> {
+        "Search fonts…".into()
+    }
+
+    fn update_matches(
+        &mut self,
+        query: String,
+        _window: &mut Window,
+        cx: &mut Context<FontPicker>,
+    ) -> Task<()> {
+        let fonts = self.fonts.clone();
+        let current_font = self.current_font.clone();
+
+        let matches: Vec<StringMatch> = if query.is_empty() {
+            fonts
+                .iter()
+                .enumerate()
+                .map(|(index, font)| StringMatch {
+                    candidate_id: index,
+                    string: font.to_string(),
+                    positions: Vec::new(),
+                    score: 0.0,
+                })
+                .collect()
+        } else {
+            let _candidates: Vec<StringMatchCandidate> = fonts
+                .iter()
+                .enumerate()
+                .map(|(id, font)| StringMatchCandidate::new(id, font.as_ref()))
+                .collect();
+
+            fonts
+                .iter()
+                .enumerate()
+                .filter(|(_, font)| font.to_lowercase().contains(&query.to_lowercase()))
+                .map(|(index, font)| StringMatch {
+                    candidate_id: index,
+                    string: font.to_string(),
+                    positions: Vec::new(),
+                    score: 0.0,
+                })
+                .collect()
+        };
+
+        let selected_index = if query.is_empty() {
+            fonts
+                .iter()
+                .position(|font| *font == current_font)
+                .unwrap_or(0)
+        } else {
+            matches
+                .iter()
+                .position(|m| fonts[m.candidate_id] == current_font)
+                .unwrap_or(0)
+        };
+
+        self.filtered_fonts = matches;
+        self.selected_index = selected_index;
+        cx.notify();
+
+        Task::ready(())
+    }
+
+    fn confirm(&mut self, _secondary: bool, _window: &mut Window, cx: &mut Context<FontPicker>) {
+        if let Some(font_match) = self.filtered_fonts.get(self.selected_index) {
+            let font = font_match.string.clone();
+            (self.on_font_changed)(font.into(), cx);
+        }
+    }
+
+    fn dismissed(&mut self, _window: &mut Window, _cx: &mut Context<FontPicker>) {}
+
+    fn render_match(
+        &self,
+        ix: usize,
+        selected: bool,
+        _window: &mut Window,
+        _cx: &mut Context<FontPicker>,
+    ) -> Option<Self::ListItem> {
+        let font_match = self.filtered_fonts.get(ix)?;
+
+        Some(
+            ListItem::new(ix)
+                .inset(true)
+                .spacing(ListItemSpacing::Sparse)
+                .toggle_state(selected)
+                .child(Label::new(font_match.string.clone()))
+                .into_any_element(),
+        )
+    }
+}
+
+pub fn font_picker(
+    current_font: SharedString,
+    on_font_changed: impl Fn(SharedString, &mut App) + 'static,
+    window: &mut Window,
+    cx: &mut Context<FontPicker>,
+) -> FontPicker {
+    let delegate = FontPickerDelegate::new(current_font, on_font_changed, cx);
+
+    Picker::uniform_list(delegate, window, cx)
+        .show_scrollbar(true)
+        .width(rems_from_px(210.))
+        .max_height(Some(rems(20.).into()))
+}

crates/ui_input/src/ui_input.rs πŸ”—

@@ -4,10 +4,12 @@
 //!
 //! It can't be located in the `ui` crate because it depends on `editor`.
 //!
+mod font_picker;
 mod numeric_stepper;
 
 use component::{example_group, single_example};
 use editor::{Editor, EditorElement, EditorStyle};
+pub use font_picker::*;
 use gpui::{App, Entity, FocusHandle, Focusable, FontStyle, Hsla, Length, TextStyle};
 pub use numeric_stepper::*;
 use settings::Settings;