Closed Swiftpaws closed 4 years ago
This looks much better now, thanks.
Meanwhile I was able to look at the changes directly in the demo app. There are a few things I noticed here.
Minor issues:
AccentButton
relies on the default button style as well which is not splitted.AccentHighlightBrush
. This is ok when it is in checked state because it transitions from the AccentBrush
. But when it is not checked there should be the cursor spotlight displayed in the background instead to be consistent with all other controls.ControlTemplate
can be moved to the style's triggers. Having those triggers directly in the ControlTemplate
protects them from being overridden by deriving styles. That's why it should be only done when it is absolutely required, e.g. when a part of the template needs to be referenced. But this seems to be not the case here.Style name:
In addition, I am struggling with the style's name. When reading the name AccentToggleButton
I would expect the ToggleButton to look just like the AccentButton
. I would expect it to have Background="AccentBrush"
all the time and not just when being checked. The AccentButton
is not just a highlighted version of the default button but is also designed to look good when placed on accented surfaces. This is something the AccentToggleButton
should apply to as well, but does not currently.
As I understood you, it is not your intention to create a complete accent version of the ToggleButton. But instead you aimed at a better visible checked state only. That's totally fine. But I think this intention should be embodied by the name. Also to have the name "AccentToggleButton" left for the complete accented version in case it will be implemented in the future.
I'd like to suggest the name DefaultToAccentToggleButton
which, in my opinion, covers the change between those two looks better. Please feel free to suggest names as well.
I think we both had different visions for the accented version. - This is also why I did not add the toggle button to the layering demo. (No layering support on this version)
That said I will go ahead and re-design it if I find the time.
I will open another pr for this because its easier for me
As you have already guessed correctly the previous pull request had formatting from code maid applied to it - this time around it should be much easier to spot the changed made.
I just re-forked and applied the changes again.