wpf icon indicating copy to clipboard operation
wpf copied to clipboard

Win11 Theming outstanding Issue Tracker

Open dipeshmsft opened this issue 1 year ago • 10 comments

Description

This issue tracks the review comments mentioned in #8870 :

  • [ ] Use ThemeGenerator to combine all the merged dictionaries ( https://github.com/dotnet/wpf/pull/8870#discussion_r1524158549 )
  • [x] Enhance accent color management ( https://github.com/dotnet/wpf/pull/8870#discussion_r1524771043 , https://github.com/dotnet/wpf/pull/8870#discussion_r1525831435 )
  • [ ] Undo TextEditorContextMenu, DataGridCheckBoxColumn, Window SetResourceReference workarounds ( https://github.com/dotnet/wpf/pull/8870#discussion_r1526098439 )
  • [ ] Document introducing dependency on fonts present in Win11 only ( https://github.com/dotnet/wpf/pull/8870#discussion_r1525128672 )
  • [ ] Handling WindowBackdrop in OnSourceInitialized ( https://github.com/dotnet/wpf/pull/8870#discussion_r1539763302 )
  • [ ] Remove dependency on Application.Current ( https://github.com/dotnet/wpf/pull/8870#discussion_r1539943016 )
  • [ ] Enabling glass frame causes client area and caption button overlap. ( https://github.com/dotnet/wpf/pull/8870#discussion_r1545270901 )
  • [ ] Access keys are not respected and rendered as character

The issues related to control styles ( XAML and resources ) will be tracked separately.

dipeshmsft avatar Apr 04 '24 15:04 dipeshmsft

I see at least two more critical ones:

  • Key names
    • https://github.com/dotnet/wpf/pull/8870#discussion_r1562619789
    • https://github.com/dotnet/wpf/pull/8870#discussion_r1562634445
  • Hardcoded font family names
    • https://github.com/dotnet/wpf/pull/8870#issuecomment-2053688794

As noted a deep comparison with and understanding of upstream WinUI source is really needed.

robloo avatar Apr 13 '24 16:04 robloo

There are a number of outstanding PRs fixing several known issues:

https://github.com/dotnet/wpf/pulls?q=is%3Apr+is%3Aopen+label%3A%22Win+11+Theming%22

Can we get these reviewed and merged in ASAP? Considering the bulk of the Fluent theme is already committed we need to keep it up to date much more quickly. Open PRs not getting merged in is a long-standing issue with this project. Management or team leads need to set aside time to review these specifically.

In this case those open PRs are preventing full feedback of the Fluent theme. The source code needs to be kept up to date so it can be well tested until release. Personally, I would be testing a lot more of it if it was all in one place with latest fixes. (I have an internal Fluent theme on a framework WPF app that will be replaced with this official theme).

robert-abeo avatar Jun 19 '24 13:06 robert-abeo

@robert-abeo, thanks for the point out, actually most of them were already merged in main. We will keep fixing these issues in styles as they come in, but currently our main priority with Fluent theme is to provide a way to make it default. I have been figuring out a way to allow users to access colors and brush resources defined in Fluent style and following that, we would like to get the theme switch API working.

Since you have been actively involved in discussions for the theming changes, I would like to know your inputs on #8932

dipeshmsft avatar Jun 20 '24 05:06 dipeshmsft

@dipeshmsft Thanks, I've joined the other issue discussion.

The PR I'm looking at specifically with the highest priority is: https://github.com/dotnet/wpf/pull/9171. There are a number of controls currently missing Fluent styles. There are a few other nice-to-have bug fixes as well.

robert-abeo avatar Jun 24 '24 22:06 robert-abeo

Thanks @robert-abeo , I am going through the comments of last night. The PR you mentioned is reviewed, but depending on the direction we are going, this may need a bit of change, that's why it's still open. Rest assured, we will get to the close of this PR by Preview 7.

dipeshmsft avatar Jun 25 '24 05:06 dipeshmsft

@robert-abeo

The PR I'm looking at specifically with the highest priority is: https://github.com/dotnet/wpf/pull/9171. There are a number of controls currently missing Fluent styles. There are a few other nice-to-have bug fixes as well.

I think there is a misunderstanding here, the styles mentioned here are not Fluent styled. The styles here are basically copies of Aero2 styles. The ones not marked "Additional styling not needed" are basically the same in Fluent and Aero2, rest of them still needs to be styled.

dipeshmsft avatar Jun 26 '24 22:06 dipeshmsft

The ones not marked "Additional styling not needed" are basically the same in Fluent and Aero2, rest of them still needs to be styled.

Ok, my mistake. Thanks for the clarification. I knew these were Aero2 based styles but I thought they were updated to use Fluent resources where needed.

robert-abeo avatar Jun 26 '24 22:06 robert-abeo

Ok, my mistake. Thanks for the clarification. I knew these were Aero2 based styles but I thought they were updated to use Fluent resources where needed.

We are getting to this.

dipeshmsft avatar Jun 26 '24 23:06 dipeshmsft

Another thing I notice as I start digging into the details more. There are a lot of default styles that don't set template bindings correctly. For example:

https://github.com/dotnet/wpf/blob/d21766ccfcdeb05aa3feeb6a195c01dbf2e1a5fb/src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/ListBoxItem.xaml#L26-L29

Should be:

<Border
  x:Name="Border"
  Padding="{TemplateBinding Padding}"
  Background="{TemplateBinding Background}"
  BorderThickness="{TemplateBinding BorderThickness}"
  CornerRadius="{TemplateBinding Border.CornerRadius}">

The Background/Border properties should be settable on the control itself and carried through to the template. These should NOT be hard-coded.

robert-abeo avatar Jul 23 '24 18:07 robert-abeo

Here is another thing I should have mentioned with the ListBoxItem.

Fluent v1 had no Pill selection indicators. So the whole background was highlighted when an item was selected usually a blue color. When Fluent v2 was released publicly first in the WinUI repository they didn't include the updated "Pill" selection indicator design for ListBox/ListView. It kept the old styles (probably for compatibility) for ListBox but they did update controls like ComboBox. However, Fluent v2 released in Windows 11 had it. But they sometimes use is differently based on controls... in the WinUI 3 [control] Gallery:

ListBox [WinUI 3 Gallery] image

ListView [WinUI 3 Gallery] image

Most 3rd party Fluent v2 libraries ported without the pill because they based on the WinUI repository and didn't really notice to update in Windows 11. But most apps in Windows 11 do use the pill selection indicator in lists:

image

Therefore, it needs to be discussed what the default needs to be: Pill or no Pill. And then we also need to have separate "Pill" styles available as well. So we should perhaps have PillListBoxItem or AlternateListBoxItem keyed resource (assuming it isn't the default).

robert-abeo avatar Aug 01 '24 13:08 robert-abeo

I've opened up separate issues to better track what I've found so far. This is only a spot check as things comes up. It will be some weeks before we can switch to an internal theme heavily based on what is in the upstream WPF repo here. At that point I'll probably have more specifics. Hopefully, there can be fixes for next year in the .NET 10 time-frame. Seems everyone is out of time at this point for .NET 9.

  • https://github.com/dotnet/wpf/issues/9561
  • https://github.com/dotnet/wpf/issues/9562
  • https://github.com/dotnet/wpf/issues/9563
  • https://github.com/dotnet/wpf/issues/9564

robert-abeo avatar Aug 13 '24 16:08 robert-abeo