-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix 🐛 issue where the theme should revert to the system default when ignoreSystemMode is disabled. #7
base: master
Are you sure you want to change the base?
fix 🐛 issue where the theme should revert to the system default when ignoreSystemMode is disabled. #7
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,7 +89,9 @@ export function createTheme({ | |
if (next.ignoreSystemMode) { | ||
Colors.setScheme(scheme as SchemeType); | ||
} else { | ||
Colors.setScheme('default'); | ||
// Revert to system default if ignoreSystemMode is false | ||
const systemScheme = Appearance.getColorScheme() as 'light' | 'dark' | null; | ||
Colors.setScheme(systemScheme || 'default'); | ||
} | ||
|
||
return next; | ||
|
@@ -121,7 +123,12 @@ export function createTheme({ | |
); | ||
|
||
React.useEffect(() => { | ||
Colors.setScheme(scheme as SchemeType); | ||
if (ignoreSystemMode) { | ||
Colors.setScheme(scheme as SchemeType); | ||
} else { | ||
const systemScheme = Appearance.getColorScheme() as 'light' | 'dark' | null; | ||
Colors.setScheme(systemScheme || 'default'); | ||
} | ||
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. Since lines 126-131 are repeated of lines 89-95, can you write a common function to handle both, please? |
||
setNavThemeState( | ||
createReactNavigationTheme( | ||
availableSchemes[scheme], | ||
|
@@ -145,7 +152,8 @@ export function createTheme({ | |
if (next.ignoreSystemMode) { | ||
Colors.setScheme(prev.scheme || defaultScheme); | ||
} else { | ||
Colors.setScheme('default'); | ||
const systemScheme = Appearance.getColorScheme() as 'light' | 'dark' | null; | ||
Colors.setScheme(systemScheme || 'default'); | ||
} | ||
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 logic seems to be repeated again; please write a function to handle all 3 places. |
||
|
||
return next; | ||
|
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.
Lines 89-95 are repeated down below on lines 126-131. You should make a function that takes
ignoreSystemMode
as a parameter and handle both with 1 function, instead of duplicating the logic in multiple places.