Skip to content
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

[Settings] New UX #12470

Merged
merged 89 commits into from
Aug 23, 2021
Merged

[Settings] New UX #12470

merged 89 commits into from
Aug 23, 2021

Conversation

niels9001
Copy link
Contributor

@niels9001 niels9001 commented Jul 22, 2021

Summary of the Pull Request

This PR introduces a new look and feel for Settings. Advantages:

  • New, beautiful look and feel :)
  • Easier to maintain: new standardized controls can now be edited once and will trickle down to all pages!
  • Now inline with Windows 11 Settings UX
  • Many accessibility improvements

NewSettings

Issues resolved:

#12507 - Chosing a mode is now a ComboBox vs. RadioButtons
#12028 - Links are now more descriptive ("See what's new in this version" vs. "Version 0.43" or "Read more")
#12023 - Updated link name "Learn more about Color Picker" vs. "Learn more"
#12011 - Narrator now announces ToggleSwitch
#11988 - Correct themeresources are now used
#11860 - Now uses the same look and feel as W11 Settings
#10801 - "Direct activation command" is now used
#10635 - Is now keyboard accessible with dropdown buttons
#9412 - Now using the new visual styles
#9345 - Setting header is now aligned
#7058 - ToolTip is no longer used - now a text description that is always visible
#11196 - HotkeyControls are now disabled whenever the module is disabled
#10725 - Hyperlinks replaced with HyperLinkButtons
#12774 - Colored icons are now used
#9176 - No longer making use of this icon
#10778 - VCM is redesigned as well
#12832 - Changed label to "Restart PowerToys as administrator"
#12632 - Changed label from "Off (Passive)" to "Inactive"
#10038 - We now use an expander, so that resolves this issue.
#9987 - OOBE now launches with the same theme applied as Settings
#11366 - New update-ing UI
#6595 - There's now a release notes button next to the current version number

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@htcfreek
Copy link
Collaborator

@niels9001

  • Checks are failing.
  • I think the "Welcome to PT" button/link should be white and not black in dark theme. Or is this default on WinUI 2.6 style? Why is it not the same on ligth and dark mode?
  • I am not sure. But it seems for me that the space between symbol and text is to big on the "Welcome to PT" button/link.

@jaimecbernardo
Copy link
Collaborator

jaimecbernardo commented Jul 22, 2021

Hi @niels9001 ,
This is pretty weird, but the update to WINUI 2.6 is causing some weird errors on my machine.
I can open Settings when running from Visual Studio, but when I try running from the build folder directly or from an installed version, settings doesn't open.
I've got these 2 errors in the Event Viewer each time I try to open Settings:

Application: PowerToys.Settings.exe
CoreCLR Version: 4.700.21.26205
.NET Core Version: 3.1.16
Description: The process was terminated due to an internal error in the .NET Runtime at IP 00007FFB18A41EE7 (00007FFB188B0000) with exit code c0000005.

and

Faulting application name: PowerToys.Settings.exe, version: 0.0.1.0, time stamp: 0x609c77fe
Faulting module name: coreclr.dll, version: 4.700.21.26205, time stamp: 0x609c312a
Exception code: 0xc0000005
Fault offset: 0x0000000000191ee7
Faulting process id: 0x420c
Faulting application start time: 0x01d77f0d9e095298
Faulting application path: D:\prog\janeasystems\PowerToys\x64\Release\Settings\PowerToys.Settings.exe
Faulting module path: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.1.16\coreclr.dll
Report Id: f99ac88b-1d90-4e6c-8c50-440fcf27980f
Faulting package full name: 
Faulting package-relative application ID: 

I've tried to understand which commit started causing this and it seems to have been: 1bd7023

I still haven't installed the "Windows 10, version 20H2" feature update, in case that's relevant.

@davidegiacometti
Copy link
Collaborator

davidegiacometti commented Jul 22, 2021

@jaimecbernardo
In current master there are some DLLs missing from setup after VCM porting. ColorPicker wasn't running in my last build.

@jaimecbernardo
Copy link
Collaborator

@davidegiacometti , There's a fix on the works for that, alongside the fix for: #12455
This "Settings not opening issue" seems to be related to the commit that brought in winUI 2.6

@niels9001
Copy link
Contributor Author

o be related to the comm

Hmm interesting.. does the master branch work for you? #12455 was merged earlier on.

@jaimecbernardo
Copy link
Collaborator

I've built 1bd7023 and I've got this error happening there.
I've build the previous commit 3c04111 and it doesn't happen there.

Now when I need to test something outside of Visual Studio I revert 1bd7023 locally in order to test it successfully.

The really annoying part here is that when I start the runner or settings from Visual Studio it just works, so I can't Debug it 🤷 .

@jaimecbernardo
Copy link
Collaborator

Hmm interesting.. does the master branch work for you? #12455 was merged earlier on.

master branch has this issue for me as well.

@jaimecbernardo
Copy link
Collaborator

jaimecbernardo commented Jul 23, 2021

Hi @niels9001 , do you know if we need the latest version?
Downgrading Microsoft.UI.Xaml from 2.6.1-prerelease.210709001 to 2.6.0-prerelease.210623001 seems to make settings work correctly for me again.

These symptoms (only working correctly when starting to debug) seem to indicate that 2.6.1-prerelease.210709001 is probably doing some undefined behavior in its code. I have only been able to replicate this issue in this specific machine, though.

@niels9001
Copy link
Contributor Author

Hi @niels9001 , do you know if we need the latest version?
Downgrading Microsoft.UI.Xaml from 2.6.1-prerelease.210709001 to 2.6.0-prerelease.210623001 seems to make settings work correctly for me again.

These symptoms (only working correctly when starting to debug) seem to indicate that 2.6.1-prerelease.210709001 is probably doing some undefined behavior in its code. I have only been able to replicate this issue in this specific machine, though.

Hmm no idea.. I think that should be fine, as long as it's higher than 2.5 and prerelease (else it won't work with XAML Islands).

I'd suggest to file an issue against the WinUI repo though so that team can have a look at the root cause.

@jaimecbernardo
Copy link
Collaborator

Opened issue here: microsoft/microsoft-ui-xaml#5546

In the meanwhile, it might make sense to downgrade the version of Microsoft.UI.Xaml we use.

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Jul 25, 2021

  • I think the "Welcome to PT" button/link should be white and not black in dark theme. Or is this default on WinUI 2.6 style? Why is it not the same on ligth and dark mode?

I think that's because of the AccentButton style? Don't know why a NavItem needs a style anyways?

Probably a bit off-topic for this pr, but if this button is moved to the Navigation, can we also move the "GitHub repo" and "Give feedback" links here?

  • I am not sure. But it seems for me that the space between symbol and text is to big on the "Welcome to PT" button/link.

There are a lot of manual margins all over the place. I'm really starting to question if that is good designing. I would remove most of them and just let WinUI (or XAML or whatever the language is) handle default styling. Especially when there are visible changes like this. Also, the margins are hardcoded for almost every control. Isn't it smarter to use styles?

@niels9001 niels9001 marked this pull request as draft July 28, 2021 20:32
@Aaron-Junker
Copy link
Collaborator

@niels9001 Will you change the labels of the corresponding issues or should I do it. I have currently some time, so it would not be a problem if you want

Feel free to do so ;)

@niels9001 Done.

@niels9001 niels9001 merged commit eb2ef70 into master Aug 23, 2021
@Aaron-Junker
Copy link
Collaborator

Big shout outs to all developers who worked on this PR and tested it. (And of course also all other developers who work on this amazing project.) I wasn't able to test it by myself, but it looks amazing. We're one of the first not core OS apps which adopted the new Windows 11 design. From my perspective this is amazing. Keep the good work up!

@htcfreek
Copy link
Collaborator

@niels9001
One last question I missed to ask:
Why do I have to rebuild the complete PT project to get changes take affect? If I only rebuild settings project the build result is not in the correct directory.

@niels9001
Copy link
Contributor Author

@niels9001
One last question I missed to ask:
Why do I have to rebuild the complete PT project to get changes take affect? If I only rebuild settings project the build result is not in the correct directory.

You need to run PowerToys.Settings.. but that is only the window. The entire UI (= XAML Island) is PowerToys.Settings.UI. so if you build only .UI you can just run PowerToys.Settings.

@nlogozzo
Copy link

nlogozzo commented Aug 25, 2021

Doesn't this program use WPF and ModernWPF? I know ModernWPF hasn't been updated in a while...how did you guys achieve the new styles?

@niels9001
Copy link
Contributor Author

Doesn't this program use WPF and ModernWPF? I know ModernWPF hasn't been updated in a while...how did you guys achieve the new styles?

Some PowerToys modules use ModernWPF (like ColorPicker and ImageResizer) and have not yet been upgraded with new styles.

Settings uses XAML Islands (UWP+WinUI 2.6 wrapped in a WPF window) for the UI. WinUI 2.6 does contain the new styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.