-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: System Theme Mode #451
Changes from all commits
f6d4f29
4b0837c
2e69c97
8295f3c
74c47f4
98848f9
cd5a2cf
b8db0cf
717048c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should rework this from Stateful Widget to work with GetX state mangement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still a Stateful Widget, it will make more sense to use GetX to keep it consistent. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,18 +5,19 @@ import 'package:ultimate_alarm_clock/app/modules/settings/controllers/theme_cont | |
import 'package:ultimate_alarm_clock/app/utils/constants.dart'; | ||
import 'package:ultimate_alarm_clock/app/utils/utils.dart'; | ||
|
||
import '../../../data/providers/secure_storage_provider.dart'; | ||
|
||
class ThemeValueTile extends StatefulWidget { | ||
const ThemeValueTile({ | ||
super.key, | ||
Key? key, | ||
required this.controller, | ||
required this.height, | ||
required this.width, | ||
required this.themeController, | ||
}); | ||
}) : super(key: key); | ||
|
||
final SettingsController controller; | ||
final ThemeController themeController; | ||
|
||
final double height; | ||
final double width; | ||
|
||
|
@@ -25,6 +26,26 @@ class ThemeValueTile extends StatefulWidget { | |
} | ||
|
||
class _ThemeValueTileState extends State<ThemeValueTile> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
final _secureStorageProvider = SecureStorageProvider(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am sorry @MarkisDev but I am not getting you on this point. |
||
RxString appTheme = ''.obs; | ||
|
||
@override | ||
void initState() { | ||
getAppTheme(); | ||
super.initState(); | ||
} | ||
|
||
void getAppTheme() async { | ||
var storedTheme = await _secureStorageProvider.readThemeValue(); | ||
if (storedTheme == AppTheme.system) { | ||
appTheme.value = 'System Mode'; | ||
} else if (storedTheme == AppTheme.light) { | ||
appTheme.value = 'Light Mode'; | ||
} else if (storedTheme == AppTheme.dark) { | ||
appTheme.value = 'Dark Mode'; | ||
} | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
return Obx( | ||
|
@@ -35,30 +56,76 @@ class _ThemeValueTileState extends State<ThemeValueTile> { | |
isLightMode: widget.themeController.isLightMode.value, | ||
), | ||
child: Padding( | ||
padding: EdgeInsets.only(left: 30, right: 20), | ||
padding: EdgeInsets.only(left: 30), | ||
child: Row( | ||
mainAxisAlignment: MainAxisAlignment.spaceBetween, | ||
children: [ | ||
Expanded( | ||
child: Text( | ||
'Enable Light Mode'.tr, | ||
style: Theme.of(context).textTheme.bodyLarge, | ||
DropdownMenu( | ||
menuStyle: MenuStyle( | ||
backgroundColor: MaterialStatePropertyAll( | ||
widget.themeController.isLightMode.value | ||
? kLightSecondaryBackgroundColor | ||
: ksecondaryBackgroundColor, | ||
), | ||
), | ||
), | ||
Obx( | ||
() => Switch.adaptive( | ||
value: widget.themeController.isLightMode.value, | ||
activeColor: ksecondaryColor, | ||
onChanged: (bool value) async { | ||
widget.themeController.toggleThemeValue(value); | ||
Get.changeThemeMode( | ||
widget.themeController.isLightMode.value | ||
? ThemeMode.light | ||
: ThemeMode.dark, | ||
); | ||
Utils.hapticFeedback(); | ||
}, | ||
inputDecorationTheme: | ||
InputDecorationTheme(enabledBorder: InputBorder.none), | ||
trailingIcon: Icon( | ||
Icons.arrow_drop_down_outlined, | ||
size: 40.0, | ||
color: widget.themeController.isLightMode.value | ||
? kLightPrimaryTextColor.withOpacity(0.8) | ||
: kprimaryTextColor.withOpacity(0.8), | ||
), | ||
width: widget.width * 0.78, | ||
initialSelection: appTheme.value, | ||
label: Text('Select Theme'), | ||
dropdownMenuEntries: [ | ||
DropdownMenuEntry( | ||
value: 'System Mode', | ||
label: 'System Mode', | ||
style: ButtonStyle( | ||
foregroundColor: MaterialStatePropertyAll( | ||
widget.themeController.isLightMode.value | ||
? kLightPrimaryTextColor | ||
: kprimaryTextColor, | ||
), | ||
), | ||
), | ||
DropdownMenuEntry( | ||
value: 'Light Mode', | ||
label: 'Light Mode', | ||
style: ButtonStyle( | ||
foregroundColor: MaterialStatePropertyAll( | ||
widget.themeController.isLightMode.value | ||
? kLightPrimaryTextColor | ||
: kprimaryTextColor, | ||
), | ||
), | ||
), | ||
DropdownMenuEntry( | ||
value: 'Dark Mode', | ||
label: 'Dark Mode', | ||
style: ButtonStyle( | ||
foregroundColor: MaterialStatePropertyAll( | ||
widget.themeController.isLightMode.value | ||
? kLightPrimaryTextColor | ||
: kprimaryTextColor, | ||
), | ||
), | ||
), | ||
], | ||
onSelected: (newValue) { | ||
if (newValue == 'System Mode') { | ||
widget.themeController.toggleSystemTheme(); | ||
} else if (newValue == 'Light Mode') { | ||
widget.themeController.toggleThemeValue(true); | ||
Get.changeThemeMode(ThemeMode.light); | ||
} else { | ||
widget.themeController.toggleThemeValue(false); | ||
Get.changeThemeMode(ThemeMode.dark); | ||
} | ||
Utils.hapticFeedback(); | ||
}, | ||
), | ||
], | ||
), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just check this from the theme details that we're alreading storing, this variable would not be needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.