AvaloniaUI / Avalonia

Develop Desktop, Embedded, Mobile and WebAssembly apps with C# and XAML. The most popular .NET UI client technology
https://avaloniaui.net
MIT License
26.17k stars 2.27k forks source link

Merge DataGrid and ColorPicker themes into a single theme per control #10148

Open maxkatz6 opened 1 year ago

maxkatz6 commented 1 year ago

Is your feature request related to a problem? Please describe.

Right now, for both of these controls we have Fluent and Simple theme variations. It adds, additional confusion on user. And makes it problematic to implement something like "generic.xaml" which would be included with the project on package reference, as we have two different "root xaml" files per each control. It also won't work nice with third party controls that might not even have dependency on build-in themes. As a reminder, Simple and Fluent theme now are completely optional.

Describe the solution you'd like

Drop "simple" variation, make "fluent" one default without any "fluent" in the name. As we have theme dictionaries support, these controls can define their own resources per theme variant. Also, it would require fixing some current issues we have with Fluent DataGrid theme - https://github.com/AvaloniaUI/Avalonia/issues/9791.

Describe alternatives you've considered

I don't see any aside from continuing what we have now.

maxkatz6 commented 1 year ago

CC @robloo as you have implemented ColorPicker. If you don't see any problems with this idea, I plan to do these changes on the color picker too. It's simpler there, as simple and fluent themes are pretty much identical aside from depending on one or another resources set.

robloo commented 1 year ago

A few thoughts:

  1. Overall, I've been wanting to remove the Simple theme for a while. I don't think it should be used in production apps for a number of reasons and its holding back the quality bar of Avalonia for those that use it. However, I spent some time getting the ColorPicker to work in simple theme so it would be a waste if this was just deleted at this point. Is your plan to completely remove the Simple styles? Or will you still keep them if an app wants to reference directly?
  2. Are you planning to merge all files together into one? I would prefer to keep theme separate and have just a generic.xaml equivalent references individual files for each control. ThemeVariant light/dark resources of course can be centralized.
  3. I'm not a fan of dropping Fluent from the name... the designs go with Fluent and more importantly... Fluent resources are required. There is a dependency there on the Fluent nuget and I don't think duplicating resources is a good idea.
  4. While the theme variants help for light/dark there still isn't a solution to plug-in to control themes like Fluent or Simple. It seems the real solution is:
    1. Provide a real mechanism to plug-in to an existing control theme set. That would require setting some global "current control theme name" globally. Then additional packages could register new control themes if they support the globally set name.
    2. Move the control themes for DataGrid and ColorPicker to the actual themes nuget packages. This is what FluentAvalonia does and it works just fine. I don't think there is a lot of overhead doing this.

So bottom line, it seems theme dictionaries didn't fully solve this issue. It handled the light/dark/etc. cases nicely but those are only variations of a theme. We need a way to register and globally define the actual control theme applied: Simple or Fluent. Then a way to register additional control themes for 3rd party controls based on the active control theme. (theme terminology is really hard to use here... still wish we went with different naming).

workgroupengineering commented 1 year ago

Hi, my two cent. In a small test project i experimented with success following method to resolve this issue:

In shared library i define class like this:

public class Theme 
{
    public static readonly AttachedProperty<string?> NameProperty =
        AvaloniaProperty.RegisterAttached<Theme, StyledElement, string?>("Name"
            , inherits: true);

    public static string? GetName(StyledElement element) =>
        element.GetValue(NameProperty);

    public static void SetName(StyledElement element, string? value) =>
        element.SetValue(NameProperty, value);
}

on my custom control add style like this:

<Styles xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        x:Class="MyControl.Generic"
        xmlns:l="using:MyControl"
        xmlns:t="using:ThemeManager">
    <!-- Add Styles Here -->
  <Styles.Resources>
    <ControlTheme x:Key="{x:Type l:AwesomControl}" TargetType="{x:Type l:AwesomControl}">
      <Setter Property="Template">
        <ControlTemplate>
          <Rectangle Width="100" Height="100" x:Name="my" Fill="{TemplateBinding Foreground}">
          </Rectangle>
        </ControlTemplate>
      </Setter>
    </ControlTheme>
  </Styles.Resources>

  <Style Selector=" l|AwesomControl[(t|Theme.Name)=FluentLight]">
    <Setter Property="Foreground" Value="Green"/>
  </Style>

  <Style Selector=" l|AwesomControl[(t|Theme.Name)=FluentDark]">
    <Setter Property="Foreground" Value="White"/>
  </Style>

</Styles>

with code behind allow to autoload the control style Generic.axaml:

using Avalonia.Styling;
using System.Runtime.CompilerServices;

namespace MyControl;

internal partial class Generic : Styles
{
    [ModuleInitializer]
    internal static void ThemeLoader()
    {
        // ...
        if (Avalonia.Application.Current is { } app)
        {
            app.Styles.Add(new Generic());
        }
        else
        {
            Avalonia.Threading.Dispatcher.UIThread.Post(ThemeLoader);
        }
    }
}

in App.axaml.cs

        public override void OnFrameworkInitializationCompleted()
        {
            if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop)
            {
                var root = new MainWindow();
                root.SetValue(ThemeManager.Theme.NameProperty, "FluentLight");
                desktop.MainWindow = root;
            }

            base.OnFrameworkInitializationCompleted();
        }

when change theme:

                if (App.Current!.ApplicationLifetime is IClassicDesktopStyleApplicationLifetime lifetime)
                {
                    var modeName = string.Empty;
                    var current = App.Current.Styles[0];
                    var style = current; ;
                    switch (currentMode)
                    {
                        case FluentThemeMode.Light:
                            modeName = "FluentLight";
                            style = FluentLight;
                            break;
                        case FluentThemeMode.Dark:
                            modeName = "FluentDark";
                            style = FluentDark;
                            break;
                        default:
                            break;
                    }
                    App.Current.Styles[0] = style;
                    lifetime.MainWindow!.SetValue(ThemeManager.Theme.NameProperty, modeName);
                }

I am attaching a small example project aligned to version 11.0.0-preview4

The Generic.axaml.cs can be generate by source generator or XamlC

ATheme.zip

robloo commented 1 year ago

@workgroupengineering Very interesting example and it is similar to what I had in mind.

workgroupengineering commented 1 year ago

@workgroupengineering Very interesting example and it is similar to what I had in mind.

* The only major issue I see is the combining of the "theme" with the "theme variant". I would think the registered theme name would be "Fluent" and the variant light/dark could be switched out using the existing theme dictionaries.

We can add Variant AttachedProperty

* ... However, I'm not sure it would work well when there are major differences between Simple/Fluent theme implementations. It would get to the point the entire control theme itself needs to be switched out rather than customizing a shared one.

I haven't had the chance to test it, I'll do it as soon as I have some more time.

robloo commented 1 year ago

Well, keep in mind Variant information is always available now with the Requested/ActualThemeVariant properties. It probably doesn't make sense to add another attached property for that.

maxkatz6 commented 1 year ago

Is your plan to completely remove the Simple styles? Or will you still keep them if an app wants to reference directly?

My plan to delete simple, yes. And delete fluent too. Specifically for these two controls, which are detached from the main core. Instead, I want to keep single theme with set of resources defined in it, which doesn't depend on the specific theme. Which means, same default DataGrid theme can be used with both Fluent and Simple, but also with Material, FluentV2 and others.

I'm not a fan of dropping Fluent from the name... the designs go with Fluent and more importantly... Fluent resources are required

DataGrid fluent theme has a little to do with fluent design itself. It's just a touch friendly data grid that fits into overall style. I can agree about ColorPicker which uses also uses other build-in controls like buttons. And it will be a bit tricky with control themes like ColorViewColorModelRadioButtonTheme. Similarly. how DataGrid has set of DataGrid*** resources, ColorPicker can define minimal set of its own resources for both themes variants.

And currently, in that way it's still possible to customize via:

That would require setting some global "current control theme name" globally. Then additional packages could register new control themes if they support the globally set name.

But isn't it possible to override default control theme by defining new resource with control type as a key? Like it was possible in UWP. Like, we can have MyCustomDataGrid.xaml:

<ControlTheme TargetType="DataGrid" x:Key="{x:Type DataGrid}" />

AFAIK you won't be able to switch global theme on the fly, but statically it should just work.

In the end, custom themes either can be defined as an addition to the default ones, which can override specific control themes or brush resources:

<Application.Styles>
   <FluentTheme />
   <StyleInclude Sourec="...Default/DataGrid.xaml" />
   <CitrusTheme />
</Application.Styles>

Or complete replacement like FluentAvalonia or Material.Avalonia.

Move the control themes for DataGrid and ColorPicker to the actual themes nuget packages. This is what FluentAvalonia does and it works just fine. I don't think there is a lot of overhead doing this.

Yes, it works. But there is still trimming overhead. As having a DataGrid used in the themes will keep DataGrid type from being trimmed out, even if it's not used in the app. Something that is pretty hard to solve in Avalonia with modern .NET tooling.

Saying that, there is actually one problem that we haven't solved yet, and that's high level API for the accent colors. Which can be overridden and used in third party controls regardless of the theme.

maxkatz6 commented 1 year ago

@workgroupengineering having current theme variant information in selectors is something that might be possible one day. I.e. with media queries. As an alternative to the resource-first approach of theme dictionaries.

robloo commented 1 year ago

I'll dig through this more when I have time. But the more I think about it for ColorPicker the more I realize how it works now is actually the best until there is a way to plug-in to existing themes.

Ideally, Avalonia's FluentTheme and SimpleTheme would have extensibility API. ColorPicker would take a dependency on FluentTheme and then register some additional styles it needs probably using Avalonia as an intermediate. Change notification is needed too as you said.

Right now though it's possible for apps to have full control and they can decide to use Simple theme for Fluent v1 Theme. Then they can dynamically change as they wish. ColorPicker can keep using resources from Fluent as it currently does (I'm strongly against duplicating these and moving to them to the ColorPicker, that is not a design or architecture direction that makes sense or is sustainable).