Open deinspanjer opened 3 years ago
Yeah, that would be fine. I think, in addition to data
parameter in AppTheme
we can provide an optional darkData
parameter or something equivalent. So, if that param is not present we can default to using the theme in data
in both light and dark modes.
Would you be fine with sending a PR? If not I guess I'll try to add this within few weeks.
Yes, I am working on a feature branch in my fork that implements support for light, dark and high contrast variants of themes in a subclass of AppTheme called CompositeAppTheme. I should have that wrapped up and available as a PR next week.
I just bumped into your library while looking for references on how to properly get MaterialApp to be a consumer of a ChangeNotifyProvider that is providing theme data.
It looks like you've already written a lot of the same code I have, and I'm looking at switching over to your library instead of my custom code.
One thing I've noticed is different with your implementation is that you don't currently make any use of Material's ThemeMode. This enum has 3 values,
system
,light
, anddark
. The default for MaterialApp issystem
. If the system is set to use a dark theme (i.e Android system preferences, or the Browser theme default) then MaterialApp will attempt to use theThemeData
provided indarkTheme
. If that property is not set, then it will default totheme
.What this allows is to have an overall theme but apply a light and dark variant of it, and set both of them in the MaterialApp properties
theme
anddarkTheme
, then either let the system default choose, or if the user wishes to override, the way that override is implemented is by changing thethemeMode
property to eitherThemeMode.light
orThemeMode.dark
.Here is an example of my AppTheme model that can use this. Would you be interested in changes to your library to try to take ThemeMode into account?