BirjuVachhani / adaptive_theme

Easiest way to add support for light and dark theme in your flutter app.
https://pub.dev/packages/adaptive_theme
Apache License 2.0
481 stars 37 forks source link

Notify listener when changing theme mode #15

Closed friebetill closed 3 years ago

friebetill commented 3 years ago

Describe the bug Widgets that depend on AdaptiveTheme.of(context) don't rebuild automatically after changing the theme. I guess it is because we are not using an InheritedWidget. The problem is especially annoying for a larger app, where one has to call setState manually everywhere.

Example

```dart import 'package:adaptive_theme/adaptive_theme.dart'; import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; void main() async { WidgetsFlutterBinding.ensureInitialized(); final savedThemeMode = await AdaptiveTheme.getThemeMode(); runApp(MyApp(savedThemeMode: savedThemeMode)); } class MyApp extends StatelessWidget { final AdaptiveThemeMode? savedThemeMode; const MyApp({Key? key, this.savedThemeMode}) : super(key: key); @override Widget build(BuildContext context) { return AdaptiveTheme( light: ThemeData( brightness: Brightness.light, primarySwatch: Colors.red, accentColor: Colors.amber, ), dark: ThemeData( brightness: Brightness.dark, primarySwatch: Colors.red, accentColor: Colors.amber, ), initial: savedThemeMode ?? AdaptiveThemeMode.light, builder: (theme, darkTheme) => MaterialApp( title: 'Adaptive Theme Demo', theme: theme, darkTheme: darkTheme, home: MyHomePage(), ), ); } } class MyHomePage extends StatefulWidget { @override _MyHomePageState createState() => _MyHomePageState(); } class _MyHomePageState extends State { @override Widget build(BuildContext context) { final theme = AdaptiveTheme.of(context); return Scaffold( appBar: AppBar(title: Text('Adaptive Theme Demo')), body: Center( child: TextButton( onPressed: () { showDialog(context: context, builder: (_) => const ThemeDialog()); }, child: Icon( theme.mode == AdaptiveThemeMode.light ? Icons.wb_sunny_outlined : theme.mode == AdaptiveThemeMode.dark ? Icons.bedtime_outlined : Icons.brightness_auto_outlined, ), ), ), ); } } class ThemeDialog extends StatefulWidget { const ThemeDialog({Key? key}) : super(key: key); @override _ThemeDialogState createState() => _ThemeDialogState(); } class _ThemeDialogState extends State { @override Widget build(BuildContext context) { final adaptiveTheme = AdaptiveTheme.of(context); return AlertDialog( title: const Text('Theme'), content: Column( mainAxisSize: MainAxisSize.min, children: [ RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('Light', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.light, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: adaptiveTheme.mode, ), RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('Dark', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.dark, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: adaptiveTheme.mode, ), RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('System', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.system, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: adaptiveTheme.mode, ), ], ), actions: [ TextButton( onPressed: () => Navigator.of(context).pop(), child: Text( MaterialLocalizations.of(context).okButtonLabel, style: Theme.of(context).primaryTextTheme.button, ), ), ], ); } } ```

Expected behavior I expected that when I change the theme, widget that depend on AdaptiveTheme.of(context) will rebuild itself automatically. But unfortunately this is not the case and I have to call setState manually. This is especially noticeable when I switch between e.g. Light and System mode (and the System mode is Light).

BirjuVachhani commented 3 years ago

@friebetill Yeah, that is correct. It won't work because it is not InheritedWidget. However you could use it like this:

Instead of this:

AdaptiveTheme.of(context).mode == AdaptiveThemeMode.dark

do this:

Theme.of(context).brightness == Brightness.light

This works perfectly for me and when I change the theme, it changes for the whole app. Let me know your thoughts on this.

friebetill commented 3 years ago

This is also problematic if one makes a switch between e.g. light mode and system (Light Mode). In this case the icon (from the example) is not adjusted, because Theme.of(context).brightness does not change.

BirjuVachhani commented 3 years ago

@friebetill That makes sense!

I guess it wouldn't be a good idea to use InheritedWidget just to satisfy this at this point. It would require a change in the whole library. Instead I could give a ValueNotifier<AdaptiveThemeMode which you can use for your use-case here.

See the example: ```dart import 'package:adaptive_theme/adaptive_theme.dart'; import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; void main() async { WidgetsFlutterBinding.ensureInitialized(); final savedThemeMode = await AdaptiveTheme.getThemeMode(); runApp(MyApp(savedThemeMode: savedThemeMode)); } class MyApp extends StatelessWidget { final AdaptiveThemeMode? savedThemeMode; const MyApp({Key? key, this.savedThemeMode}) : super(key: key); @override Widget build(BuildContext context) { return AdaptiveTheme( light: ThemeData( brightness: Brightness.light, primarySwatch: Colors.red, accentColor: Colors.amber, ), dark: ThemeData( brightness: Brightness.dark, primarySwatch: Colors.red, accentColor: Colors.amber, ), initial: savedThemeMode ?? AdaptiveThemeMode.light, builder: (theme, darkTheme) => MaterialApp( title: 'Adaptive Theme Demo', theme: theme, darkTheme: darkTheme, home: MyHomePage(), ), ); } } class MyHomePage extends StatefulWidget { @override _MyHomePageState createState() => _MyHomePageState(); } class _MyHomePageState extends State { @override Widget build(BuildContext context) { final theme = AdaptiveTheme.of(context); return Scaffold( appBar: AppBar(title: Text('Adaptive Theme Demo')), body: Center( child: TextButton( onPressed: () { showDialog(context: context, builder: (_) => const ThemeDialog()); }, child: Icon( theme.mode == AdaptiveThemeMode.light ? Icons.wb_sunny_outlined : theme.mode == AdaptiveThemeMode.dark ? Icons.bedtime_outlined : Icons.brightness_auto_outlined, ), ), ), ); } } class ThemeDialog extends StatefulWidget { const ThemeDialog({Key? key}) : super(key: key); @override _ThemeDialogState createState() => _ThemeDialogState(); } class _ThemeDialogState extends State { @override Widget build(BuildContext context) { return AlertDialog( title: const Text('Theme'), content: ValueListenableBuilder( valueListenable: AdaptiveTheme.of(context).modeChangeNotifier, builder: (BuildContext context, AdaptiveThemeMode value, Widget? child) => Column( mainAxisSize: MainAxisSize.min, children: [ RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('Light', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.light, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: value, ), RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('Dark', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.dark, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: value, ), RadioListTile( autofocus: true, selected: true, dense: true, title: const Text('System', style: TextStyle(fontSize: 14)), value: AdaptiveThemeMode.system, onChanged: (value) => adaptiveTheme.setThemeMode(value!), groupValue: value, ), ], ), ), actions: [ TextButton( onPressed: () => Navigator.of(context).pop(), child: Text( MaterialLocalizations.of(context).okButtonLabel, style: Theme.of(context).primaryTextTheme.button, ), ), ], ); } } ```
friebetill commented 3 years ago

Yes that would be sufficient for me! :)

I understand if InheritedWidget is too much work, but I guess in the long run it is the better version. (But I haven't looked at all the advantages and disadvantages and specifically why you did your architecture decisions.)

BirjuVachhani commented 3 years ago

@friebetill I can add this fix in this weekend. Will update you once published.

BirjuVachhani commented 3 years ago

Available in 2.1.0