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

Use themeMode in SharezoneMaterialApp. #266

Merged
merged 2 commits into from
Jun 29, 2022

Conversation

Jonas-Sander
Copy link
Collaborator

@Jonas-Sander Jonas-Sander commented Jun 27, 2022

From themeMode docs:

Determines which theme will be used by the application if both [theme] and [darkTheme] are provided.

If set to [ThemeMode.system], the choice of which theme to use will be based on the user's system preferences. If the [MediaQuery.platformBrightnessOf] is [Brightness.light], [theme] will be used. If it is [Brightness.dark], [darkTheme] will be used (unless it is null, in which case [theme] will be used.

If set to [ThemeMode.light] the [theme] will always be used, regardless of the user's system preference.

If set to [ThemeMode.dark] the [darkTheme] will be used regardless of the user's system preference. If [darkTheme] is null then it will fallback to using [theme].

The default value is [ThemeMode.system].

@github-actions
Copy link

github-actions bot commented Jun 27, 2022

Visit the preview URL for this PR (updated for commit 17341fc):

https://sharezone-test--pr266-material-app-use-the-enedxa9y.web.app

(expires Wed, 06 Jul 2022 20:57:01 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Member

@nilsreichardt nilsreichardt left a comment

Choose a reason for hiding this comment

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

LGTM

return ThemeMode.light;
case ThemeBrightness.system:
return ThemeMode.system;
default:
Copy link
Member

Choose a reason for hiding this comment

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

Would it make more sense to don't have a default value? Therefore, the analyzer would show a warning when add a new value for ThemeBrithness

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! Yeah of course, was maybe left over from the standard template.

@Jonas-Sander Jonas-Sander enabled auto-merge (squash) June 29, 2022 20:52
@Jonas-Sander Jonas-Sander merged commit 4a8e0b9 into main Jun 29, 2022
@Jonas-Sander Jonas-Sander deleted the material-app-use-theme-mode branch June 29, 2022 21:15
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