Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using XamlColorSchemeGenerator to generate themes and merging AppTheme and Accent to Theme #3296

Merged
merged 7 commits into from
Aug 25, 2018

Conversation

batzen
Copy link
Collaborator

@batzen batzen commented Aug 5, 2018

Currently you have to choose between BaseDark and BaseLight in combination with an accent like Blue.
This makes creating custom themes much more difficult than it should be because the so called AppTheme and Accent depend on each other.
To make it easier the new theming will work with a Theme only.
Such theme then contains all colors and brushes needed.

This means that instead of referencing two files you just have to reference something like Dark.Blue or Light.Blue.
The ThemeManager was also changed to reflect these changes.
To maintain interop with Fluent.Ribbon these changes will also be made there.

@PhyxionNL
Copy link

I'm not sure if Light.Blue is the best naming for it, one might think it's light blue as a color. Perhaps BlueDark/BlueLight would be better?

@batzen
Copy link
Collaborator Author

batzen commented Aug 6, 2018

I got no personal preference about the ordering of those words, just that there should be a separator.
So i would also be fine with "Blue.Dark", but it might cause the same confusion.
The reasoning for me to choose "Dark.Blue" was that it's "BaseColor.ColorScheme" ;-)

@PhyxionNL
Copy link

Blue Dark isn't a color (or Blue Light). Light Blue and Dark Blue are. So I think that should help some.

As for this change, I can see this being useful to some degree, but as you're now generating them (with Light and Dark only), won't the current way work? That's easier to understand (no confusion possible really) and is also more similar to how UWP is doing it.

@batzen
Copy link
Collaborator Author

batzen commented Aug 6, 2018

Blue Dark isn't a color (or Blue Light). Light Blue and Dark Blue are. So I think that should help some.

I get that point.
Lets see what @punker76 prefers. Changing the file names only takes a few seconds and a new version of XamlColorSchemeGenerator ;-)

The reasoning behind the combined file is:

  • Creating and maintaining themes is easier because you only need one file (and only one template file)
  • Creating complex themes where you have to use different colors based on the base color scheme is very painful because the AppTheme and Accent can currently be changed independently using one file eliminitates all dependencies between those two files because you only have one. In Fluent.Ribbon i had to introduce quirks and workarounds for these cases.

(with Light and Dark only)

Currently yes, but now we can easily introduce DarkGray etc. easily.

won't the current way work?

It would, but you end up with two tightly coupled files which depend on each other.
In MahApps.Metro it was a bit worse than having two files because you had to use Colors.xaml, BaseLight.xaml and your accent theme. Omitting Colors.xaml would have omitted several colors/brushes.
Maintaining one file/template is also way less error prone than having to keep an eye on three different files.

That's easier to understand (no confusion possible really) and is also more similar to how UWP is doing it.

Only developers see this. As you can see in the showcase application it's abstracted away for the user.

@punker76 punker76 added this to the 2.0.0 milestone Aug 25, 2018
@punker76 punker76 merged commit 69c80f3 into MahApps:develop Aug 25, 2018
@FinlayDaG33k
Copy link

I agree with Blue.Dark or Blue.Light, it's a lot easier to remember for developers since (as @Phyxion already mentioned), Light.Blue and Dark.Blue can easily be interpreted as the colors Light Blue and Dark Blue, especially by newer developers.

punker76 added a commit that referenced this pull request Dec 11, 2018
@batzen batzen deleted the Theming branch January 6, 2019 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants