From 0c7d639cbd448b210b1108890a02f532e6329eae Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Tue, 30 Dec 2025 14:24:33 -0300 Subject: [PATCH] ui: Add fixes to the `NumberField` edit mode (#45871) Follow up to https://github.com/zed-industries/zed/pull/45447 and in preparation to enable the edit mode in number field instances within the settings UI. This PR fixes the editor in the number field capturing focus automatically (unnecessary), tab index in the buttons and editor, and other things. Release Notes: - N/A --- crates/ui_input/src/number_field.rs | 176 +++++++++++++++++++--------- 1 file changed, 120 insertions(+), 56 deletions(-) diff --git a/crates/ui_input/src/number_field.rs b/crates/ui_input/src/number_field.rs index b07cc5d9dc17dc6c761d02222807101d19cecc33..b72ce39d4c619600ddc104223087a759081449cf 100644 --- a/crates/ui_input/src/number_field.rs +++ b/crates/ui_input/src/number_field.rs @@ -238,6 +238,8 @@ impl_numeric_stepper_nonzero_int!(NonZeroU32, u32); impl_numeric_stepper_nonzero_int!(NonZeroU64, u64); impl_numeric_stepper_nonzero_int!(NonZero, usize); +type OnChangeCallback = Rc; + #[derive(IntoElement, RegisterComponent)] pub struct NumberField { id: ElementId, @@ -246,6 +248,10 @@ pub struct NumberField { mode: Entity, /// Stores a weak reference to the editor when in edit mode, so buttons can update its text edit_editor: Entity>>, + /// Stores the on_change callback in Entity state so it's not stale in focus_out handlers + on_change_state: Entity>>, + /// Tracks the last prop value we synced to, so we can detect external changes (like reset) + last_synced_value: Entity>, format: Box String>, large_step: T, small_step: T, @@ -261,17 +267,29 @@ impl NumberField { pub fn new(id: impl Into, value: T, window: &mut Window, cx: &mut App) -> Self { let id = id.into(); - let (mode, focus_handle, edit_editor) = window.with_id(id.clone(), |window| { - let mode = window.use_state(cx, |_, _| NumberFieldMode::default()); - let focus_handle = window.use_state(cx, |_, cx| cx.focus_handle()); - let edit_editor = window.use_state(cx, |_, _| None); - (mode, focus_handle, edit_editor) - }); + let (mode, focus_handle, edit_editor, on_change_state, last_synced_value) = + window.with_id(id.clone(), |window| { + let mode = window.use_state(cx, |_, _| NumberFieldMode::default()); + let focus_handle = window.use_state(cx, |_, cx| cx.focus_handle()); + let edit_editor = window.use_state(cx, |_, _| None); + let on_change_state: Entity>> = + window.use_state(cx, |_, _| None); + let last_synced_value: Entity> = window.use_state(cx, |_, _| None); + ( + mode, + focus_handle, + edit_editor, + on_change_state, + last_synced_value, + ) + }); Self { id, mode, edit_editor, + on_change_state, + last_synced_value, value, focus_handle: focus_handle.read(cx).clone(), format: Box::new(T::default_format), @@ -338,6 +356,11 @@ impl NumberField { self.on_change = Rc::new(on_change); self } + + fn sync_on_change_state(&self, cx: &mut App) { + self.on_change_state + .update(cx, |state, _| *state = Some(self.on_change.clone())); + } } #[derive(Clone, Copy)] @@ -348,7 +371,9 @@ enum ValueChangeDirection { impl RenderOnce for NumberField { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let mut tab_index = self.tab_index; + // Sync the on_change callback to Entity state so focus_out handlers can access it + self.sync_on_change_state(cx); + let is_edit_mode = matches!(*self.mode.read(cx), NumberFieldMode::Edit); let get_step = { @@ -456,14 +481,13 @@ impl RenderOnce for NumberField { this.child( IconButton::new("reset", IconName::RotateCcw) .icon_size(IconSize::Small) - .when_some(tab_index.as_mut(), |this, tab_index| { - *tab_index += 1; - this.tab_index(*tab_index - 1) - }) + .when_some(self.tab_index, |this, _| this.tab_index(0isize)) .on_click(on_reset), ) }) - .child( + .child({ + let on_change_for_increment = self.on_change.clone(); + h_flex() .map(|decrement| { let decrement_handler = { @@ -487,22 +511,14 @@ impl RenderOnce for NumberField { decrement.child( base_button(IconName::Dash) - .id("decrement_button") + .id((self.id.clone(), "decrement_button")) .rounded_tl_sm() .rounded_bl_sm() - .tab_index( - tab_index - .as_mut() - .map(|tab_index| { - *tab_index += 1; - *tab_index - 1 - }) - .unwrap_or(0), - ) + .when_some(self.tab_index, |this, _| this.tab_index(0isize)) .on_click(decrement_handler), ) }) - .child( + .child({ h_flex() .min_w_16() .size_full() @@ -515,20 +531,26 @@ impl RenderOnce for NumberField { .px_1() .flex_1() .justify_center() - .child(Label::new((self.format)(&self.value))) + .child( + Label::new((self.format)(&self.value)).color(Color::Muted), + ) .into_any_element(), - NumberFieldMode::Edit => h_flex() - .flex_1() - .child(window.use_state(cx, { - |window, cx| { + NumberFieldMode::Edit => { + let expected_text = format!("{}", self.value); + + let editor = window.use_state(cx, { + let expected_text = expected_text.clone(); + + move |window, cx| { let mut editor = Editor::single_line(window, cx); editor.set_text_style_refinement(TextStyleRefinement { + color: Some(cx.theme().colors().text), text_align: Some(TextAlign::Center), ..Default::default() }); - editor.set_text(format!("{}", self.value), window, cx); + editor.set_text(expected_text, window, cx); let editor_weak = cx.entity().downgrade(); @@ -601,36 +623,86 @@ impl RenderOnce for NumberField { .detach(); cx.on_focus_out(&editor.focus_handle(cx), window, { - let mode = self.mode.clone(); - let on_change = self.on_change.clone(); + let on_change_state = self.on_change_state.clone(); move |this, _, window, cx| { if let Ok(parsed_value) = this.text(cx).parse::() { let new_value = clamp_value(parsed_value); - on_change(&new_value, window, cx); + let on_change = + on_change_state.read(cx).clone(); + + if let Some(on_change) = on_change.as_ref() + { + on_change(&new_value, window, cx); + } }; - mode.write(cx, NumberFieldMode::Read); } }) .detach(); - window.focus(&editor.focus_handle(cx), cx); - editor } - })) - .on_action::({ - move |_, window, _| { - window.blur(); + }); + + let focus_handle = editor.focus_handle(cx); + let is_focused = focus_handle.is_focused(window); + + if !is_focused { + let current_text = editor.read(cx).text(cx); + let last_synced = *self.last_synced_value.read(cx); + + // Detect if the value changed externally (e.g., reset button) + let value_changed_externally = last_synced + .map(|last| last != self.value) + .unwrap_or(true); + + let should_sync = if value_changed_externally { + true + } else { + match current_text.parse::().ok() { + Some(parsed) => parsed == self.value, + None => true, + } + }; + + if should_sync && current_text != expected_text { + editor.update(cx, |editor, cx| { + editor.set_text(expected_text.clone(), window, cx); + }); } - }) - .into_any_element(), - }), - ) + + self.last_synced_value + .update(cx, |state, _| *state = Some(self.value)); + } + + let focus_handle = if self.tab_index.is_some() { + focus_handle.tab_index(0isize).tab_stop(true) + } else { + focus_handle + }; + + h_flex() + .flex_1() + .track_focus(&focus_handle) + .border_1() + .border_color(cx.theme().colors().border_transparent) + .when(is_focused, |this| { + this.border_color(cx.theme().colors().border_focused) + }) + .child(editor) + .on_action::({ + move |_, window, _| { + window.blur(); + } + }) + .into_any_element() + } + }) + }) .map(|increment| { let increment_handler = { - let on_change = self.on_change.clone(); + let on_change = on_change_for_increment.clone(); let get_current_value = get_current_value.clone(); let update_editor_text = update_editor_text.clone(); @@ -650,22 +722,14 @@ impl RenderOnce for NumberField { increment.child( base_button(IconName::Plus) - .id("increment_button") + .id((self.id.clone(), "increment_button")) .rounded_tr_sm() .rounded_br_sm() - .tab_index( - tab_index - .as_mut() - .map(|tab_index| { - *tab_index += 1; - *tab_index - 1 - }) - .unwrap_or(0), - ) + .when_some(self.tab_index, |this, _| this.tab_index(0isize)) .on_click(increment_handler), ) - }), - ) + }) + }) } }