dynamic_theme icon indicating copy to clipboard operation
dynamic_theme copied to clipboard

master

Open ThinkDigitalSoftware opened this issue 4 years ago • 28 comments

Add PlatformBrightness widget to allow automatic updating of the app's theme when the platform changes it's brightness.

ThinkDigitalSoftware avatar Nov 11 '19 01:11 ThinkDigitalSoftware

How does the app get notified when the system brightness changes? And how is the PlatformBrightness integrated with the DynamicTheme? I can't seem to find a connection.

Norbert515 avatar Nov 12 '19 09:11 Norbert515

@Norbert515 , There was an API to be notified via callback, however, it required platform integrations and it only worked on Android Q. This was a complex solution, so I found out that there's a way to check for brightness. It's a simple builder that always rebuilds with the current platform Brightness setting. There's no notification, it just stays current anytime the widget tree rebuilds. I couldn't find anything to listen to unless I created my own stream, but it would only be updated during a build.

I tested it by putting a breakpoint in the builder of the PlatformBrightness widget and then went to settings and changed the theme. The builder runs immediately after opening the app again and it contains the new value. The only other way to test this without switching screens is if you did split-screen and changed it, but I still believe that this widget is called.

ThinkDigitalSoftware avatar Nov 12 '19 20:11 ThinkDigitalSoftware

And how is the PlatformBrightness integrated with the DynamicTheme? I can't seem to find a connection.

What do you mean by that? I didn't make it a part of the existing widget because of separation of concerns. DynamicTheme is for controlling the app's theme, and PlatformBrightness is for having awareness of the system setting, which is different from the app's theme

ThinkDigitalSoftware avatar Nov 12 '19 20:11 ThinkDigitalSoftware

Sounds good! But should the DynamicTheme also adapt to the platform brightness (maybe with a flag to toggle that behavior)?

Norbert515 avatar Nov 14 '19 10:11 Norbert515

That should be up to the user. I believe it's only job should be to provide the current value, like orientation builders

ThinkDigitalSoftware avatar Dec 10 '19 02:12 ThinkDigitalSoftware

~~didPlatformChangeBrightness does look like the official way to do this actually~~

Oh, Nevermind. These are all methods. We're trying to provide a widget with the most recent value

ThinkDigitalSoftware avatar Dec 24 '19 16:12 ThinkDigitalSoftware

Sure, you can use onPlatformBrightnessChanged to get the most recent value.

Reprevise avatar Dec 24 '19 17:12 Reprevise

I don't see how it's better than my suggestion solution. When the platformBrightness changes, MediaQuery rebuilds its descendants with that, which my widget depends on, so it works. No callbacks needed

ThinkDigitalSoftware avatar Dec 24 '19 18:12 ThinkDigitalSoftware

Okay so if I understand this correctly, the PlatformBrightness widget gets placed somewhere and provides the brightness. Right now, I'm thinking of apps like Gmail that let you choose between the system theme and dark and light mode. Would that be possible with this change?

Reprevise avatar Dec 24 '19 18:12 Reprevise

Okay so if I understand this correctly, the PlatformBrightness widget gets placed somewhere and provides the brightness. Right now, I'm thinking of apps like Gmail that let you choose between the system theme and dark and light mode. Would that be possible with this change?

That's what the original DynamicTheme offers already. The ability to change the apps's theme manually and dynamically. The new widget I'm proposing taps into the system's brightness mode and returns that. You can use this to make your app switch themes based on the system brightness instead of manually. There are some apps, like Google search, that automatically turn to dark mode if the system UI is dark. or they have manual modes too.

ThinkDigitalSoftware avatar Dec 24 '19 18:12 ThinkDigitalSoftware

I forked the repo and added your changes to it. I also added to the example. I think I'm doing it wrong but your builder always returned Brightness.light. It would be great if you could check it out and run it when you get time :)

Reprevise avatar Dec 24 '19 18:12 Reprevise

What are you doing to view the changes? You have to go into system settings on the phone and change the theme to dark mode . This is an android pie or q thing

ThinkDigitalSoftware avatar Dec 24 '19 18:12 ThinkDigitalSoftware

I'm running an emulator w/ Android 10 and changing the theme to dark mode.

Reprevise avatar Dec 24 '19 19:12 Reprevise

I'm running an emulator w/ Android 10 and changing the theme to dark mode.

OK, let me retest

ThinkDigitalSoftware avatar Dec 24 '19 19:12 ThinkDigitalSoftware

Here's my test on a pixel physical device on Q. 20191224_122425

ThinkDigitalSoftware avatar Dec 24 '19 20:12 ThinkDigitalSoftware

Can I see your code?

Reprevise avatar Dec 24 '19 20:12 Reprevise

Sure, I just pushed another commit with the example added https://github.com/Norbert515/dynamic_theme/blob/0cedc2233014ca9f74aa7b1a37ce1fb0c0bb5fe0/example/lib/main.dart is the example file

ThinkDigitalSoftware avatar Dec 24 '19 20:12 ThinkDigitalSoftware

Alright, I see. Only issue I see is that it doesn't work above MaterialApp so users are going to have to wrap all of their routes with a consumer or the PlatformBrightness widget

Reprevise avatar Dec 24 '19 21:12 Reprevise

It works for me ... Screen Shot 2019-12-24 at 1 49 02 PM

ThinkDigitalSoftware avatar Dec 24 '19 21:12 ThinkDigitalSoftware

Not for me: image

Reprevise avatar Dec 24 '19 21:12 Reprevise

OK, I was expecting to see an error. It does show Light for me as well. One sec

ThinkDigitalSoftware avatar Dec 24 '19 21:12 ThinkDigitalSoftware

I don't know how to resolve this cause it requires a WidgetsApp class above it.

ThinkDigitalSoftware avatar Dec 24 '19 21:12 ThinkDigitalSoftware

How does Flutter do it with ThemeMode (property of MaterialApp) then?

Reprevise avatar Dec 24 '19 22:12 Reprevise

Because they access the variable you set there or default it to ThemeMode.system. Inside the widget, it uses MediaQuery.platformBrightnessOf(context); which works because this is separated by a builder and the MediaQuery is already in the widget tree by that time.

  builder: (BuildContext context, Widget child) {
        // Use a light theme, dark theme, or fallback theme.
        final ThemeMode mode = widget.themeMode ?? ThemeMode.system;
        ThemeData theme;
        if (widget.darkTheme != null) {
          final ui.Brightness platformBrightness = MediaQuery.platformBrightnessOf(context);
          if (mode == ThemeMode.dark ||
              (mode == ThemeMode.system && platformBrightness == ui.Brightness.dark)) {
            theme = widget.darkTheme;
          }
        }

ThinkDigitalSoftware avatar Dec 24 '19 22:12 ThinkDigitalSoftware

@Reprevise best thing I can do is make a warning or throw an error if it's put above the MaterialApp, mentioning that there's no ancestor found, since MediaQuery by default returns Brightness.light if it can't find the value

ThinkDigitalSoftware avatar Dec 26 '19 20:12 ThinkDigitalSoftware

Alright, sounds like the best option

Reprevise avatar Dec 26 '19 20:12 Reprevise

@ThinkDigitalSoftware Conflicting files

shinriyo avatar Oct 07 '20 05:10 shinriyo