From e6f06f14fdb3c2d7bc766fc58506e73bb1343764 Mon Sep 17 00:00:00 2001 From: Anthony Date: Tue, 19 Aug 2025 18:50:12 -0400 Subject: [PATCH] Refactor Numeric stepper to use generics Users now pass in an entity that can observes T for changes and can react to those changes Co-authored-by: Mikayla Maki --- crates/editor/src/editor_settings_controls.rs | 2 +- crates/gpui/src/window.rs | 4 +- crates/onboarding/src/editing_page.rs | 239 ++++++------ .../src/appearance_settings_controls.rs | 41 ++- crates/ui_input/src/numeric_stepper.rs | 340 ++++++++++-------- .../src/derive_register_component.rs | 1 + 6 files changed, 338 insertions(+), 289 deletions(-) diff --git a/crates/editor/src/editor_settings_controls.rs b/crates/editor/src/editor_settings_controls.rs index 3ae08c7821..2562a94d3d 100644 --- a/crates/editor/src/editor_settings_controls.rs +++ b/crates/editor/src/editor_settings_controls.rs @@ -143,7 +143,7 @@ impl RenderOnce for BufferFontSizeControl { h_flex() .gap_2() .child(Icon::new(IconName::FontSize)) - .child(div()) // todo!(Numeric stepper was here) + .child(div()) // TODO: Re-evaluate this whole crate once settings UI is complete } } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 62aeb0df11..2597c001d2 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2504,7 +2504,7 @@ impl Window { &mut self, key: impl Into, cx: &mut App, - init: impl FnOnce(&mut Self, &mut App) -> S, + init: impl FnOnce(&mut Self, &mut Context) -> S, ) -> Entity { let current_view = self.current_view(); self.with_global_id(key.into(), |global_id, window| { @@ -2537,7 +2537,7 @@ impl Window { pub fn use_state( &mut self, cx: &mut App, - init: impl FnOnce(&mut Self, &mut App) -> S, + init: impl FnOnce(&mut Self, &mut Context) -> S, ) -> Entity { self.use_keyed_state( ElementId::CodeLocation(*core::panic::Location::caller()), diff --git a/crates/onboarding/src/editing_page.rs b/crates/onboarding/src/editing_page.rs index 9dfd9356cc..06d20c83be 100644 --- a/crates/onboarding/src/editing_page.rs +++ b/crates/onboarding/src/editing_page.rs @@ -15,7 +15,6 @@ use ui::{ ButtonLike, ListItem, ListItemSpacing, PopoverMenu, SwitchField, ToggleButtonGroup, ToggleButtonGroupStyle, ToggleButtonSimple, ToggleState, Tooltip, prelude::*, }; -use ui_input::NumericStepper; use crate::{ImportCursorSettings, ImportVsCodeSettings, SettingsImportState}; @@ -110,7 +109,7 @@ fn write_ui_font_family(font: SharedString, cx: &mut App) { }); } -fn write_ui_font_size(size: Pixels, cx: &mut App) { +fn _write_ui_font_size(size: Pixels, cx: &mut App) { let fs = ::global(cx); update_settings_file::(fs, cx, move |theme_settings, _| { @@ -118,7 +117,7 @@ fn write_ui_font_size(size: Pixels, cx: &mut App) { }); } -fn write_buffer_font_size(size: Pixels, cx: &mut App) { +fn _write_buffer_font_size(size: Pixels, cx: &mut App) { let fs = ::global(cx); update_settings_file::(fs, cx, move |theme_settings, _| { @@ -278,10 +277,10 @@ fn render_font_customization_section( cx: &mut App, ) -> impl IntoElement { let theme_settings = ThemeSettings::get_global(cx); - let ui_font_size = theme_settings.ui_font_size(cx); + // let ui_font_size = theme_settings.ui_font_size(cx); let ui_font_family = theme_settings.ui_font.family.clone(); let buffer_font_family = theme_settings.buffer_font.family.clone(); - let buffer_font_size = theme_settings.buffer_font_size(cx); + // let buffer_font_size = theme_settings.buffer_font_size(cx); let ui_font_picker = cx.new(|cx| font_picker(ui_font_family.clone(), write_ui_font_family, window, cx)); @@ -307,67 +306,62 @@ fn render_font_customization_section( .gap_1() .child(Label::new("UI Font")) .child( - h_flex() - .w_full() - .justify_between() - .gap_2() - .child( - PopoverMenu::new("ui-font-picker") - .menu({ - let ui_font_picker = ui_font_picker.clone(); - move |_window, _cx| Some(ui_font_picker.clone()) - }) - .trigger( - ButtonLike::new("ui-font-family-button") - .style(ButtonStyle::Outlined) - .size(ButtonSize::Medium) - .full_width() - .tab_index({ - *tab_index += 1; - *tab_index - 1 - }) - .child( - h_flex() - .w_full() - .justify_between() - .child(Label::new(ui_font_family)) - .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_font_handle), - ) - .child( - NumericStepper::new( - "ui-font-size", - ui_font_size.to_string(), - move |size, cx| { - write_ui_font_size(Pixels::from(size), cx); - }, - move |_, _, cx| { - write_ui_font_size(ui_font_size - px(1.), cx); - }, - move |_, _, cx| { - write_ui_font_size(ui_font_size + px(1.), cx); - }, - window, - cx, + h_flex().w_full().justify_between().gap_2().child( + PopoverMenu::new("ui-font-picker") + .menu({ + let ui_font_picker = ui_font_picker.clone(); + move |_window, _cx| Some(ui_font_picker.clone()) + }) + .trigger( + ButtonLike::new("ui-font-family-button") + .style(ButtonStyle::Outlined) + .size(ButtonSize::Medium) + .full_width() + .tab_index({ + *tab_index += 1; + *tab_index - 1 + }) + .child( + h_flex() + .w_full() + .justify_between() + .child(Label::new(ui_font_family)) + .child( + Icon::new(IconName::ChevronUpDown) + .color(Color::Muted) + .size(IconSize::XSmall), + ), + ), ) - .style(ui_input::NumericStepperStyle::Outlined) - .tab_index({ - *tab_index += 2; - *tab_index - 2 - }), - ), + .full_width(true) + .anchor(gpui::Corner::TopLeft) + .offset(gpui::Point { + x: px(0.0), + y: px(4.0), + }) + .with_handle(ui_font_handle), + ), // .child( + // NumericStepper::new( + // "ui-font-size", + // ui_font_size.to_string(), + // move |size, cx| { + // write_ui_font_size(Pixels::from(size), cx); + // }, + // move |_, _, cx| { + // write_ui_font_size(ui_font_size - px(1.), cx); + // }, + // move |_, _, cx| { + // write_ui_font_size(ui_font_size + px(1.), cx); + // }, + // window, + // cx, + // ) + // .style(ui_input::NumericStepperStyle::Outlined) + // .tab_index({ + // *tab_index += 2; + // *tab_index - 2 + // }), + // ), ), ) .child( @@ -376,67 +370,62 @@ fn render_font_customization_section( .gap_1() .child(Label::new("Editor Font")) .child( - h_flex() - .w_full() - .justify_between() - .gap_2() - .child( - PopoverMenu::new("buffer-font-picker") - .menu({ - let buffer_font_picker = buffer_font_picker.clone(); - move |_window, _cx| Some(buffer_font_picker.clone()) - }) - .trigger( - ButtonLike::new("buffer-font-family-button") - .style(ButtonStyle::Outlined) - .size(ButtonSize::Medium) - .full_width() - .tab_index({ - *tab_index += 1; - *tab_index - 1 - }) - .child( - h_flex() - .w_full() - .justify_between() - .child(Label::new(buffer_font_family)) - .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(buffer_font_handle), - ) - .child( - NumericStepper::new( - "buffer-font-size", - buffer_font_size.to_string(), - move |size, cx| { - write_buffer_font_size(Pixels::from(size), cx); - }, - move |_, _, cx| { - write_buffer_font_size(buffer_font_size - px(1.), cx); - }, - move |_, _, cx| { - write_buffer_font_size(buffer_font_size + px(1.), cx); - }, - window, - cx, + h_flex().w_full().justify_between().gap_2().child( + PopoverMenu::new("buffer-font-picker") + .menu({ + let buffer_font_picker = buffer_font_picker.clone(); + move |_window, _cx| Some(buffer_font_picker.clone()) + }) + .trigger( + ButtonLike::new("buffer-font-family-button") + .style(ButtonStyle::Outlined) + .size(ButtonSize::Medium) + .full_width() + .tab_index({ + *tab_index += 1; + *tab_index - 1 + }) + .child( + h_flex() + .w_full() + .justify_between() + .child(Label::new(buffer_font_family)) + .child( + Icon::new(IconName::ChevronUpDown) + .color(Color::Muted) + .size(IconSize::XSmall), + ), + ), ) - .style(ui_input::NumericStepperStyle::Outlined) - .tab_index({ - *tab_index += 2; - *tab_index - 2 - }), - ), + .full_width(true) + .anchor(gpui::Corner::TopLeft) + .offset(gpui::Point { + x: px(0.0), + y: px(4.0), + }) + .with_handle(buffer_font_handle), + ), // .child( todo! + // NumericStepper::new( + // "buffer-font-size", + // buffer_font_size.to_string(), + // move |size, cx| { + // write_buffer_font_size(Pixels::from(size), cx); + // }, + // move |_, _, cx| { + // write_buffer_font_size(buffer_font_size - px(1.), cx); + // }, + // move |_, _, cx| { + // write_buffer_font_size(buffer_font_size + px(1.), cx); + // }, + // window, + // cx, + // ) + // .style(ui_input::NumericStepperStyle::Outlined) + // .tab_index({ + // *tab_index += 2; + // *tab_index - 2 + // }), + // ), ), ) } diff --git a/crates/settings_ui/src/appearance_settings_controls.rs b/crates/settings_ui/src/appearance_settings_controls.rs index e3c04bfa43..f5b1bc5702 100644 --- a/crates/settings_ui/src/appearance_settings_controls.rs +++ b/crates/settings_ui/src/appearance_settings_controls.rs @@ -9,7 +9,7 @@ use ui::{ CheckboxWithLabel, ContextMenu, DropdownMenu, SettingsContainer, SettingsGroup, ToggleButton, prelude::*, }; -use ui_input::NumericStepper; +// use ui_input::NumericStepper; #[derive(IntoElement)] pub struct AppearanceSettingsControls {} @@ -255,27 +255,26 @@ impl EditableSettingControl for UiFontSizeControl { } impl RenderOnce for UiFontSizeControl { - fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let value = Self::read(cx); + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + // let value = Self::read(cx); - h_flex() - .gap_2() - .child(Icon::new(IconName::FontSize)) - .child(NumericStepper::new( - "ui-font-size", - value.to_string(), - move |size, cx| { - Self::write(Pixels::from(size), cx); - }, - move |_, _, cx| { - Self::write(value - px(1.), cx); - }, - move |_, _, cx| { - Self::write(value + px(1.), cx); - }, - window, - cx, - )) + h_flex().gap_2().child(Icon::new(IconName::FontSize)) + // TODO: Re-evaluate this whole crate once settings UI project starts + // .child(NumericStepper::new( + // "ui-font-size", + // value.to_string(), + // move |size, cx| { + // Self::write(Pixels::from(size), cx); + // }, + // move |_, _, cx| { + // Self::write(value - px(1.), cx); + // }, + // move |_, _, cx| { + // Self::write(value + px(1.), cx); + // }, + // window, + // cx, + // )) } } diff --git a/crates/ui_input/src/numeric_stepper.rs b/crates/ui_input/src/numeric_stepper.rs index 9c7f1a7c67..f04d1ed759 100644 --- a/crates/ui_input/src/numeric_stepper.rs +++ b/crates/ui_input/src/numeric_stepper.rs @@ -1,5 +1,11 @@ +use std::{ + fmt::Display, + ops::{Add, Sub}, + str::FromStr, +}; + use editor::Editor; -use gpui::{ClickEvent, Entity, Focusable}; +use gpui::{ClickEvent, Entity, FocusHandle, Focusable, Modifiers}; use ui::{IconButtonShape, prelude::*}; @@ -17,94 +23,128 @@ pub enum NumericStepperMode { Edit, } -#[derive(IntoElement, RegisterComponent)] -pub struct NumericStepper { +pub trait NumericStepperType: + Display + Add + Sub + Copy + Clone + Sized + FromStr + 'static +{ + fn default_format(value: &Self) -> String { + format!("{}", value) + } + fn default_step() -> Self; + fn large_step() -> Self; + fn small_step() -> Self; +} + +macro_rules! impl_numeric_stepper_int { + ($type:ident) => { + impl NumericStepperType for $type { + fn default_step() -> Self { + 1 + } + + fn large_step() -> Self { + 10 + } + + fn small_step() -> Self { + 1 + } + } + }; +} + +macro_rules! impl_numeric_stepper_float { + ($type:ident) => { + impl NumericStepperType for $type { + fn default_format(value: &Self) -> String { + format!("{:.2}", value) + } + + fn default_step() -> Self { + 1.0 + } + + fn large_step() -> Self { + 10.0 + } + + fn small_step() -> Self { + 0.1 + } + } + }; +} + +impl_numeric_stepper_float!(f32); +impl_numeric_stepper_float!(f64); +impl_numeric_stepper_int!(isize); +impl_numeric_stepper_int!(usize); +impl_numeric_stepper_int!(i32); +impl_numeric_stepper_int!(u32); +impl_numeric_stepper_int!(i64); +impl_numeric_stepper_int!(u64); + +// TODO: Add a new register component macro to support a specific type when using generics +pub struct NumericStepper { id: ElementId, - value: SharedString, + value: Entity, style: NumericStepperStyle, - input_field: Entity, + focus_handle: FocusHandle, mode: Entity, - set_value_to: Box, - on_decrement: Box, - on_increment: Box, - /// Whether to reserve space for the reset button. - reserve_space_for_reset: bool, + format: Box String>, + large_step: T, + small_step: T, + step: T, on_reset: Option>, tab_index: Option, } -impl NumericStepper { +impl NumericStepper { pub fn new( id: impl Into, - value: impl Into, - set_value_to: impl Fn(usize, &mut App) + 'static, - on_decrement: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static, - on_increment: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static, + value: Entity, window: &mut Window, cx: &mut App, ) -> Self { let id = id.into(); - let value = value.into(); - - let (input_field, mode) = window.with_global_id(id.clone(), |global_id, window| { - // todo! Make sure that using this api is inline and appropriate with the codebase - window.with_element_state::<(Entity, Entity), _>( - global_id, - |mut editor, window| { - let state = editor - .get_or_insert_with(|| { - let mode = cx.new(|_| NumericStepperMode::default()); - let weak_mode = mode.downgrade(); - let editor = cx.new(|cx| { - let editor = Editor::single_line(window, cx); - - cx.on_focus_out( - &editor.focus_handle(cx), - window, - move |this, _, window, cx| { - this.clear(window, cx); - - weak_mode - .update(cx, |mode, _| *mode = NumericStepperMode::Read) - .ok(); - }, - ) - .detach(); - - editor - }); - - (editor, mode) - }) - .clone(); - - (state.clone(), state) - }, - ) - }); + let mode = window.use_state(cx, |_, _| NumericStepperMode::default()); Self { id, - value, - input_field, + focus_handle: cx.focus_handle(), mode, - set_value_to: Box::new(set_value_to), - on_decrement: Box::new(on_decrement), - on_increment: Box::new(on_increment), + value, style: NumericStepperStyle::default(), - reserve_space_for_reset: false, + format: Box::new(T::default_format), + large_step: T::large_step(), + step: T::default_step(), + small_step: T::small_step(), on_reset: None, tab_index: None, } } - pub fn style(mut self, style: NumericStepperStyle) -> Self { - self.style = style; + pub fn format(mut self, format: impl FnOnce(&T) -> String + 'static) -> Self { + self.format = Box::new(format); self } - pub fn reserve_space_for_reset(mut self, reserve_space_for_reset: bool) -> Self { - self.reserve_space_for_reset = reserve_space_for_reset; + pub fn small_step(mut self, step: T) -> Self { + self.small_step = step; + self + } + + pub fn normal_step(mut self, step: T) -> Self { + self.step = step; + self + } + + pub fn large_step(mut self, step: T) -> Self { + self.large_step = step; + self + } + + pub fn style(mut self, style: NumericStepperStyle) -> Self { + self.style = style; self } @@ -122,7 +162,15 @@ impl NumericStepper { } } -impl RenderOnce for NumericStepper { +impl IntoElement for NumericStepper { + type Element = gpui::Component; + + fn into_element(self) -> Self::Element { + gpui::Component::new(self) + } +} + +impl RenderOnce for NumericStepper { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let shape = IconButtonShape::Square; let icon_size = IconSize::Small; @@ -130,31 +178,36 @@ impl RenderOnce for NumericStepper { let is_outlined = matches!(self.style, NumericStepperStyle::Outlined); let mut tab_index = self.tab_index; + let get_step = { + let large_step = self.large_step; + let step = self.step; + let small_step = self.small_step; + move |modifiers: Modifiers| -> T { + if modifiers.shift { + large_step + } else if modifiers.alt { + small_step + } else { + step + } + } + }; + h_flex() .id(self.id.clone()) + .track_focus(&self.focus_handle) .gap_1() - .map(|element| { - if let Some(on_reset) = self.on_reset { - element.child( - IconButton::new("reset", IconName::RotateCcw) - .shape(shape) - .icon_size(icon_size) - .when_some(tab_index.as_mut(), |this, tab_index| { - *tab_index += 1; - this.tab_index(*tab_index - 1) - }) - .on_click(on_reset), - ) - } else if self.reserve_space_for_reset { - element.child( - h_flex() - .size(icon_size.square(window, cx)) - .flex_none() - .into_any_element(), - ) - } else { - element - } + .when_some(self.on_reset, |this, on_reset| { + this.child( + IconButton::new("reset", IconName::RotateCcw) + .shape(shape) + .icon_size(icon_size) + .when_some(tab_index.as_mut(), |this, tab_index| { + *tab_index += 1; + this.tab_index(*tab_index - 1) + }) + .on_click(on_reset), + ) }) .child( h_flex() @@ -171,6 +224,15 @@ impl RenderOnce for NumericStepper { } }) .map(|decrement| { + let decrement_handler = { + let value = self.value.clone(); + move |click: &ClickEvent, _: &mut Window, cx: &mut App| { + let step = get_step(click.modifiers()); + let current_value = *value.read(cx); + value.write(cx, current_value - step); + } + }; + if is_outlined { decrement.child( h_flex() @@ -188,7 +250,7 @@ impl RenderOnce for NumericStepper { style.bg(cx.theme().colors().element_hover) }) }) - .on_click(self.on_decrement), + .on_click(decrement_handler), ) } else { decrement.child( @@ -199,64 +261,66 @@ impl RenderOnce for NumericStepper { *tab_index += 1; this.tab_index(*tab_index - 1) }) - .on_click(self.on_decrement), + .on_click(decrement_handler), ) } }) - .child(if matches!(self.mode.read(cx), NumericStepperMode::Read) { - div() - .id(SharedString::new(format!( - "numeric_stepper_label{}", - &self.id, - ))) - .child(Label::new(self.value).mx_3()) + .child(match *self.mode.read(cx) { + NumericStepperMode::Read => div() + .id("numeric_stepper_label") + .child(Label::new((self.format)(self.value.read(cx))).mx_3()) .on_click({ - let mode = self.mode.downgrade(); - let input_field_focus_handle = self.input_field.focus_handle(cx); + let mode = self.mode.clone(); - move |click, window, cx| { + move |click, _, cx| { if click.click_count() == 2 { - mode.update(cx, |mode, _| { - *mode = NumericStepperMode::Edit; - }) - .ok(); - - window.focus(&input_field_focus_handle); + mode.write(cx, NumericStepperMode::Edit); } } }) - .into_any_element() - } else { - div() - .child(self.input_field.clone()) - .child("todo!(This should be removed. It's only here to get input_field to render correctly)") + .into_any_element(), + NumericStepperMode::Edit => div() + .child(window.use_state(cx, { + |window, cx| { + let mut editor = Editor::single_line(window, cx); + editor.set_text(format!("{}", self.value.read(cx)), window, cx); + cx.on_focus_out(&editor.focus_handle(cx), window, { + let mode = self.mode.clone(); + let value = self.value.clone(); + move |this, _, _window, cx| { + if let Ok(new_value) = this.text(cx).parse::() { + value.write(cx, new_value); + }; + mode.write(cx, NumericStepperMode::Read); + } + }) + .detach(); + + window.focus(&editor.focus_handle(cx)); + + editor + } + })) .on_action::({ - let input_field = self.input_field.downgrade(); - let mode = self.mode.downgrade(); - let set_value = self.set_value_to; - - move |_, _, cx| { - input_field - .update(cx, |input_field, cx| { - if let Some(number) = - input_field.text(cx).parse::().ok() - { - set_value(number, cx); - - mode.update(cx, |mode, _| { - *mode = NumericStepperMode::Read - }) - .ok(); - } - }) - .ok(); + let focus = self.focus_handle.clone(); + move |_, window, _| { + window.focus(&focus); } }) .w_full() .mx_3() - .into_any_element() + .into_any_element(), }) .map(|increment| { + let increment_handler = { + let value = self.value.clone(); + move |click: &ClickEvent, _: &mut Window, cx: &mut App| { + let step = get_step(click.modifiers()); + let current_value = *value.read(cx); + value.write(cx, current_value + step); + } + }; + if is_outlined { increment.child( h_flex() @@ -274,7 +338,7 @@ impl RenderOnce for NumericStepper { style.bg(cx.theme().colors().element_hover) }) }) - .on_click(self.on_increment), + .on_click(increment_handler), ) } else { increment.child( @@ -285,7 +349,7 @@ impl RenderOnce for NumericStepper { *tab_index += 1; this.tab_index(*tab_index - 1) }) - .on_click(self.on_increment), + .on_click(increment_handler), ) } }), @@ -293,7 +357,7 @@ impl RenderOnce for NumericStepper { } } -impl Component for NumericStepper { +impl Component for NumericStepper { fn scope() -> ComponentScope { ComponentScope::Input } @@ -311,6 +375,8 @@ impl Component for NumericStepper { } fn preview(window: &mut Window, cx: &mut App) -> Option { + let first_stepper = window.use_state(cx, |_, _| 10usize); + let second_stepper = window.use_state(cx, |_, _| 10.0); Some( v_flex() .gap_6() @@ -321,10 +387,7 @@ impl Component for NumericStepper { "Default", NumericStepper::new( "numeric-stepper-component-preview", - "10", - move |_, _| {}, - move |_, _, _| {}, - move |_, _, _| {}, + first_stepper, window, cx, ) @@ -334,10 +397,7 @@ impl Component for NumericStepper { "Outlined", NumericStepper::new( "numeric-stepper-with-border-component-preview", - "10", - move |_, _| {}, - move |_, _, _| {}, - move |_, _, _| {}, + second_stepper, window, cx, ) diff --git a/crates/ui_macros/src/derive_register_component.rs b/crates/ui_macros/src/derive_register_component.rs index 27248e2aac..64ab132cc0 100644 --- a/crates/ui_macros/src/derive_register_component.rs +++ b/crates/ui_macros/src/derive_register_component.rs @@ -4,6 +4,7 @@ use syn::{DeriveInput, parse_macro_input}; pub fn derive_register_component(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); + let name = input.ident; let register_fn_name = syn::Ident::new( &format!("__component_registry_internal_register_{}", name),