PowerToys
PowerToys copied to clipboard
[Settings] Reset Activation Key to Default Value
Summary of the Pull Request
- Add Default value to Properties
- Add Reset button next to shortcut
- Clicking the Reset button on Video Conferencing will crash the Settings UI
PR Checklist
- [x] Closes: #22568
- [ ] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
- [ ] Tests: Added/updated and all pass
- [x] Localization: All end user facing strings can be localized
- [ ] Dev docs: Added/updated
- [ ] New binaries: Added on the required places
- [ ] JSON for signing for new binaries
- [ ] WXS for installer for new binaries and localization folder
- [ ] YML for CI pipeline for new test projects
- [ ] YML for signed pipeline
- [ ] Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
@microsoft-github-policy-service agree
I don't like how my implementation of the button looks. Do you have any suggestions on the location or appearance of the button? Should it be placed inside the shortcut menu? Is an "Undo" icon the right label?
I don't like how my implementation of the button looks. Do you have any suggestions on the location or appearance of the button? Should it be placed inside the shortcut menu? Is an "Undo" icon the right label?
I would prefer if the reset button is inside the pop-up which appears when you click the Edit button on the Shortcut. Also the icon should be more like , the current one you have used is more Undo than Reset and people might expect Undo behavior which can be different from Reset.
@svenlaa To keep the UI as clean as possible, would it make sense to have the reset button in the actual shortcut dialog?
Personal ideas:
- I would - at least - place the reset Button inside the same area, in line with the edit button. Moving it to the fly-out is also possible, but that does mean more clicks, which I find not user friendly.
- The second StackPanel can be removed.
- The Click action can be moved to the actual edit ~FontIcon~ → Button (in stead of the larger area)
Awesome feature! Thanks a lot for this contribution! Is it ready for review?
Awesome feature! Thanks a lot for this contribution! Is it ready for review?
Not quite. A few other folks have commented on it and I'd like to implement their feedback. I got a few personal things going on, so I won't be available for coding 'till Thursday
Thanks for the contribution and the update :) Just press "Ready for review" when you want the core team to review. Thanks a lot, once again.
Personal ideas:
- I would - at least - place the reset Button inside the same area, in line with the edit button. Moving it to the fly-out is also possible, but that does mean more clicks, which I find not user friendly.
It's a secondary action however, so an additional click might not be that big of a deal? My worry is that we now have 3 buttons next to each other - the shortcut button, reset button and the expand/collapse icon. Users might accidently hit the reset button which would be annoying.
I'd put a reset functionality in the dialog (or next to the title similar to Terminal?)
Sure, it not that bad. What about Save-Reset-Cancel in one line?
Sure, it not that bad. What about Save-Reset-Cancel in one line?
I really like this one, and it was easier to implement than I thought. I used a "Use Default" label instead of "Reset" which is clearer in my opinion. What should it reset to? The default shortcut
I think "use default" and "reset" are both possible. Personally I would go for "reset" because it's nice and short. Also thinking about translation + available space.
@niels9001 is the comment (in shortcutcontrol.cs) about C# versus XAML still valid?
When running/debugging: It looks and works good. When building, I get a few issues. (don't know how much it's related to this branch)
Try removing the obj
and bin
folder and/or restart Visual Studio.. weird cache issue you see sometimes, but not related to the branch :).
@Svenlaa do you have an updated screenshot in the main post? I like the 3 button approach!
@niels9001. I just updated the screenshot in the main post. Glad you like the approach!
Yep, simple and clean solution. 👌🏻 Just hoping it will work when a different language uses long words. 🤞🏻
Merged it. Thank you for the contribution!