From ea60a7b1721aa9d0ef04240d95cdb496e3b862b3 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Mon, 6 Oct 2025 13:04:43 -0400 Subject: [PATCH] settings ui: Use font picker element from onboarding instead of editor for font components (#39593) 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 image Release Notes: - N/A --- 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(-) create mode 100644 crates/ui_input/src/font_picker.rs diff --git a/Cargo.lock b/Cargo.lock index 0226b09bbfbe11a8eaf701752e8fb8d9eb074bc3..de38de388979ecc1aa2db86346c4fae7453917bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17134,8 +17134,10 @@ version = "0.1.0" dependencies = [ "component", "editor", + "fuzzy", "gpui", "menu", + "picker", "settings", "theme", "ui", diff --git a/crates/onboarding/src/editing_page.rs b/crates/onboarding/src/editing_page.rs index 0aa1579f582c81ea002156a2193f5c9c70981f88..7b965facb5e8cf8e06df4fccdecf7894c51610d5 100644 --- a/crates/onboarding/src/editing_page.rs +++ b/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; - -pub struct FontPickerDelegate { - fonts: Vec, - filtered_fonts: Vec, - selected_index: usize, - current_font: SharedString, - on_font_changed: Arc, -} - -impl FontPickerDelegate { - fn new( - current_font: SharedString, - on_font_changed: impl Fn(SharedString, &mut App) + 'static, - cx: &mut Context, - ) -> 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) { - 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 { - "Search fonts…".into() - } - - fn update_matches( - &mut self, - query: String, - _window: &mut Window, - cx: &mut Context, - ) -> Task<()> { - let fonts = self.fonts.clone(); - let current_font = self.current_font.clone(); - - let matches: Vec = 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 = 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) { - 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) {} - - fn render_match( - &self, - ix: usize, - selected: bool, - _window: &mut Window, - _cx: &mut Context, - ) -> Option { - 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 { - 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, diff --git a/crates/onboarding/src/onboarding.rs b/crates/onboarding/src/onboarding.rs index 25961c4e78366369b146d8a57b4f3f6022ce1009..ab47eef8b75f632936f8272d5f608d713956599f 100644 --- a/crates/onboarding/src/onboarding.rs +++ b/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, diff --git a/crates/settings/src/settings_content/theme.rs b/crates/settings/src/settings_content/theme.rs index 9e02f23a29fb0ed79dd73b13ac820bc0760bdf4b..85ca09443bfd9fc701fafa81b40585fa8cc6a1fb 100644 --- a/crates/settings/src/settings_content/theme.rs +++ b/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 for FontFamilyName { } } +impl From for FontFamilyName { + fn from(value: SharedString) -> Self { + Self(Arc::from(value)) + } +} + +impl From for SharedString { + fn from(value: FontFamilyName) -> Self { + SharedString::new(value.0) + } +} + impl From for FontFamilyName { fn from(value: String) -> Self { Self(Arc::from(value)) diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 49afae873bb5e608d14a16f699037bb05abd29d9..c2e3b656028527873593210b887b6995588d646b 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/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::(|settings_field, file, _, window, cx| { render_dropdown(*settings_field, file, window, cx) }) - .add_renderer::(|settings_field, file, metadata, _, cx| { + .add_renderer::(|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_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 { + 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 + From + Copy>( .into_any_element() } +fn render_font_picker( + field: SettingField, + 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( field: SettingField, file: SettingsUiFile, diff --git a/crates/ui_input/Cargo.toml b/crates/ui_input/Cargo.toml index 97f250c6ae97b82d30814f1c90bfd2002427b0da..0f107e42c382d55c2e2d6725336bc3af569a222d 100644 --- a/crates/ui_input/Cargo.toml +++ b/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 diff --git a/crates/ui_input/src/font_picker.rs b/crates/ui_input/src/font_picker.rs new file mode 100644 index 0000000000000000000000000000000000000000..f5ebd41360fc5b035ca506c481f2c4552efc2fd1 --- /dev/null +++ b/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; + +pub struct FontPickerDelegate { + fonts: Vec, + filtered_fonts: Vec, + selected_index: usize, + current_font: SharedString, + on_font_changed: Arc, +} + +impl FontPickerDelegate { + fn new( + current_font: SharedString, + on_font_changed: impl Fn(SharedString, &mut App) + 'static, + cx: &mut Context, + ) -> 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) { + 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 { + "Search fonts…".into() + } + + fn update_matches( + &mut self, + query: String, + _window: &mut Window, + cx: &mut Context, + ) -> Task<()> { + let fonts = self.fonts.clone(); + let current_font = self.current_font.clone(); + + let matches: Vec = 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 = 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) { + 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) {} + + fn render_match( + &self, + ix: usize, + selected: bool, + _window: &mut Window, + _cx: &mut Context, + ) -> Option { + 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 { + 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())) +} diff --git a/crates/ui_input/src/ui_input.rs b/crates/ui_input/src/ui_input.rs index f2b9d8e195e23f2ebbd5c6e3f9b7c7ddaed7f73f..0e3baec69b57469317834d0c50365c136dfebf75 100644 --- a/crates/ui_input/src/ui_input.rs +++ b/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;