feat: Auto detect dark theme #1276

AshAman999 · 2022-03-20T14:14:43Z

What

Let's user change the theme according to the device theme
Also need help in the file lib/pages/user_preferences_settings.dart as a comment left on line 84

demo.mp4

Fixes bug(s)

Automatically identify the system dark mode status #1257

teolemon · 2022-03-20T15:04:44Z

@AshAman999 adding 'feat: ' in the title makes your PR semantic, and thus Release Please will generate the Changelog automatically. I just edited the title accordingly.

packages/smooth_app/lib/generic_lib/theme_constants.dart

teolemon · 2022-03-20T15:05:41Z

packages/smooth_app/lib/pages/user_preferences_settings.dart

+                },
+                items: <DropdownMenuItem<String>>[
+                  DropdownMenuItem<String>(
+                    // need suggestions on how to set my own appLoxalizations for the string System Default


Suggested change

// need suggestions on how to set my own appLoxalizations for the string System Default

// need suggestions on how to set my own appLocalizations for the string System Default

M123-dev · 2022-03-20T14:27:23Z

packages/smooth_app/lib/pages/user_preferences_settings.dart

+                },
+                items: <DropdownMenuItem<String>>[
+                  DropdownMenuItem<String>(
+                    // need suggestions on how to set my own appLoxalizations for the string System Default


You can create new strings in lib/l10n/app_en.arb they are probably not directly picked up by the analyzer but after restarting the app they should definitely be there

I am still not able to figure this out, as it was throwing an error and I couldn't find it :(

Then just add a todo comment like this:

//TODO(X): Localize

M123-dev · 2022-03-20T14:27:58Z

packages/smooth_app/lib/data_models/user_preferences.dart

@@ -18,6 +18,7 @@ class UserPreferences extends ChangeNotifier {
  static const String _TAG_PREFIX_IMPORTANCE = 'IMPORTANCE_AS_STRING';
  static const String _TAG_INIT = 'init';
  static const String _TAG_THEME_DARK = 'themeDark';


Can't this and the methods for getting and setting darkmode be removed

Yeah, and then the whole places where the bool darkTheme was used,
As the bool darkTheme will be removed the places where it was being needed to have a bool to show diff colors according to the theme, as with the provided can't check exactly what theme is being used when using ThemeMode.system

old way if (themeProvider.darkTheme) { _textFieldBackgroundColor = Colors.white10; }

new proposed way to check if app uses dark theme for now

if (MediaQuery.platformBrightnessOf(context) == Brightness.dark) { _textFieldBackgroundColor = Colors.white10; }

As for now, it is working, should I do it?

M123-dev · 2022-03-20T17:28:52Z

packages/smooth_app/lib/data_models/user_preferences.dart

@@ -18,6 +18,7 @@ class UserPreferences extends ChangeNotifier {
  static const String _TAG_PREFIX_IMPORTANCE = 'IMPORTANCE_AS_STRING';
  static const String _TAG_INIT = 'init';
  static const String _TAG_THEME_DARK = 'themeDark';


Suggested change

static const String _TAG_THEME_DARK = 'themeDark';

M123-dev · 2022-03-20T17:30:12Z

packages/smooth_app/lib/themes/smooth_theme.dart

@@ -47,8 +47,9 @@ class SmoothTheme {

  static MaterialColor getMaterialColor(
    final ThemeProvider themeProvider,


You can remove the ThemeProvider here and get it inside getMaterialColorImpl since you already pass the context

did the same, but some unit tests are failing Idk why? rest the app was working real smooth

Probably because of screenshot test. There the test check if the changes in UI are over a cartain percentage. The question is why the ui changed.

Maybe when we use MediaQuery.platformBrightnessOf(context) == Brightness.dark instead of context.watch<ThemeProvider>().darkMode it doesn't change when changing the theme.
This should defintely be checked

yeah the screenshot test was failing I tried to look into this and still scratching my head,I am kinda stuck guess I need some help here ;(

Heyy yeah sure, sorry for the late reply. You can regenerate them by running flutter test --update-goldens.

AshAman999 · 2022-03-21T09:39:33Z

Can it be possible that the test cases were written into taking consideration according to the previous code, so even if my code is doing the work it is failing the tests because it wasn't supposed to be for it? @M123-dev

g123k · 2022-03-22T09:30:59Z

packages/smooth_app/lib/generic_lib/buttons/smooth_large_button_with_icon.dart

@@ -25,7 +23,7 @@ class SmoothLargeButtonWithIcon extends StatelessWidget {
        children: <Widget>[
          Icon(
            icon,
-            color: isDarkMode
+            color: MediaQuery.platformBrightnessOf(context) == Brightness.dark


It would be better to create a single utility method or an extension for this kind of check

…9/smooth-app into auto-detect-dark-theme

M123-dev

Heyyyy thanks @AshAman999, just some places where your updated isDarkMode method should be used besided that the code looks good.

But please also remove the screenshots in test/pages/failures

M123-dev · 2022-03-23T15:10:05Z

packages/smooth_app/lib/generic_lib/buttons/smooth_large_button_with_icon.dart

+    final bool isDarkMode =
+        MediaQuery.platformBrightnessOf(context) == Brightness.dark;


Please use the isDarkMode method

M123-dev · 2022-03-23T15:10:15Z

packages/smooth_app/lib/pages/user_management/forgot_password_page.dart

+    final bool isDarkMode =
+        MediaQuery.platformBrightnessOf(context) == Brightness.dark;


M123-dev · 2022-03-23T15:10:22Z

packages/smooth_app/lib/pages/user_management/login_page.dart

+    final bool isDarkMode =
+        MediaQuery.platformBrightnessOf(context) == Brightness.dark;


AshAman999 · 2022-03-23T16:35:46Z

@M123-dev did the changes, you can have a look at the recent changes

M123-dev · 2022-03-24T08:26:29Z

packages/smooth_app/lib/generic_lib/buttons/smooth_large_button_with_icon.dart

  final EdgeInsets? padding;

  @override
  Widget build(BuildContext context) {
    final ThemeData themeData = Theme.of(context);
+    final UserPreferences userPreferences =
+        Provider.of<UserPreferences>(context, listen: false);
+    final bool isDarkMode = ThemeProvider(userPreferences).isDarkMode(context);


Just one thing I just noticed. The ThemeProvider is a provider, so you shouldn't create it all over again but directly access it through it

…9/smooth-app into auto-detect-dark-theme

AshAman999 · 2022-03-24T09:36:31Z

@M123-dev id the changes, please check again,
sorry for the troubles, feels like there is a lot of ongoings for a simple dark theme implementation ;(

M123-dev · 2022-03-24T15:07:56Z

No need to apologize, now after the small touch ups the code looks great.Only the screenshot tests for the dark preferences pages (left tab) have over 80% deviation. It seems there is some problem

AshAman999 · 2022-03-24T15:39:34Z

No need to apologize, now after the small touch ups the code looks great.Only the screenshot tests for the dark preferences pages (left tab) have over 80% deviation. It seems there is some problem

Though i may be wrong , is it possible that the tests were written so as to be for the previous code, since we are using a different method to chose dark theme so it may be possible that the code might be doing okay but the tests needs to be rewritten

M123-dev · 2022-03-24T15:48:52Z

That's a likely answer, I just ran your PR and it worked fine for me. Merging is blocken as long as the tests are failing. I'll have a look at the screenshot test later

M123-dev · 2022-03-26T09:55:52Z

Heyy @AshAman999 I just checked the implementation here: https://github.com/openfoodfacts/smooth-app/blob/develop/packages/smooth_app/test/pages/user_preferences_page_test.dart#L32

There we create mock SharedPreferences with the key themeDark to either true or false.
This should be adopted for your New currentTheme String with the options Dark and Light. You can leave out System Default as it's always Light by default.

M123-dev · 2022-03-30T16:22:29Z

Heyy @AshAman999 fixed the screenshot test + resolved conflicts

AshAman999 · 2022-03-30T16:33:28Z

I am looking at the changes you did, needed to see how exactly how it was done.
Thanks for the rescue here, had nearly given up here and is it now ready to be merged?

M123-dev · 2022-03-30T16:51:45Z

Yeah the screenshot tests can be sometimes confusing and yes I think so, didn't do any major changes outside of the tests and we've been waiting for the automatic darkmode for way too long now

M123-dev · 2022-03-30T16:52:17Z

But yeah thanks a lot for this great new feature

AshAman999 · 2022-03-30T16:58:59Z

Really happy to see this pr merged :)
thanks a lot for the guidance @M123-dev 💯

AshAman999 added 3 commits March 20, 2022 19:32

made changes for getting theme from shared pref

1f58875

constants and methods added

f73e78f

ui changes for the selecting themeMode

c926cff

AshAman999 requested a review from a team as a code owner March 20, 2022 14:14

teolemon linked an issue Mar 20, 2022 that may be closed by this pull request

Automatically identify the system dark mode status #1257

Closed

teolemon added the darkmode Ensuring we look good in the dark label Mar 20, 2022

teolemon changed the title ~~Auto detect dark theme~~ feat: Auto detect dark theme Mar 20, 2022

teolemon reviewed Mar 20, 2022

View reviewed changes

packages/smooth_app/lib/generic_lib/theme_constants.dart Outdated Show resolved Hide resolved

teolemon reviewed Mar 20, 2022

View reviewed changes

M123-dev reviewed Mar 20, 2022

View reviewed changes

AshAman999 added 6 commits March 20, 2022 22:11

check dark theme with platform brightness

9b4e623

removed unnessary fields and imports

34e5afc

used conext wherever nessacry

9413a34

temp fix for showing systemDefault on ui

d2769e4

formatted the code

ca498e2

ran flutter format .

4892da4

M123-dev reviewed Mar 20, 2022

View reviewed changes

AshAman999 added 4 commits March 20, 2022 23:20

removed unused dark theme prefs

985e32e

pulled the changes

7edafca

removed themeprovider from wherever un-necessary

3aa02ae

solve merge conflicts

c6acee8

Merge branch 'develop' into auto-detect-dark-theme

cdb4fa2

g123k requested changes Mar 22, 2022

View reviewed changes

AshAman999 and others added 5 commits March 22, 2022 15:54

create single utility method to check Darktheme

7f3dac8

Merge branch 'auto-detect-dark-theme' of https://github.com/AshAman99…

0ff1742

…9/smooth-app into auto-detect-dark-theme

Merge branch 'develop' into auto-detect-dark-theme

5e25ded

formatted the code

09c918b

added todo for translation

2dd7337

M123-dev reviewed Mar 23, 2022

View reviewed changes

AshAman999 and others added 3 commits March 23, 2022 21:57

used isDarkMode function throughout the code

fb2fccd

deleted scrshots from test/pages/failures

81e3962

Merge branch 'develop' into auto-detect-dark-theme

09faee2

Merge branch 'develop' into auto-detect-dark-theme

ef95aa9

M123-dev reviewed Mar 24, 2022

View reviewed changes

AshAman999 and others added 3 commits March 24, 2022 14:47

call provider to get theme data

7d184ea

Merge branch 'auto-detect-dark-theme' of https://github.com/AshAman99…

6fa399e

…9/smooth-app into auto-detect-dark-theme

Merge branch 'develop' into auto-detect-dark-theme

ba257c3

AshAman999 added 2 commits March 25, 2022 17:39

Merge branch 'develop' into auto-detect-dark-theme

e30dff1

Merge branch 'develop' into auto-detect-dark-theme

c41271f

M123-dev and others added 2 commits March 30, 2022 18:17

fix: Mock data for screenshot tests

28aa581

Merge branch 'develop' into auto-detect-dark-theme

f311007

M123-dev added 2 commits March 30, 2022 18:27

Update next_button.dart

7cb36f8

Update next_button.dart

2a76cce

AshAman999 requested a review from g123k March 30, 2022 16:36

M123-dev merged commit 809e2ef into openfoodfacts:develop Mar 30, 2022

github-actions bot mentioned this pull request Mar 30, 2022

chore(develop): release 1.0.0 #962

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Auto detect dark theme #1276

feat: Auto detect dark theme #1276

AshAman999 commented Mar 20, 2022

teolemon commented Mar 20, 2022

teolemon Mar 20, 2022

M123-dev Mar 20, 2022

AshAman999 Mar 20, 2022

M123-dev Mar 20, 2022

M123-dev Mar 20, 2022

AshAman999 Mar 20, 2022

M123-dev Mar 20, 2022

M123-dev Mar 20, 2022

AshAman999 Mar 20, 2022

M123-dev Mar 20, 2022 •

edited

Loading

AshAman999 Mar 21, 2022

M123-dev Mar 23, 2022

AshAman999 commented Mar 21, 2022

g123k Mar 22, 2022

M123-dev left a comment

M123-dev Mar 23, 2022

M123-dev Mar 23, 2022

M123-dev Mar 23, 2022

AshAman999 commented Mar 23, 2022

M123-dev Mar 24, 2022

AshAman999 commented Mar 24, 2022

M123-dev commented Mar 24, 2022

AshAman999 commented Mar 24, 2022

M123-dev commented Mar 24, 2022

M123-dev commented Mar 26, 2022

M123-dev commented Mar 30, 2022

AshAman999 commented Mar 30, 2022 •

edited

Loading

M123-dev commented Mar 30, 2022

M123-dev commented Mar 30, 2022

AshAman999 commented Mar 30, 2022

	// need suggestions on how to set my own appLoxalizations for the string System Default
	// need suggestions on how to set my own appLocalizations for the string System Default

		@@ -47,8 +47,9 @@ class SmoothTheme {

		static MaterialColor getMaterialColor(
		final ThemeProvider themeProvider,

		final bool isDarkMode =
		MediaQuery.platformBrightnessOf(context) == Brightness.dark;

feat: Auto detect dark theme #1276

feat: Auto detect dark theme #1276

Conversation

AshAman999 commented Mar 20, 2022

What

Fixes bug(s)

teolemon commented Mar 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

M123-dev Mar 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshAman999 commented Mar 21, 2022

Choose a reason for hiding this comment

M123-dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshAman999 commented Mar 23, 2022

Choose a reason for hiding this comment

AshAman999 commented Mar 24, 2022

M123-dev commented Mar 24, 2022

AshAman999 commented Mar 24, 2022

M123-dev commented Mar 24, 2022

M123-dev commented Mar 26, 2022

M123-dev commented Mar 30, 2022

AshAman999 commented Mar 30, 2022 • edited Loading

M123-dev commented Mar 30, 2022

M123-dev commented Mar 30, 2022

AshAman999 commented Mar 30, 2022

M123-dev Mar 20, 2022 •

edited

Loading

AshAman999 commented Mar 30, 2022 •

edited

Loading