Skip to content
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

Add toggle in settings page for Appearance #621

Merged

Conversation

AlaaElattar
Copy link

@AlaaElattar AlaaElattar commented Oct 2, 2024

  • Added toggle to switch between light and dark modes from settings page.
  • If app is installed for the first time, the theme will be the systems's theme.

Copy link
Contributor

@AhmedHanafy725 AhmedHanafy725 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The app doesn't fetch the initial state of the system
  • On the dark mode the switch doesn't reach the end

app/lib/screens/preference_screen.dart Outdated Show resolved Hide resolved
@AlaaElattar
Copy link
Author

Comment on lines 26 to 30
var brightness =
SchedulerBinding.instance.platformDispatcher.platformBrightness;
_themeMode = brightness == Brightness.dark
? ThemeMode.dark
: ThemeMode.light;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the brightness here will make the app start with one mode and not reflect when changing the system mode.

I prefer to return ThemeMode.system here and check the brightness before rendering the switch if the mode is the system.

Comment on lines 81 to 88
bool isSystemDarkMode = false;
if (themeProvider.themeMode == ThemeMode.system){
var brightness =
SchedulerBinding.instance.platformDispatcher.platformBrightness;
isSystemDarkMode = brightness == Brightness.dark;
}
bool isDarkMode = themeProvider.themeMode == ThemeMode.dark ||
(themeProvider.themeMode == ThemeMode.system && isSystemDarkMode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool isSystemDarkMode = false;
if (themeProvider.themeMode == ThemeMode.system){
var brightness =
SchedulerBinding.instance.platformDispatcher.platformBrightness;
isSystemDarkMode = brightness == Brightness.dark;
}
bool isDarkMode = themeProvider.themeMode == ThemeMode.dark ||
(themeProvider.themeMode == ThemeMode.system && isSystemDarkMode);
bool isDarkMode;
if (themeProvider.themeMode == ThemeMode.system){
final brightness =
SchedulerBinding.instance.platformDispatcher.platformBrightness;
isDarkMode = brightness == Brightness.dark;
} else {
isDarkMode = themeProvider.themeMode == ThemeMode.dark;
}

@AhmedHanafy725 AhmedHanafy725 merged commit 05b3c6d into development_cleanup Oct 3, 2024
@AhmedHanafy725 AhmedHanafy725 deleted the development_cleanup_theme_toggle branch October 3, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants