jithware / brethap

https://jithware.github.io/brethap
GNU General Public License v3.0
50 stars 11 forks source link

Dark mode overrides colour selection #37

Closed jacfod closed 2 years ago

jacfod commented 2 years ago

Describe the bug

Primary colour and Background colour do not change if the phone is set to dark mode

To Reproduce

Steps to reproduce the behavior:

  1. Set phone to dark mode
  2. Click on preferences
  3. Pick a background colour/ primary colour
  4. See error

Expected behavior

The colours would change with after confirming my choice

Smartphone (please complete the following information):

Additional context

It might be because Flutter's theme is overridden by Samsung's colour choice? I offered to look through the code in my review, though I have never programmed using Flutter, so I do not think I would be of much help.

If I'd have to guess, I would say that GetMaterialApp function in main.dart seems like a likely suspect.

return GetMaterialApp(
      debugShowCheckedModeBanner: false,
      theme: ThemeData(
        primarySwatch: primaryColor,
        canvasColor: backgroundColor,
      ),
      darkTheme: ThemeData(
        brightness: Brightness.dark,
        primaryColor: primaryColor, // The problem might be here.
      ),
      localizationsDelegates: AppLocalizations.localizationsDelegates,
      supportedLocales: AppLocalizations.supportedLocales,
      home: HomeWidget(
          appName: appName,
          version: version,
          preferences: preferences,
          sessions: sessions),
    );
jithware commented 2 years ago

@jacfod thanks for taking a look at this. Dark mode has been brought up in the past by @wolftune (see #2, #18, #23, #25 ). Part of the problem is that when in dark mode, Get.changeTheme() won't change the theme (see https://github.com/jithware/brethap/issues/25#issuecomment-1001010151). Here's some possible solutions:

  1. Update the README.md to state that when in dark mode, colors will be overridden by the default system dark mode colors and only the circle color is changeable. If you are currently in dark mode color change will take affect upon application restart.
  2. Remove the darkTheme entirely. If user wants dark colors they can select a combination of black/greys. If you are currently in dark mode color change will take affect upon application restart.

It is possible to add a preference selector like light/dark/system, however the only change in app behavior would be the dark selection would be some color combination I choose, which can already be done dynamically by the user. Application restart is still necessary if in dark mode.

Could also do a selector of light/system, but presumably if the user is in dark mode, they are not going to want the light selection otherwise they would just go back to light mode in the system.

Thoughts?

wolftune commented 2 years ago

It gets messy, but it could make sense to have a quick setting to choose from 3 options: light, dark, or follow-system. And then separate color choices for "light" and for "dark" that are remembered.

jithware commented 2 years ago

Decided in the spirit of keeping the app simple, to go with a combination of both 1 and 2. When the Get package supports changing the theme while in dark mode, will be able to revisit better support of the dark theme.

In release >= 1.5.1+31