Skip to content

Commit

Permalink
fix(android): ensure Appearance change listener does not skip events (#…
Browse files Browse the repository at this point in the history
…46017)

Summary:
I'm able to reproduce a case when Appearance module methods are called in the following order:

starting point: dark mode enabled
1. call `setColorScheme` light
2. call `getColorScheme`, which sets `colorScheme` to light [here](https://github.com/facebook/react-native/blob/7bb7a6037bd78bbfa6d9e8499973ea921e9c70e1/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.kt#L57)
3. [onConfigurationChanged](https://github.com/facebook/react-native/blob/7bb7a6037bd78bbfa6d9e8499973ea921e9c70e1/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.kt#L82) is called but `if (colorScheme != newColorScheme)` does not evaluate to true, so no event is dispatched to JS. That means JS is not in sync with the native state.

The issue was the `getColorScheme` had a side-effect of setting `colorScheme` private member (not sure what its use was). The fix remembers the last emitted color scheme value and emits event if new value is different.

## Changelog:

[ANDROID] [FIXED] - ensure Appearance change listener does not skip events

Pull Request resolved: #46017

Test Plan: tested locally with RN tester

Reviewed By: NickGerleman

Differential Revision: D62016949

Pulled By: cipolleschi

fbshipit-source-id: b7b5755d38becda655cf376749d9a996daff7e07
  • Loading branch information
vonovak authored and facebook-github-bot committed Sep 9, 2024
1 parent 148066e commit 7041ed2
Showing 1 changed file with 14 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ constructor(
private val overrideColorScheme: OverrideColorScheme? = null
) : NativeAppearanceSpec(reactContext) {

private var colorScheme = colorSchemeForCurrentConfiguration(reactContext)
private var lastEmittedColorScheme: String? = null

/** Optional override to the current color scheme */
public fun interface OverrideColorScheme {
Expand All @@ -42,10 +42,10 @@ constructor(

val currentNightMode =
context.resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK
when (currentNightMode) {
Configuration.UI_MODE_NIGHT_NO -> return "light"
Configuration.UI_MODE_NIGHT_YES -> return "dark"
else -> return "light"
return when (currentNightMode) {
Configuration.UI_MODE_NIGHT_NO -> "light"
Configuration.UI_MODE_NIGHT_YES -> "dark"
else -> "light"
}
}

Expand All @@ -54,16 +54,16 @@ constructor(
// scheme. This covers the scenario when AppCompatDelegate.setDefaultNightMode()
// is called directly (which can occur in Brownfield apps for example).
val activity = getCurrentActivity()
colorScheme = colorSchemeForCurrentConfiguration(activity ?: getReactApplicationContext())
return colorScheme
return colorSchemeForCurrentConfiguration(activity ?: getReactApplicationContext())
}

public override fun setColorScheme(style: String) {
when {
style == "dark" -> AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_YES)
style == "light" -> AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO)
style == "unspecified" ->
when (style) {
"dark" -> AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_YES)
"light" -> AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO)
"unspecified" ->
AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM)

}
}

Expand All @@ -79,9 +79,9 @@ constructor(
*/
public fun onConfigurationChanged(currentContext: Context) {
val newColorScheme = colorSchemeForCurrentConfiguration(currentContext)
if (colorScheme != newColorScheme) {
colorScheme = newColorScheme
emitAppearanceChanged(colorScheme)
if (lastEmittedColorScheme != newColorScheme) {
lastEmittedColorScheme = newColorScheme
emitAppearanceChanged(newColorScheme)
}
}

Expand Down

0 comments on commit 7041ed2

Please sign in to comment.