migrations/egui
1
0
Fork 0
mirror of https://github.com/emilk/egui.git synced 2026-08-29 04:40:03 -04:00
Code Activity

Don't revert external changes to a focused DragValue (#8403)

Browse Source 
* Closes <https://github.com/emilk/egui/issues/8339>
* [x] I have followed the instructions in the PR template

## The bug

While a `DragValue` has focus it is rendered as a `TextEdit`, and the
text being
edited is stored in `Memory::data` between frames. That is needed so
that
half-finished input such as `"1."` or `"-"` isn't thrown away just
because it
doesn't parse to the current value.

The stored text was only discarded when the widget *gained* focus or
when the
widget itself changed the value. If something else changed the value
while the
`DragValue` was focused, the stored text was kept, shown to the user,
and
written back to the value when focus was lost — silently undoing the
external
change:

```rust
ui.add(egui::DragValue::new(&mut self.value));
if ui.button("increment").clicked() {
    self.value += 1;
}
```

Click into the `DragValue` so it has focus, then press "increment": the
value
goes up for one frame and then snaps back. `Slider` shows the same
behaviour,
since it uses a `DragValue` for its value field.

## The fix

Store the value the text belongs to next to the text, and discard the
text when
the value no longer matches it. The remembered value is read back from
the
get/set closure *after* the widget has applied its own edits, so a
change the
widget made itself never looks like an external one — this matters for
values
that can't represent what was typed, e.g. `"12.5"` in a
`DragValue<i32>`.

This keeps the reason the text is stored in the first place intact: as
long as
nothing else touches the value, the text the user is typing is preserved
verbatim.

## Tests

Three tests in `crates/egui_kittest/tests/regression_tests.rs`:

* `drag_value_should_not_revert_external_changes_while_focused` — the
actual
  regression. Fails on `main`:

  ```
---- drag_value_should_not_revert_external_changes_while_focused stdout
----
  assertion `left == right` failed
    left: Some("0")
   right: Some("42")
  ```

and, with the display assertion removed so the test reaches the blur, on
the
  value itself:

  ```
  assertion `left == right` failed
    left: 0
   right: 42
  ```

* `drag_value_should_keep_text_while_typing` and
`drag_value_should_keep_text_the_value_cannot_represent` — guards for
the
behaviour the stored text exists for. Both pass on `main` and after the
fix,
  and both fail if the text is re-read from the value too eagerly.

`cargo test -p egui_kittest` and `cargo test -p egui` pass, as do
`cargo fmt --all --check`, `scripts/lint.py` and
`cargo clippy -p egui -p egui_kittest --all-targets --all-features -- -D
warnings`.

## Not changed

`DragValue` still ignores the stored text when <kbd>Escape</kbd> is
pressed, and
`update_while_editing` still decides when typed text is applied —
neither is
touched here.
This commit is contained in:
Teddy Tennant
2026-08-11 08:04:09 -04:00
committed by GitHub
parent 3c69fb4833
commit d802a982ce
2 changed files with 137 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
crates/egui/src/widgets/drag_value.rs
View File

@@ -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::<String>(id));
ui.data_mut(|data| data.remove::<EditState>(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::<String>(id));
ui.data_mut(|data| data.remove::<EditState>(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::<String>(id));
if let Some(value_text) = value_text {
let edit_state = ui.data_mut(|data| data.remove_temp::<EditState>(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::<String>(id))
.unwrap_or_else(|| value_text.clone());
.data_mut(|data| data.remove_temp::<EditState>(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::<String>(id));
ui.data_mut(|data| data.remove::<EditState>(id));
ui.memory_mut(|mem| mem.request_focus(id));
select_all_text(ui, id, response.id, &value_text);
} else if response.dragged() {

97
crates/egui_kittest/tests/regression_tests.rs
View File

@@ -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 <https://github.com/emilk/egui/issues/8339>.
#[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\"");
}