diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 81f686fe1..eed264a82 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -25,6 +25,24 @@ fn set(get_set_value: &mut GetSetValue<'_>, value: f64) { (get_set_value)(Some(value)); } +// ---------------------------------------------------------------------------- + +/// What the user has typed into a [`DragValue`] that is being edited as text. +/// +/// Stored in [`crate::Memory::data`] between frames, because the text can be +/// something that doesn't (yet) parse to a number, e.g. `"1."` or `"-"`. +#[derive(Clone, Default)] +struct EditState { + /// The text the user is editing. + text: String, + + /// The value of the [`DragValue`] the last time we stored `text`. + /// + /// If the value has changed since then it was changed by something other than + /// this widget, and `text` is stale and must not be written back to the value. + value: f64, +} + /// A numeric value that you can change by dragging the number. More compact than a [`crate::Slider`]. /// /// ``` @@ -466,7 +484,7 @@ impl Widget for DragValue<'_> { }); if ui.memory_mut(|mem| mem.gained_focus(id)) { - ui.data_mut(|data| data.remove::(id)); + ui.data_mut(|data| data.remove::(id)); } let old_value = get(&mut get_set_value); @@ -524,7 +542,7 @@ impl Widget for DragValue<'_> { if old_value != value { set(&mut get_set_value, value); - ui.data_mut(|data| data.remove::(id)); + ui.data_mut(|data| data.remove::(id)); } let value_text = match custom_formatter { @@ -538,8 +556,13 @@ impl Widget for DragValue<'_> { let text_style = ui.style().drag_value_text_style.clone(); if ui.memory(|mem| mem.lost_focus(id)) && !ui.input(|i| i.key_pressed(Key::Escape)) { - let value_text = ui.data_mut(|data| data.remove_temp::(id)); - if let Some(value_text) = value_text { + let edit_state = ui.data_mut(|data| data.remove_temp::(id)); + // Ignore the text if the value was changed by something else while we were editing it, + // or we would revert that change. + if let Some(value_text) = edit_state + .filter(|edit_state| edit_state.value == old_value) + .map(|edit_state| edit_state.text) + { // We were editing the value as text last frame, but lost focus. // Make sure we applied the last text value: let parsed_value = parse(custom_parser.as_ref(), &value_text); @@ -552,9 +575,12 @@ impl Widget for DragValue<'_> { } let mut response = if is_kb_editing { + // Keep editing the text from last frame, unless the value was changed by + // something else in the meantime, in which case the text is stale. let mut value_text = ui - .data_mut(|data| data.remove_temp::(id)) - .unwrap_or_else(|| value_text.clone()); + .data_mut(|data| data.remove_temp::(id)) + .filter(|edit_state| edit_state.value == old_value) + .map_or_else(|| value_text.clone(), |edit_state| edit_state.text); let response = ui.add( TextEdit::singleline(&mut value_text) .clip_text(false) @@ -589,7 +615,13 @@ impl Widget for DragValue<'_> { set(&mut get_set_value, parsed_value); } } - ui.data_mut(|data| data.insert_temp(id, value_text)); + // Remember the value the text belongs to, so that next frame we can tell + // whether the value was changed by us or by something else. + let edit_state = EditState { + text: value_text, + value: get(&mut get_set_value), + }; + ui.data_mut(|data| data.insert_temp(id, edit_state)); response } else { atoms.map_atoms(|atom| { @@ -631,7 +663,7 @@ impl Widget for DragValue<'_> { } if response.clicked() { - ui.data_mut(|data| data.remove::(id)); + ui.data_mut(|data| data.remove::(id)); ui.memory_mut(|mem| mem.request_focus(id)); select_all_text(ui, id, response.id, &value_text); } else if response.dragged() { diff --git a/crates/egui_kittest/tests/regression_tests.rs b/crates/egui_kittest/tests/regression_tests.rs index 12e66ac56..1f47cd422 100644 --- a/crates/egui_kittest/tests/regression_tests.rs +++ b/crates/egui_kittest/tests/regression_tests.rs @@ -814,3 +814,100 @@ pub fn textedit_hint_text_should_follow_text_alignment() { edit_center_x={edit_center_x}, edit_rect={edit_rect:?}", ); } + +/// A focused `DragValue` keeps the text the user is editing in memory. +/// +/// If something else changes the value while the `DragValue` has focus, +/// that memorized text is stale, and must not be written back to the value. +/// +/// Regression test for . +#[test] +pub fn drag_value_should_not_revert_external_changes_while_focused() { + let mut harness = Harness::new_ui_state( + |ui, value: &mut i32| { + ui.add(egui::DragValue::new(value)); + }, + 0, + ); + + // Focus the `DragValue`, putting it in text-edit mode. + harness.key_press(egui::Key::Tab); + harness.run(); + + // Something else changes the value while the `DragValue` is focused. + *harness.state_mut() = 42; + harness.run(); + + assert_eq!(harness.state(), &42); + let drag_value = harness.get_by_role(accesskit::Role::SpinButton); + assert_eq!(drag_value.value(), Some("42".to_owned())); + + // Losing focus must not restore the value the `DragValue` had when it gained focus. + harness.key_press(egui::Key::Tab); + harness.run(); + + assert_eq!(harness.state(), &42); +} + +/// While the user is typing into a `DragValue`, the half-finished text must be kept +/// between frames, even though it doesn't always parse back to the same text. +#[test] +pub fn drag_value_should_keep_text_while_typing() { + let mut harness = Harness::new_ui_state( + |ui, value: &mut f64| { + ui.add(egui::DragValue::new(value)); + }, + 0.0, + ); + + // Focus the `DragValue`, putting it in text-edit mode with the old text selected. + harness.key_press(egui::Key::Tab); + harness.run(); + + // Type one character per frame. `"1."` parses to `1`, which is formatted as `"1"`, + // so re-reading the text from the value would eat the decimal point. + for character in "1.25".chars() { + harness + .get_by_role(accesskit::Role::SpinButton) + .type_text(&character.to_string()); + harness.run(); + } + + harness.key_press(egui::Key::Enter); + harness.run(); + + assert_eq!(harness.state(), &1.25); +} + +/// An integer `DragValue` cannot represent everything the user types into it, +/// but the text must still survive until the user is done typing. +#[test] +pub fn drag_value_should_keep_text_the_value_cannot_represent() { + let mut harness = Harness::new_ui_state( + |ui, value: &mut i32| { + ui.add(egui::DragValue::new(value)); + }, + 0, + ); + + // Focus the `DragValue`, putting it in text-edit mode with the old text selected. + harness.key_press(egui::Key::Tab); + harness.run(); + + // `"12.5"` is stored as `12`, which is formatted as `"12"`. + harness + .get_by_role(accesskit::Role::SpinButton) + .type_text("12.5"); + harness.run(); + + // If the text was re-read from the value now, this would append to `"12"`. + harness + .get_by_role(accesskit::Role::SpinButton) + .type_text("9"); + harness.run(); + + harness.key_press(egui::Key::Enter); + harness.run(); + + assert_eq!(harness.state(), &12, "The text should have been \"12.59\""); +}