benruehl / adonis-ui

Lightweight UI toolkit for WPF applications offering classic but enhanced windows visuals
https://benruehl.github.io/adonis-ui/
MIT License
1.71k stars 143 forks source link

Add AccentToggleButton #58

Closed Swiftpaws closed 4 years ago

Swiftpaws commented 4 years ago

Currently the togglebutton checked state is hard to see. Instead of changing the existing design I decided to createthe accented toggle button as a new named style.

This style is based on the current toggle button - but for that to work properly I had to split the base style into two separate styles. The second one only has the triggers which I do not want in the accented style because the accent has no layers.

Swiftpaws commented 4 years ago

I have also incorporated the button in the sample app

benruehl commented 4 years ago

Thanks for contributing and welcome on board!

Currently I don't have the opportunity to test your changes, but I hope I can get to it within the next few days. Until then I can only look at the code on GitHub.

So what I noticed is that you produced an immense amount of formatting changes within your commit. It is kind of hard for me to spot the actual changes you did because they are surrounded by ~300 lines of changes where just a single space (' ') has been added.

There are also changes like these in there:

<!-- from -->
Margin="{adonisUi:Space 1, 0, 0, 0}"

<!-- to -->
Margin="{adonisUi:Space 1,
                        0,
                        0,
                        0}"

... which actually looks worse in my opinion.

I don't really mind if you want your lines of xaml to end with space like <Foo /> or without like <Foo/>. But please don't unnecessarily change all the formatting others have chosen for their code. My guess is that those changes were not made on purpose but instead by some formatter of your IDE. But please understand that tracking actual changes becomes hard when there is so much unnecessary 'noise' in the history.

I hope I do not sound too harsh here. Your changes are really welcome in general and adding an AccentToggleButton makes totally sense.

Okay, so how do you get rid of these formatting changes as they are already commited? I think the best option would be to reset you HEAD using git reset --soft. Using this you can reset your commit but keep the changes, then stage only what you really changed and commit again. Force push this commit to your branch afterwards.

Hope this helps.