feat: System Theme Mode #451

duckcommit · 2024-02-16T14:04:36Z

Description

Our app now features a system theme mode, which will listen automatically to the changes in the system theme mode and apply that.

Fixes #427

Screenrecording

Screen_Recording_20240216_191346.1.mp4

Checklist

Code follows the established coding style guidelines
All tests are passing

lib/app/modules/settings/views/theme_value_tile.dart

MarkisDev · 2024-03-02T16:12:06Z

lib/app/modules/settings/views/theme_value_tile.dart

I think we should rework this from Stateful Widget to work with GetX state mangement.

This is still a Stateful Widget, it will make more sense to use GetX to keep it consistent.

MarkisDev · 2024-03-02T16:13:24Z

lib/app/modules/settings/controllers/theme_controller.dart

-      theme: isLightMode.value ? AppTheme.light : AppTheme.dark,
-    );
+  void _startBrightnessCheckTimer() {
+    _brightnessCheckTimer = Timer.periodic(Duration(seconds: 2), (timer) {


This Timer is going to be resource intensive for something as simple as a theme change. We can just make it work on app startup and not keep it "real time"

But if so then it wouln't check for light or dark difference right?

Wait, as I understand this timer is to check if the the device's theme is dark or light and it updates the theme on the app right? The timer runs every 2 seconds to check this, right?

I think we can just set the app to the appropriate theme when it's opened up rather than having to change it in realtime.

Correct me if I am wrong, but isn't that how system theme works? As in if my app in System mode, it needs to check if my device is in light or dark mode right? It is always listening for changes, If we are checking just at the start of the app then its going to be simply like a default light and dark mode yea?

Maybe, but this timer is gonna be unnecessary at the minute. I think we should just set the appropriate theme when the app is restarted, we don't really need it to be real-time.

Ok. Fine. I will make that change.

MarkisDev · 2024-03-02T16:17:19Z

lib/app/modules/settings/controllers/theme_controller.dart


 class ThemeController extends GetxController {
  var isLightMode = true.obs;
  final _secureStorageProvider = SecureStorageProvider();

+  late Timer _brightnessCheckTimer;
+  bool isSystemModeActive = false;


You can just check this from the theme details that we're alreading storing, this variable would not be needed

I tried that, but the problem was that, either the system mode text won't update, or else it won't change completely to light or dark issue. Weird issue but this was the most optimized way I found out

Could you elaborate more on this? We are storing the theme details in storage and fetching it in onInit so we should have access to the option chosen by the user, yes?

In this existing method, we can retrieve the current theme settings. But it isn't working properly for the system mode. As in, all the elements/widgets are not changing accordingly. So I had to do it manually itself. For that I created a new variable. Again it is a weird issue, but this was the only work around for it. Its the optimal one for now.

MarkisDev · 2024-03-10T13:56:42Z

lib/app/modules/settings/views/theme_value_tile.dart

This is still a Stateful Widget, it will make more sense to use GetX to keep it consistent.

MarkisDev · 2024-03-10T13:59:26Z

lib/app/modules/settings/controllers/theme_controller.dart

-      theme: isLightMode.value ? AppTheme.light : AppTheme.dark,
-    );
+  void _startBrightnessCheckTimer() {
+    _brightnessCheckTimer = Timer.periodic(Duration(seconds: 2), (timer) {


Maybe, but this timer is gonna be unnecessary at the minute. I think we should just set the appropriate theme when the app is restarted, we don't really need it to be real-time.

MarkisDev · 2024-03-10T14:03:34Z

lib/app/modules/settings/controllers/theme_controller.dart


 class ThemeController extends GetxController {
  var isLightMode = true.obs;
  final _secureStorageProvider = SecureStorageProvider();

+  late Timer _brightnessCheckTimer;
+  bool isSystemModeActive = false;


Could you elaborate more on this? We are storing the theme details in storage and fetching it in onInit so we should have access to the option chosen by the user, yes?

duckcommit · 2024-03-13T18:27:03Z

@MarkisDev , I have made the changes. Now our app only looks for the system theme on the initial. No more periodic checks.

When we directly retrieve the themes using existing method, system mode is not working properly. It is not change the entire widget. So what I did was, I made a systemenabled variable, after that I do a brightness check to find out if its dark or light. And apply the theme as theme.dark, at the same time maintaining the dropdown option as System,

This is the most optimal method for now.

MarkisDev

Just need some more clarifications before I can merge this in.

MarkisDev · 2024-03-26T14:08:41Z

lib/app/modules/settings/views/theme_value_tile.dart

@@ -25,6 +26,26 @@ class ThemeValueTile extends StatefulWidget {
 }

 class _ThemeValueTileState extends State<ThemeValueTile> {
+  final _secureStorageProvider = SecureStorageProvider();


I don't see the point of making multiple instances of SecureStorageProvider. It's declared as private in the theme controller, when it shouldn't be. Kindly make that change,

I am sorry @MarkisDev but I am not getting you on this point.

MarkisDev · 2024-03-26T14:09:30Z

lib/app/modules/settings/views/theme_value_tile.dart

@@ -25,6 +26,26 @@ class ThemeValueTile extends StatefulWidget {
 }

 class _ThemeValueTileState extends State<ThemeValueTile> {


Why is this a Stateful widget? Could you please explain why this is declared as a stateful widget when GetX is being used for state management within this file?

duckcommit added 2 commits February 15, 2024 23:57

feat: Listener added

f6d4f29

feat: System Theme Mode

4b0837c

its-me-abhishek reviewed Feb 18, 2024

View reviewed changes

lib/app/modules/settings/views/theme_value_tile.dart Outdated Show resolved Hide resolved

duckcommit added 2 commits February 18, 2024 19:25

fix: Typo

2e69c97

fix: remove invalid print statement

8295f3c

MarkisDev requested changes Mar 2, 2024

View reviewed changes

duckcommit requested a review from MarkisDev March 9, 2024 19:04

MarkisDev requested changes Mar 10, 2024

View reviewed changes

duckcommit added 4 commits March 13, 2024 23:17

fix: No periodic check for brightness.

74c47f4

Merge

98848f9

removed unwanted code.

cd5a2cf

Merge branch 'main' into thememodes

b8db0cf

duckcommit requested a review from MarkisDev March 13, 2024 18:27

Merge branch 'main' into thememodes

717048c

MarkisDev requested changes Mar 26, 2024

View reviewed changes

AryanSarafDev closed this Aug 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: System Theme Mode #451

feat: System Theme Mode #451

duckcommit commented Feb 16, 2024

MarkisDev Mar 2, 2024

MarkisDev Mar 10, 2024

MarkisDev Mar 2, 2024

duckcommit Mar 2, 2024

MarkisDev Mar 2, 2024

duckcommit Mar 2, 2024

MarkisDev Mar 10, 2024

duckcommit Mar 13, 2024

MarkisDev Mar 2, 2024

duckcommit Mar 2, 2024

MarkisDev Mar 10, 2024

duckcommit Mar 13, 2024

MarkisDev Mar 10, 2024

MarkisDev Mar 10, 2024

MarkisDev Mar 10, 2024

duckcommit commented Mar 13, 2024

MarkisDev left a comment

MarkisDev Mar 26, 2024

duckcommit Mar 26, 2024

MarkisDev Mar 26, 2024

		@@ -25,6 +26,26 @@ class ThemeValueTile extends StatefulWidget {
		}

		class _ThemeValueTileState extends State<ThemeValueTile> {

feat: System Theme Mode #451

feat: System Theme Mode #451

Conversation

duckcommit commented Feb 16, 2024

Description

Fixes #427

Screenrecording

Checklist

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

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

duckcommit commented Mar 13, 2024

MarkisDev 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