-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Code Quality: Introduced AppThemeModeService #14068
Conversation
{ | ||
public enum AppThemeMode | ||
{ | ||
Default, |
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.
Should this be "System"?
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 don't have preferences. Would like to rename to System?
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.
As long as we keep the existing user settings
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.
Please add comments
I could not figure out why the tests have kept failing though I unintalled, did clean build, did rebuild and the app worked well. |
Summary
This is one of the efforts to migrate static Helpers to singleton Services.
Task Checklist
AppThemeModeService
:IAppThemeModeService
Steps To Validate Changes
Known Issues
N/A
PR Checklist
Screenshots
N/A
Footnotes
If the request hasn't been agreed by the team, this work might be rejected. Make sure that you get approval from the team before opening any PR related the request. ↩
If you removed any en-US string resources, make sure that there are no references of those resources. When you add a new en-US string resources, do not make any changes on other languages' resources; Crowdin will do that, instead. ↩