microsoft / PowerToys

Windows system utilities to maximize productivity
MIT License
108.13k stars 6.39k forks source link

[Settings] review InfoBar properties #20279

Open Jay-o-Way opened 1 year ago

Jay-o-Way commented 1 year ago

Description of the new feature / enhancement

<HyperlinkButton x:Uid="SeeWhatsNew" is redundant; move to different place

ID change
General_UpToDate IsClosable="True"
General_NewVersionAvailable IsClosable="True"; Button → ActionContent
General_NewVersionReadyToInstall Button → ActionContent
General_FailedToDownloadTheNewVersion Button → ActionContent
Run_NotAccessibleWarning Severity = ??
FileExplorerPreview_PreviewHandlerOutlookIncompatibility IsClosable="True"
FileExplorerPreview_RebootRequired IsClosable="True"
FileExplorerPreview_ThumbnailsMightNotAppearOnRemoteFolders Info
MeasureTool_ContinuousCapture_Information Info

Bonus: closing

For both, we would need a way to remember if the user has closed them. This could be via IsOpen property, or (especially for Tips) use a list with the IDs-to-show, and when a Tip is closed, we pop that ID out of the list.

Also

  • The indeterminate state for ProgressBar shows that an operation is underway, does not block user interaction with the app, and its completion time is unknown.
  • The indeterminate state for ProgressRing shows that an operation is underway, blocks user interaction with the app, and its completion time is unknown.

https://learn.microsoft.com/nl-nl/windows/apps/design/controls/progress-controls

This means we should use a ProgressBar. (properties: IsIndeterminate, ShowPaused, ShowError)

Note to self: check ScreenReader Windows - Apps - Design Win32 msg guide

htcfreek commented 1 year ago
Jay-o-Way commented 1 year ago
  • VideoConference_RunAsAdminRequired is no error We should keep warning style. Also to be in line with the other admin warnings.

If something is unable to continue, it should he an error.

  • MeasureTool_ContinuousCapture_Information can still be a warning. This makes sense for me and the decision to have it as warning was made by the core team.

The _information sounds like a hint. I don't see anything that will break.

htcfreek commented 1 year ago
  • VideoConference_RunAsAdminRequired is no error We should keep warning style. Also to be in line with the other admin warnings.

If something is unable to continue, it should he an error.

The toy is still working. You need only admin permission to change the settings. It is the same case we have on RunAsAdmin setting.

  • MeasureTool_ContinuousCapture_Information can still be a warning. This makes sense for me and the decision to have it as warning was made by the core team.

The _information sounds like a hint. I don't see anything that will break.

Sure. Nothing break. But I think it's important to know that it will impact your system resources when having this mode enabled. For me warning feels okay on such an important Information.

htcfreek commented 1 year ago

@Jay-o-Way I appreciate your work to clean up settings code. But to be honest I don't think that this is an task we should ficus on. We have much more important issue and you know that our resources are limited. Nothing is broken here and these are only noce to have things.

Jay-o-Way commented 1 year ago

I have the time