CommunityToolkit
Update RadialGauge to use KeyboardAccelerators
Replace KeyDown event handling by KeyboardAccelerators. This does not change the behavior. It improves performance by not reacting to just any key stroke. I also removes the call to CoreWindow.GetKeyState - an API that doesn't exist in WinUI 3.
Fixes
Replace KeyDown event handling by KeyboardAccelerators.
PR Type
What kind of change does this PR introduce?
Refactoring (no functional changes, no api changes)
What is the current behavior?
The value can be manipulated via keyboard input: arrows, with our without CTRL.
What is the new behavior?
The same.
It improves performance by not reacting to just any key stroke. I also removes the call to CoreWindow.GetKeyState - an API that doesn't exist in WinUI 3. It is possible to create a RadialGauge control for WinUI 2 (UWP) as well as WinUI3 (Win32) from the same source code, and this modification brings this situation closer.
PR Checklist
Please check if your PR fulfills the following requirements:
- [ * ] Tested code with current UWP (and WinUI 3) versions.
- [ ] New component
- [ ] Pull Request has been submitted to the documentation repository instructions. Link:
- [ ] Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
- [ ] If control, added to Visual Studio Design project
- [ ] Sample in sample app has been added / updated (for bug fixes / features)
- [ ] Icon has been created (if new sample) following the Thumbnail Style Guide and templates
- [ ] New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
- [ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [ ] Header has been added to all new source files (run build/UpdateHeaders.bat)
- [ * ] Contains NO breaking changes
Other information
Thanks XamlBrewer for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌
I guess it makes sense to create a Toolkit Labs Experiment for this control, so we can easily debug on all platforms (UWP, WinUI 3, UNO).
I guess it makes sense to create a Toolkit Labs Experiment for this control, so we can easily debug on all platforms (UWP, WinUI 3, UNO).
The plan after the upcoming hotfix is to bring the labs infrastructure to the main repo so we can merge all our branches together. If you do want to test in the interim across both though, copying over to a Labs environment does work. We've done it for a few things we've been working on like the new GridSplitter.
The migration to Labs will take place as 8.0. We'd like to include this as a hotfix in 7.1.3/7.2, so we'll need to fix the regression before moving forward @XamlBrewer
To me, the most probable reason for this is that you forgot to set 'IsInteractive' to True. The control only listens to keystrokes and touch gestures when it is in interactive mode. I'm pretty confident about the source code - I can share a simple UWP host app running this exact version if you want.
Other possibility: mismatch between 'Maximum' (default 1, from RangeBase) and 'SmallChange'/'LargeChange' (set to 1 and 10 in the constructor - maybe not the best idea). In that case you might not see changes when using arrow keys.
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 30 days of this comment.
@XamlBrewer Looks like the arrow keys only work when the Ctrl key is pressed, and only after reducing the StepSize
The default values for Minimum, Maximum, StepSize, SmallChange, and LargeChange are not ideal, but they did not change since 2018 (unless there was a change in RangeBase). So I do not understand how this PR can create a regression in this domain.
@Arlodotexe When you configure SmallChange and/or LargeChange to be smaller than the StepSize, then you can't change the Value using the arrow keys - since the result is rounded back to the original Value. To me, this is expected behavior. As a workaround we could use Max(*Change, StepSize) as increment or decrement in the keyboard accelerators. But then we're basically ignoring the property values set by the developer. How do other controls (like the Slider) deal with this?
As a workaround we could use Max(*Change, StepSize) as increment or decrement in the keyboard accelerators. But then we're basically ignoring the property values set by the developer. How do other controls (like the Slider) deal with this?
This line wasn't carried over during the refactor. I've pushed a commit that fixes the regression while using the new code.