search

MaterialDesignInXAML / MaterialDesignInXamlToolkit

Google's Material Design in XAML & WPF, for C# & VB.Net.
http://materialdesigninxaml.net
MIT License
15.23k stars 3.44k forks source link

PrimaryBody and SecondaryBody colors for theme #1757

Open JorisCleVR opened 4 years ago

JorisCleVR commented 4 years ago

For my project I am currently writing a style for RadioButton that is styled as a ChoiceChip (https://material.io/components/chips#choice-chips) Currently I have the following result: image

As you can see the light theme is nice, but unfortunately the foreground color of the dark theme does not have enough contrast. The problem is that for light theme I need the foreground to be PrimaryHueDarkBrush, but for the dark theme it must be PrimaryHueLightBrush. (Same for Secondary) I currently think the best way to achieve this is by introducing a PrimaryBody and SecondaryBody color to the theme. The naming because it reacts in a similar way as currently the MaterialDesignBody color does.

I am willing to look into creating a PR with the necessary changes if you also see this as a good solution. Otherwise I hope you can come up with a better solution to fix this issue.

Keboo commented 4 years ago

Hi @JorisCleVR this is an interesting problem. This is not the first time we have run into an issues like this with dark theme colors. Let me sleep on it, and I will get back to you tomorrow.

JorisCleVR commented 4 years ago

First of all thanks for looking into it. In the mean time I integrated the code for the ChoiceChips into a branch: https://github.com/CleVR-bv/MaterialDesignInXamlToolkit/tree/feature/MaterialDesignChoiceChipListBox

When we solve the problem with the coloring I could make a pull request for it.

I tried to keep it close to the specifications on https://material.io/components/chips: image

Keboo commented 4 years ago

So coming back around to this. I think the solution is to handle the opacity in the theme brushes which is the source of several outstanding bugs. There is an internal converter in the library RemoveAlphaBrushConverter that can handle removing the alpha channel from an RGBA color, and returning back just the resulting RGB color. I have been favoring this approach to dealing with the colors since it allows for a more consistent appearance especially with custom or user controlled themes. I would suggest using that for the foreground/background on the colored version of the choice chips to ensure a consistent look in light and dark theme. Please note, I have not tested this on that control and would be very curious to see how it looks (let me know if you have any questions on this).

Part of the issue that I keep running into with additional foreground hue brushes is backwards compatibility (how to handle with the existing 2014 themes), and accessibility. Though it is certainly possible to add additional theme brushes I want to be cautious about doing so.

JorisCleVR commented 4 years ago

It is good to think about backwards compatibility and accessibility :). I tried to use the RemoveAlphaBrushConverter (See https://github.com/CleVR-bv/MaterialDesignInXamlToolkit/tree/feature/ChoiceChip-RemoveAlphaBrushConverter)

Unfortunately I do not seem to be getting much result: image

I used the ComboBox style as an example on how to use it. Am I doing something wrong or is it just not applicable for this case?

Keboo commented 4 years ago

Ah I see some of the difficulty. The opacity is not entirely from the brushes and is also being directly applied to the borders that make up the background as well.

I put together a small update with your code and a value converter that can detect dark/light theme. This allows for inverting the Foreground/Background. I played around with the colors a bit and suspect that this is only part of the solution. Just inverting the brushes doesn't seem to give quite the desired result since the opacity values seem a little off. Thoughts?

JorisCleVR commented 4 years ago

Yes, that is the difficulty of the choice chip component. Furthermore Material.io is not entirely clear about how the colors are build up.

I see what you are trying to do. Currently it also changes the background colors (and also in not selected state). The thing is I only want to change the foreground color. I am trying to think of a way we could change the dark color into a light one with a converter. But so far I have not been able to come up with a solution.

Need to do some more tests I guess. If you come up with any more ideas please let me know.

JorisCleVR commented 4 years ago

Tried to come up with some solution using a color converter. But unfortunately I don't seem to find a good way to get a good consistent change in the foreground color. Furthermore I was thinking if there somewhere is a boolean that can be checked whether dark mode is enabled? I could then create a converter that switches to the light color when dark mode is enabled. Not a great fan of that solution, but I seem to be getting out of options...