Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DragValue: when keyboard editing, only update the value on focus lost #2688

Merged
merged 5 commits into from
Feb 8, 2023
Merged

DragValue: when keyboard editing, only update the value on focus lost #2688

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
826bc89
test
harrisonmg Feb 7, 2023
29b6308
moved some accesskit stuff
harrisonmg Feb 7, 2023
92be531
reverted accesskit change
harrisonmg Feb 7, 2023
cce435d
Add explanatory comment
emilk Feb 8, 2023
2b0b278
fmt
emilk Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions crates/egui/src/widgets/drag_value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,13 +475,17 @@ impl<'a> Widget for DragValue<'a> {
.desired_width(ui.spacing().interact_size.x)
.font(text_style),
);
let parsed_value = match custom_parser {
Some(parser) => parser(&value_text),
None => value_text.parse().ok(),
};
if let Some(parsed_value) = parsed_value {
let parsed_value = clamp_to_range(parsed_value, clamp_range.clone());
set(&mut get_set_value, parsed_value);
// Only update the value when the user presses enter, or clicks elsewhere. NOT every frame.
// See https://github.com/emilk/egui/issues/2687
if response.lost_focus() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if you also added a comment justifying why this check should be done. The fact that it was absent before allows us to conclude that it is not self evident. Description of the reasons why this check was added and what it affects will make it easier to resolve problems if this change leads to them.

A good tone will also leave a link to the GitHub Issue, where you can get more information.

emilk marked this conversation as resolved.
Show resolved Hide resolved
let parsed_value = match custom_parser {
Some(parser) => parser(&value_text),
None => value_text.parse().ok(),
};
if let Some(parsed_value) = parsed_value {
let parsed_value = clamp_to_range(parsed_value, clamp_range.clone());
set(&mut get_set_value, parsed_value);
}
}
ui.memory_mut(|mem| mem.drag_value.edit_string = Some(value_text));
response
Expand Down