-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
: update value on each key press by default
#2880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does mean that it will change the value as you are typing now, which essentially reverts what #2687 wanted. Pinging @harrisonmg to give them a chance to comment.
I would prefer the way this PR has it anyway. Updating the value on every keypress allows you to see the change happening in real time as you type for quickly previewing small adjustments, otherwise you have to press enter and re-focus it every time you want to preview the change. Think about positioning objects precisely in a 3d scene. So I think this is better anyway.
You can always make your own temporary variable and only update it when focus is lost to recreate the functionality #2687 wanted:
// see the hello_world example for the where self.age would come from
let mut temp_value =
ui.data_mut(|data| *data.get_temp_mut_or(Id::new("my_temp"), self.age));
let response = ui.add(egui::DragValue::new(&mut temp_value));
if response.changed() {
ui.data_mut(|data| data.insert_temp(Id::new("my_temp"), temp_value));
}
if response.lost_focus() {
self.age = temp_value;
}
Obviously would want to wrap that in a function or your own widget if you use it a lot.
tldr; I approve of this PR's change, but it should be noted that it is changing the functionality back closer to what it was, but I think it's for the better anyway.
Yes, it's kind of a middle ground. Previously, it would parse the text every frame. With this change, it would only do that work when the |
I prefer it updating while editing the value also. |
In my case, I'm updating motor controller tunings, so this behavior presents a bit of an issue for that. Seems like an option might be nice in the future. |
It sounds like the problem you want to solve is that updating your motor controller tunings too frequently causes problems with the motor. Personally I wouldn't see that as a problem that a UI library should be making changes to try to solve. Instead, you should use something like a debounce function when setting the motor's values from anywhere to prevent it from being updated more often than it can support. This would be more robust and would prevent things like users still being able to focus and unfocus inputs more quickly than is supported. That said, if there's a way to add an option to update the value only on focused lost I wouldn't be opposed to it. It needs to work correctly when tabbing and unfocusing by clicking on other dragvalues/widgets though, which is hard to do because of how DragValue changes it's widget based on if it's focused or not, which causes checks using lost_focus in DragValue's internal logic to be inconsistent. |
Is it possible, in your code, to update on |
Maybe, I'll probably stick to an older egui version for now. Specifically, my issue is that typing "100" results in the gain being set to 1, then 10, then 100. It's the order of magnitude changes, not the frequency. Anyway, I do support this change especially if it improves consistency. |
I think we should add an option for this, so users can control what behavior they want. I definitely see the use in both |
@Barugon could you give a few examples of when you found that this fails? I think it would be helpful to drive future improvements to the focus logic if need be. |
Edit the value and then click on another There's a video in the issue linked above that shows the problem. |
Ok, so nothing beyond what's described in #2142? If so, seems like this option + fixing 2142 would be perfect for everyone |
Loosing your edit due to tabbing or clicking on another |
In #2818 I found the culprit for clicking off but it doesn't fix tabbing. |
Would be nice to see this merged. Some of my users complained that the way it is now is sort of unintuitive and annoying, especially since we're doing rapid data entry on many different |
@emilk |
DragValue
editingDragValue
: update value on each key press by default
Thanks! |
Closes #2818.
Can be changed with
DragValue::update_while_editing
.Use
Response::changed
instead ofResponse::lost_focus
(which is problematic).