DragValue: when keyboard editing, only update the value on focus lost #2688
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Actually, it is very convenient to be able to move keyboard focus to |
emilk
requested changes
Feb 7, 2023
Mingun
reviewed
Feb 8, 2023
| 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); | ||
| if response.lost_focus() { |
There was a problem hiding this comment.
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
reviewed
Feb 8, 2023
emilk
reviewed
Feb 8, 2023
This was referenced Apr 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2687
Since the underlying
TextEditissingleline, pressing enter also results in lost focus.I demonstrate two instances of this below. I press enter after the first edit, then click away after the second.
I also shuffled some of the accesskit stuff. The up and down arrow keys probably shouldn't change the value during keyboard editing either. That little change allowed me to consolidate the accesskit code a bit better. I'm not sure how to test it.
As far as testing goes,
./sh/check.shfailed in my environment (WSL 2) for some winit / platform reason. Clippy has no complaints about my changes.