From fc5cf54d6d26bb0024f8ac34c9a16dbee8a62eea Mon Sep 17 00:00:00 2001 From: Brent Kelly Date: Tue, 5 Oct 2021 20:21:48 -0700 Subject: [PATCH] Addressing various issues with the Appearance API (#28823) (#29106) Summary: This PR fixes a few issues with the Appearance API (as noted here https://github.com/facebook/react-native/issues/28823). 1. For the Appearance API to work correctly on Android you need to call `AppearanceModule.onConfigurationChanged` when the current Activity goes through a configuration change. This was being called in the RNTester app but not in `ReactActivity` so it meant the Appearance API wouldn't work for Android in newly generated RN projects (or ones upgraded to the latest version of RN). 2. The Appearance API wasn't working correctly for brownfield scenarios on Android. It's possible to force an app light or dark natively on Android by calling `AppCompatDelegate.setDefaultNightMode()`. The Appearance API wasn't picking up changes from this function because it was using the Application context instead of the current Activity context. 3. The Appearance API wasn't working correctly for brownfield scenarios on iOS. Just like on Android its possible to force an app light or dark natively by setting `window.overrideUserInterfaceStyle`. The Appearance API didn't work with this override because we were overwriting `_currentColorScheme` back to default as soon as we set it. ## Changelog ### Fixed https://github.com/facebook/react-native/issues/28823 * [Android] [Fixed] - Appearance API now works on Android * [Android] [Fixed] - Appearance API now works correctly when calling `AppCompatDelegate.setDefaultNightMode()` * [iOS] [Fixed] - Appearance API now works correctly when setting `window.overrideUserInterfaceStyle` Pull Request resolved: https://github.com/facebook/react-native/pull/29106 Test Plan: Ran RNTester on iOS and Android and verified the Appearance examples still worked [correctly.](url) Reviewed By: hramos Differential Revision: D31284331 Pulled By: sota000 fbshipit-source-id: 45bbe33983e506eb177d596d33ddf15f846708fd --- React/CoreModules/RCTAppearance.mm | 4 +++- .../main/java/com/facebook/react/ReactActivity.java | 7 +++++++ .../com/facebook/react/ReactActivityDelegate.java | 7 +++++++ .../react/modules/appearance/AppearanceModule.java | 11 ++++++++++- .../com/facebook/react/uiapp/RNTesterActivity.java | 12 ------------ 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/React/CoreModules/RCTAppearance.mm b/React/CoreModules/RCTAppearance.mm index 820063db4eaa2d..1d762b67cc5fca 100644 --- a/React/CoreModules/RCTAppearance.mm +++ b/React/CoreModules/RCTAppearance.mm @@ -89,7 +89,9 @@ - (dispatch_queue_t)methodQueue RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSString *, getColorScheme) { - _currentColorScheme = RCTColorSchemePreference(nil); + if (_currentColorScheme == nil) { + _currentColorScheme = RCTColorSchemePreference(nil); + } return _currentColorScheme; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactActivity.java b/ReactAndroid/src/main/java/com/facebook/react/ReactActivity.java index 4504a261cb8b4d..f7f1a53a4d585c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactActivity.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactActivity.java @@ -8,6 +8,7 @@ package com.facebook.react; import android.content.Intent; +import android.content.res.Configuration; import android.os.Bundle; import android.view.KeyEvent; import androidx.annotation.Nullable; @@ -120,6 +121,12 @@ public void onWindowFocusChanged(boolean hasFocus) { mDelegate.onWindowFocusChanged(hasFocus); } + @Override + public void onConfigurationChanged(Configuration newConfig) { + super.onConfigurationChanged(newConfig); + mDelegate.onConfigurationChanged(newConfig); + } + protected final ReactNativeHost getReactNativeHost() { return mDelegate.getReactNativeHost(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java index 7083f86bfc9cbb..7ca749c8a046f3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java @@ -11,6 +11,7 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; +import android.content.res.Configuration; import android.os.Build; import android.os.Bundle; import android.view.KeyEvent; @@ -154,6 +155,12 @@ public void onWindowFocusChanged(boolean hasFocus) { } } + public void onConfigurationChanged(Configuration newConfig) { + if (getReactNativeHost().hasInstance()) { + getReactInstanceManager().onConfigurationChanged(getContext(), newConfig); + } + } + @TargetApi(Build.VERSION_CODES.M) public void requestPermissions( String[] permissions, int requestCode, PermissionListener listener) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java index 99b7c5311becc6..ab47e66c526b7d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java @@ -7,6 +7,7 @@ package com.facebook.react.modules.appearance; +import android.app.Activity; import android.content.Context; import android.content.res.Configuration; import androidx.annotation.Nullable; @@ -74,7 +75,15 @@ public String getName() { @Override public String getColorScheme() { - mColorScheme = colorSchemeForCurrentConfiguration(getReactApplicationContext()); + // Attempt to use the Activity context first in order to get the most up to date + // scheme. This covers the scenario when AppCompatDelegate.setDefaultNightMode() + // is called directly (which can occur in Brownfield apps for example). + Activity activity = getCurrentActivity(); + + mColorScheme = + colorSchemeForCurrentConfiguration( + activity != null ? activity : getReactApplicationContext()); + return mColorScheme; } diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java index 4cbb384cded703..9d7964a5f22447 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java @@ -7,12 +7,10 @@ package com.facebook.react.uiapp; -import android.content.res.Configuration; import android.os.Bundle; import androidx.annotation.Nullable; import com.facebook.react.ReactActivity; import com.facebook.react.ReactActivityDelegate; -import com.facebook.react.ReactInstanceManager; import com.facebook.react.ReactRootView; public class RNTesterActivity extends ReactActivity { @@ -64,14 +62,4 @@ protected ReactActivityDelegate createReactActivityDelegate() { protected String getMainComponentName() { return "RNTesterApp"; } - - @Override - public void onConfigurationChanged(Configuration newConfig) { - super.onConfigurationChanged(newConfig); - ReactInstanceManager instanceManager = getReactInstanceManager(); - - if (instanceManager != null) { - instanceManager.onConfigurationChanged(this, newConfig); - } - } }