jonataslaw / getx

Open screens/snackbars/dialogs/bottomSheets without context, manage states and inject dependencies easily with Get.
MIT License
10.35k stars 1.63k forks source link

Get.isDarkMode returns improper value #816

Closed bdevay closed 3 years ago

bdevay commented 3 years ago

Describe the bug The Get.isDarkMode returns improper value

**Reproduction code Use a Switch widget to show the current mode and give the opt-in/out functionality.

example:

Switch (
   onChanged: (nightMode) => {Get.changeTheme(Get.isDarkMode ? ThemeData.light() : ThemeData.dark())},
   value: Get.isDarkMode,
   )

To Reproduce Steps to reproduce the behavior:

  1. Launch the application and go to the screen where the switch is placed
  2. See that the initial value is correct
  3. Tap the Switch
  4. See that the Switch doesn't change but the night mode turns on
  5. Tap the Switch again and again
  6. See that the Switch doesn't correspond to the actual mode anymore

Expected behavior The return value of the Get.isDarkMode should return the actual mode

Screenshots

  1. See that the initial value is correct

image

  1. See that the Switch doesn't change but the night mode turns on

image

  1. See that the Switch doesn't correspond to the actual mode anymore

image

Flutter Version: Flutter (Channel dev, 1.24.0-10.2.pre, on Microsoft Windows [Version 10.0.18363.1198], locale en-US) • Flutter version 1.24.0-10.2.pre at C:\Flutter\SDK\flutter • Framework revision 022b333a08 (5 days ago), 2020-11-18 11:35:09 -0800 • Engine revision 07c1eed46b • Dart version 2.12.0 (build 2.12.0-29.10.beta)

Getx Version: 3.17.1

Describe on which device you found the bug: Android emulator

Minimal reproduce code

import 'package:flutter/material.dart';
import 'package:get/get.dart';

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return GetMaterialApp(
      title: 'Get Theme Bug',
      home: MyHomePage(title: 'Get Theme Bug'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  MyHomePage({Key key, this.title}) : super(key: key);
  final String title;

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(
              'Use the Switcher to change the mode',
            ),
            Switch(
              onChanged: (nightMode) => {
                setState(() {
                  Get.changeTheme(
                      Get.isDarkMode ? ThemeData.light() : ThemeData.dark());
                })
              },
              value: Get.isDarkMode,
            ),
          ],
        ),
      ),
    );
  }
}
bdevay commented 3 years ago

I tried this too but still behaves strangely. With this code, I need to tap the Switch twice to change the mode

            Switch(
              onChanged: (nightMode) => {
                setState(() {
                  Get.changeTheme(
                      nightMode ? ThemeData.dark() : ThemeData.light());
                })
              },
              value: Get.isDarkMode,
            ),
eduardoflorence commented 3 years ago

Hi @bdevay,

import 'package:flutter/material.dart';
import 'package:get/get.dart';

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return GetMaterialApp(
      title: 'Get Theme Bug',
      home: MyHomePage(title: 'Get Theme Bug'),
    );
  }
}

class MyHomePage extends StatelessWidget {
  MyHomePage({Key key, this.title}) : super(key: key);
  final String title;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text('Use the Switcher to change the mode'),
            ValueBuilder<bool>(
              initialValue: Get.isDarkMode,
              builder: (value, updateFn) =>
                  Switch(value: value, onChanged: updateFn),
              onUpdate: (nightMode) => Get.changeTheme(
                  nightMode ? ThemeData.dark() : ThemeData.light()),
            ),
          ],
        ),
      ),
    );
  }
}
import 'package:flutter/material.dart';
import 'package:get/get.dart';

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  runApp(GetMaterialApp(
    initialRoute: '/home',
    getPages: [
      GetPage(
        name: '/home',
        page: () => Home(),
        binding: HomeBinding()
      ),
    ],
  ));
}

class Home extends StatelessWidget {
  final controller = Get.find<ThemeController>();

  @override
  Widget build(context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            Obx(
              () => Text('I am in ${controller.isDarkMode ? 'Dark' : 'Light'} mode'),
            ),
            Obx(() => Switch(
              onChanged: (value) => controller.changeTheme(value),
              value: controller.isDarkMode,
            )),
          ],
        ),
      ),
    );
  }
}

class HomeBinding implements Bindings {
  @override
  void dependencies() {
    Get.put(ThemeController());
  }
}

class ThemeController extends GetxController {
  RxBool _isDarkMode = Get.isDarkMode ? true.obs : false.obs;

  bool get isDarkMode => _isDarkMode.value;
  set isDarkMode(value) => _isDarkMode.value = value;

  changeTheme(nightMode) {
    if (nightMode != isDarkMode) {
      Get.changeTheme(nightMode ? ThemeData.dark() : ThemeData.light());
      isDarkMode = nightMode;
    }
  }
}
bdevay commented 3 years ago

Hi @eduardoflorence,

I appreciate again your comprehensive explanation and the nice example.

eduardoflorence commented 3 years ago

It is not a limitation of isDarkMode, but Get.changeTheme takes a few milliseconds to update the environment. If you wait this time, you can use setState. See an example:

Switch(
  onChanged: (nightMode) async {
    Get.changeTheme(Get.isDarkMode ? ThemeData.light() : ThemeData.dark());
    await Future.delayed(Duration(milliseconds: 300));
    setState(() {});
  },
  value: Get.isDarkMode,
),
eduardoflorence commented 3 years ago

Hi @bdevay, GetX version 3.20.0 has just been released and added context.isDarkMode to context extensions and this resolves this issue. Replace Get.isDarkMode with context.isDarkMode.

Switch(
  onChanged: (nightMode) => {
    setState(() {
      Get.changeTheme(
          context.isDarkMode ? ThemeData.light() : ThemeData.dark());
    })
  },
  value: context.isDarkMode,
)
bdevay commented 3 years ago

Wow, that's great, thanks.

vaishnavmhetre commented 3 years ago

I'm using carousel_slider, and have 4 slides. Simple stuff. Now each slide uses this buildSlideScaffold to create basic card like structure and then add childWidget as desired for the scenario in it.

Now the great big issues are -

  1. Initially it starts with Dark theme even when mentioned to start with ThemeMode.light, and tried using changeTheme with ThemeHandler.lightTheme and ThemeHandler.darkTheme, but it never changes theme. And Get.isDarkMode always stays unchanged from initial value it recovered when first launched. changeThemeMode worked a bit but facing issue for some unknown reason.
  2. When changed theme the first time, button changes the theme and disappears (Be advised the location permission button isn't using context reference but adapting to change - not sure if correctly as disappearing but the least. But the text doesn't change the color after change of theme - it is expected as the childWidget passed to buildSlideScaffold is inside the Obx and when the Box rebuilds, the Text should also rebuild with updated textTheme (uses context for getting textTheme)

Light Version (Initial) WhatsApp Image 2020-12-20 at 10 17 34 PM Changed to dark (First change) WhatsApp Image 2020-12-20 at 10 07 18 PM (3)

  1. When changed theme to dark and if I'm on 1st slide, and go to 3rd slide (which is off the screen when I'm on 1st slide), it sets the background of 3rd slide to pink (background which which is the color if I call context.theme.backgroundColor when the theme was light, but isDarkMode is true, hence triggering this logic, else if false will set bg to white) Weird that it is getting light theme from context (lol dont know which context it is picking and getting light theme - I never set it), and when I go back from 3rd to 1st slide in same state, 1st slide also changed to pink (before it was grey - as expected).

Changed to dark - currently on 3rd screen WhatsApp Image 2020-12-20 at 10 07 18 PM (3)

Returned to 3rd slide after just going to 1st slide and coming back to 3rd slide WhatsApp Image 2020-12-20 at 10 07 18 PM

You can see the 2nd slide never changed the background to pink (will happen only if I got to 4th slide and come back - this way it will put 2nd slide out of screen)- so this happens to only the slides that leave screen space.

More weird - it does the same thing in case of Light theme where text disappears of the slides that are out of screen.

Switched to light theme - its 2nd slide WhatsApp Image 2020-12-20 at 10 07 18 PM (2)

Went to 4th slide and came back to 2nd slide

WhatsApp Image 2020-12-20 at 10 07 18 PM (1)

Here the text on light theme just turns white when come back to 2nd slide from 4th slide ? Why? Don't know :(

And this is not individual slide issue, this happens with all slides. I have made sure there is no specific color, set to text or anything - they just derive from the theme/textTheme.

Permission Slide

Widget buildLocationPermissionSlide(BuildContext context) {
    return buildSlideScaffold(
      context,
      Column(
        children: [
          Expanded(
            child: Column(
              mainAxisSize: MainAxisSize.max,
              mainAxisAlignment: MainAxisAlignment.center,
              children: [
                Expanded(
                  child: Icon(
                    Icons.add_location_alt_rounded,
                    size: 156.0,
                    color: Colors.red,
                  ),
                ),
                Padding(
                  padding: const EdgeInsets.only(bottom: 72.0),
                  child: Text(
                    'We need location permission for connection optimization',
                    style: context.textTheme.headline5,
                    textAlign: TextAlign.center,
                  ),
                ),
              ],
            ),
          ),
          Padding(
            padding: const EdgeInsets.only(bottom: 12.0),
            child: FutureBuilder(
              future: Nearby().checkLocationPermission(),
              builder: (context, snapshot) {
                if (snapshot.hasData) {
                  updateLocationPermission(snapshot.data);
                  return Obx(
                    () => RaisedButton(
                      onPressed: locationPermission.value
                          ? null
                          : () async {
                              return updateLocationPermission(
                                  await Nearby().askLocationPermission());
                            }, // Button for location access
                      child: Text(locationPermission.value
                          ? 'This is done!'
                          : 'Allow Location Access!'),
                      color: Colors.red,
                      textColor: Colors.white,
                    ),
                  );
                }
                return Column(
                  mainAxisSize: MainAxisSize.min,
                  children: [
                    Container(
                      width: 72.0,
                      child: LinearProgressIndicator(),
                    ),
                    SizedBox(height: 12.0),
                    Text('Checking Permission'),
                  ],
                );
              },
            ),
          ),
        ],
      ),
    );
  }

Context usage - generic method for slides

Widget buildSlideScaffold(BuildContext context, Widget childWidget) {
    return Obx(() {
      log("${Get.isDarkMode}");
      return Container(
        padding: EdgeInsets.all(18.0),
        width: MediaQuery.of(context).size.width,
        decoration: BoxDecoration(
          color: ThemeHandler.isDarkTheme
              ? context.theme.backgroundColor
              : Colors.white,
          borderRadius: BorderRadius.circular(12.0),
        ),
        child: childWidget,
      );
    });
  }

Theme Handler

class ThemeHandler extends GetxController {
  final isDarkMode = false.obs;

  changeTheme() {
    Get.changeThemeMode(
        Get.isDarkMode ? ThemeMode.light : ThemeMode.dark);
    isDarkMode.value = Get.isDarkMode;

    log("Is Dark: ${Get.isDarkMode}");
  }

  static toggleBrightness() {
    Get.find<ThemeHandler>().changeTheme();
  }

  static bool get isDarkTheme => controller.isDarkMode.value;

  static ThemeHandler get controller => Get.find<ThemeHandler>();

  static final ThemeData lightTheme = ThemeData(
    primarySwatch: Colors.red,
    visualDensity: VisualDensity.adaptivePlatformDensity,
    brightness: Brightness.light,
  );

  static final ThemeData darkTheme = ThemeData(
    primarySwatch: Colors.red,
    visualDensity: VisualDensity.adaptivePlatformDensity,
    brightness: Brightness.dark,
  );
}

MyApp

class MyApp extends StatelessWidget {
  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    // Nearby().askLocationAndExternalStoragePermission();
    ThemeHandler.controller.isDarkMode.value = context.isDarkMode;
    return GetMaterialApp(
      title: 'Flutter Demo',
      theme: ThemeHandler.lightTheme,
      darkTheme: ThemeHandler.darkTheme,
      themeMode: ThemeMode.light,
      home: SendIt(),
    );
  }
}
eduardoflorence commented 3 years ago

@vaishnavmhetre, replace Get.isDarkMode with context.isDarkMode

vaishnavmhetre commented 3 years ago

@vaishnavmhetre, replace Get.isDarkMode with context.isDarkMode

Tried it. Doesn't work for the background color nor.fpr the button.

Does the app redraw completely if I change the theme?

For the button, I'm changing the icon based on dark or light value post click, so I might need the Obx tracking isDarkMode. obs in ThemeHandler.

vaishnavmhetre commented 3 years ago

Update: I tried updating every literal call to Get.isDarkMode to context.isDarkMode, even when checking darkMode whilst changeTheme and it worked somehow!

Secondly I set the value of isDarkMode.obs (default set to false) to ThemeHandler.controller.isDarkMode.value = context.isDarkMode; right at start of App (the first build function in hierarchy).

This worked, but why is it so? Just curious to know in case face some issues in future.

MrCsabaToth commented 2 years ago

My problem is that Get.textTheme.headline5 still doesn't change right after I perform a Get.changeThemeMode.