feat: Dark theme support

[wal33d006]

After a lot of thought, I added a converter for theme so that the consumer application now can use CosmosTheme completely without having to use Flutter's theme for some parts. In order to make that happen, the consumer app will have to inject the CosmosThemeData in order to be able to change themes rather than replying on operating system's theme settings. The reason for doing this was the fact that if the consumer application uses CosmosTheme.of(context).colors in their application, then the system theme would not reflect the colors from CosmosTheme as they would not have been injected from outside, therefore leading to discrepancies.

Now what ideally will happen in the consumer app is this:

In case of dark theme:

CosmosTheme(
   themeData: CosmosTheme.darkThemeData
   ...
   child: Builder(builder: (context) {
        return MaterialApp(
          theme: CosmosTheme.buildTheme(context),
         ...

and in case of light theme:

CosmosTheme(
   themeData: const CosmosThemeData(),
   ...
   child: Builder(builder: (context) {
        return MaterialApp(
          theme: CosmosTheme.buildTheme(context),
         ...

This can be handled on a toggle somewhere in consumer app's settings. I hope I am able to communicate what I have in my mind. Do let me know if there is any better approach or a better way to code this approach.

[Zfinix]

After a lot of thought, I added a converter for theme so that the consumer application now can use CosmosTheme completely without having to use Flutter's theme for some parts. In order to make that happen, the consumer app will have to inject the CosmosThemeData in order to be able to change themes rather than replying on operating system's theme settings. The reason for doing this was the fact that if the consumer application uses CosmosTheme.of(context).colors in their application, then the system theme would not reflect the colors from CosmosTheme as they would not have been injected from outside, therefore leading to discrepancies.

Now what ideally will happen in the consumer app is this:

In case of dark theme:

CosmosTheme(
   themeData: CosmosTheme.darkThemeData
   ...
   child: Builder(builder: (context) {
        return MaterialApp(
          theme: CosmosTheme.buildTheme(context),
         ...

and in case of light theme:

CosmosTheme(
   themeData: const CosmosThemeData(),
   ...
   child: Builder(builder: (context) {
        return MaterialApp(
          theme: CosmosTheme.buildTheme(context),
         ...

This can be handled on a toggle somewhere in consumer app's settings. I hope I am able to communicate what I have in my mind. Do let me know if there is any better approach or a better way to code this approach.

I'll look into the code more to give suggestions

[andrzejchm]

we have to decide we either follow flutter's theme or ditch it completely in favor of CosmosTheme that gives us much more flexibility. Since we already introduced the CosmosTheme I don't see the reason to conform to Flutter's theme.

What we can do instead, is to provide CosmosTheme with two separate theme data, one for light theme and the other for dark theme. we can also add a brightnessResolver to the CosmosTheme that will help determine which theme data to use in CosmosTheme.of(context). i.e:

CosmosTheme(
   themeData: CosmosThemeData(...),
   darkThemeData: CosmosThemeData(...),
   brightness: () => Brightness.dark, // this makes it always dark

   // OR 

   brightness: () => MediaQuery.of(context).platformBrightness, // this uses system's brightness, CAUTION: in order for it to work, the `CosmosTheme` needs to be wrapped with `MediaQuery.fromWindow(..)` widget
)```

[wal33d006]

we have to decide we either follow flutter's theme or ditch it completely in favor of CosmosTheme that gives us much more flexibility. Since we already introduced the CosmosTheme I don't see the reason to conform to Flutter's theme.

What we can do instead, is to provide CosmosTheme with two separate theme data, one for light theme and the other for dark theme. we can also add a brightnessResolver to the CosmosTheme that will help determine which theme data to use in CosmosTheme.of(context). i.e:

CosmosTheme(
   themeData: CosmosThemeData(...),
   darkThemeData: CosmosThemeData(...),
   brightness: () => Brightness.dark, // this makes it always dark

   // OR 

   brightness: () => MediaQuery.of(context).platformBrightness, // this uses system's brightness, CAUTION: in order for it to work, the `CosmosTheme` needs to be wrapped with `MediaQuery.fromWindow(..)` widget
)```

In this case, we won't be giving a theme to MaterialApp's theme property, right? If yes, the theme won't even apply because we have to give something to MaterialApp's theme property which right now is being done by CosmosTheme.build method. Also, if we keep both darkTheme and themeData parameters for CosmosTheme widget, how do we switch between themes? Through brightness? What would be the toggle? Will it be system's theme? A little confused about this.

[andrzejchm]

Good questions @wal33d006 ! So to reiterate:

1. Get rid entirely of CosmosAppTheme class with all its contents.
2. in starport template, update all the uses ofTheme.of(context) to CosmosTheme.of(context) and add missing equivalents (especially, textStyles is missing)
3. In starport template, let's get rid of theme: CosmosTheme.buildAppTheme(), inside the StarportApp widget and see how it affects the overall appearance of the app
4. Let's remove the CosmosTheme#buildAppTheme and CosmosTheme#buildDarkAppTheme methods

The brightness param, as mentioned in my previous comment would be responsible of determining what is the current brignthess of the app. This way we can either set it up to use the system's brightness by providing:

brightness: () => MediaQuery.of(context).platformBrightness,

or make it constant, despite the system's brightness:

brightness: () => Brightness.dark,

or get the brightness from the app's custom settings

brightness: () => _appSettings.brightness,

[wal33d006]

@andrzejchm My findings after doing all of the stuff that you recommended:

Just removing Theme.of(context) might not be enough. We will have to explicitly apply colors/theme to all widgets which we use from within Flutter's framework, one such example is Scaffold, at every page we will have to do this:

...
final theme = CosmosTheme.of(context);
...
Scaffold(
   backgroundColor: theme.colors.background,

I removed the theme from the template and let it use CosmosTheme entirely, and I figured out we will have to update every widget (all those widgets the colors/theme of which we are not defining explicitly) in our design system to use colors from CosmosTheme.of(context), because by default they internally use Flutter's theme, one example is the CosmosElevatedButton consumed like this:

CosmosElevatedButton(
  text: 'Create account',
  onTap: _onTapCreateAccount,
),

and defined like this:

...
ElevatedButton(
      onPressed: onTap,
      style: ElevatedButton.styleFrom(
        primary: backgroundColor,
        onPrimary: textColor,
        fixedSize: Size.fromHeight(height),
        shape: RoundedRectangleBorder(borderRadius: theme.borderRadiusM),
        elevation: elevation ?? 0,
        shadowColor: shadowColor ?? theme.colors.shadowColor,
      ),
...

here all the color properties are nullable and are not being passed from outside so it automatically picks up the default Flutter theme colors which would be light/blue theme colors (I just tried doing that and everything went blue/white) because we aren't passing anything inside MaterialApp's theme property.

Conclusion: We can update every widget in our cosmos_ui_components to explicitly use colors/theme/styles from CosmosTheme.of(context) and NOT leave them unapplied (This will be an intensive task but it will be worth it)

[andrzejchm]

great findings @wal33d006 ! Yeah, it will be lengthy process but I believe its definitely worth it, as we will have much more control over the process. what we can do, is to gradually update the core widgets one by one with separate Pull requests to use the CosmosTheme instead of implicitly relying on flutter's theme. I'll then add some screenshot tests to the cosmos_ui_components package to make sure those stay consistent and are prone to any regressions

[wal33d006]

Awesome! So in this PR I have gotten rid of everything related to CosmosAppTheme and the theme converter I created

[andrzejchm]

as first step you can also simply mark the buildAppTheme and buildDarkAppTheme as deprecated

[wal33d006]

Marked the functions as deprecated, and added a new buildTheme function in order for the consumer app to use the theme converter to be able to successfully use both dark/light themes using a toggle of their own. (This is only for now until we convert everything to use CosmosTheme)

[Zfinix]

I believe this is super neat and ready 🍾