-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[ZoomIt] Settings UX improvements #43073
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
base: main
Are you sure you want to change the base?
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Jay-o-Way
left a comment
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.
Got a screenshot? Worried about all the formatting...
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
check-spelling run (pull_request_target) for dev/vanzue/zoomit-hotkey Signed-off-by: check-spelling-bot <[email protected]> on-behalf-of: @check-spelling <[email protected]>
Jay-o-Way
left a comment
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.
Please add screenshots to the main pr message. Notice how the code formatting messes up parts like Ctrl '' C? Please adjust to what I suggested.
Jay-o-Way
left a comment
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.
No-go
|
@Jay-o-Way Great feedback! I've pushed some improvements, see the original post for a screenshot. Looking at Windows 11 Settings, there doesn't seem to be a clear standard of how to visualize shortcut instructions. I'm sticking to 12pt / Nit: should be |
|
Idea: the Footer of the Settings expander is often overlooked. Would it be good to place te (explanatory) text in there? |
There wouldn't be a visible difference, and we need the |
…ft/PowerToys into dev/vanzue/zoomit-hotkey
| <RadioButton x:Uid="ZoomIt_Break_ShowImageFile" /> | ||
| </RadioButtons> | ||
| <tkcontrols:SettingsCard Name="ZoomItBreakTimerOpacity" x:Uid="ZoomIt_Break_TimerOpacity"> | ||
| <ComboBox MinWidth="{StaticResource SettingActionControlMinWidth}" SelectedIndex="{x:Bind Path=ViewModel.BreakTimerOpacityIndex, Mode=TwoWay}"> |
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.
I think this should be a Slider.
| </ComboBox> | ||
| </tkcontrols:SettingsCard> | ||
| <tkcontrols:SettingsCard Name="ZoomItBreakTimerPosition" x:Uid="ZoomIt_Break_TimerPosition"> | ||
| <ComboBox MinWidth="{StaticResource SettingActionControlMinWidth}" SelectedIndex="{x:Bind Path=ViewModel.BreakTimerPosition, Mode=TwoWay}"> |
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.
I believe the MinWidth property is built-in (right, @niels9001 ?)
|
|
||
|
|
||
| <tkcontrols:SettingsCard Name="ZoomItBreakShowBackgroundBitmap" x:Uid="ZoomIt_Break_ShowBackgroundBitmap"> | ||
| <ComboBox MinWidth="{StaticResource SettingActionControlMinWidth}" SelectedIndex="{x:Bind Path=ViewModel.BreakBackgroundSelectionIndex, Mode=TwoWay}"> |
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.
General - Path= shouldn't bee needed with x:Bind.
| </tkcontrols:SettingsCard> | ||
| <tkcontrols:SettingsExpander.Items> | ||
| <tkcontrols:SettingsCard Name="ZoomItRecordScaling" x:Uid="ZoomIt_Record_Scaling"> | ||
| <ComboBox MinWidth="{StaticResource SettingActionControlMinWidth}" SelectedIndex="{x:Bind Path=ViewModel.RecordScalingIndex, Mode=TwoWay}"> |
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.
Same: Slider.
Summary of the Pull Request
This PR enhances the ZoomIt settings UI by refactoring some of the XAML code and putting instructions as part of the settingsexpanders. Additionally, the alternate hotkey combinations are now shown too and will be updated based on the configured hotkey. Users will now be able to see all the hotkeys.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
ZoomIt Extended (Derived) Hotkeys