Add Appearance.setColorScheme support #35989

birkir · 2023-01-27T11:01:31Z

Summary

Both Android and iOS allow you to set application specific user interface style, which is useful for applications that support both light and dark mode.

With the newly added Appearance.setColorScheme, you can natively manage the application's user interface style rather than keeping that preference in JavaScript. The benefit is that native dialogs like alert, keyboard, action sheets and more will also be affected by this change.

Implemented using Android X AppCompatDelegate.setDefaultNightMode and iOS 13+ overrideUserInterfaceStyle

Changelog

[GENERAL] [ADDED] - Added setColorScheme to Appearance module

Test Plan

This is a void function so testing is rather limited.

// Lets assume a given device is set to **dark** mode.

Appearance.getColorScheme(); // `dark`

// Set the app's user interface to `light`
Appearance.setColorScheme('light');

Appearance.getColorScheme(); // `light`

// Set the app's user interface to `unspecified`
Appearance.setColorScheme(null);

Appearance.getColorScheme() // `dark`

facebook-github-bot · 2023-01-27T11:06:38Z

@jacdebug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

analysis-bot · 2023-01-27T11:24:35Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,465,948	-8,400
android	hermes	armeabi-v7a	7,786,614	-9,381
android	hermes	x86	8,940,187	-10,050
android	hermes	x86_64	8,798,247	-9,643
android	jsc	arm64-v8a	9,649,210	-7,609
android	jsc	armeabi-v7a	8,383,474	-8,580
android	jsc	x86	9,711,166	-9,263
android	jsc	x86_64	10,188,251	-8,832

Base commit: 62fa6d9
Branch: main

facebook-github-bot · 2023-01-27T12:03:14Z

@jacdebug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Libraries/Utilities/Appearance.js

ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java

NickGerleman · 2023-01-27T22:23:51Z

Overall question I wanted to check, are we able to call the API early enough on the JS side to avoid fluttering between the settings when starting the app?

birkir · 2023-01-28T09:48:09Z

Thanks for the feedback, especially on the flow front. I didn’t want to add unspecified to the flow type because it wouldn’t be relevant in getColorScheme, but I will add it there, makes everything more clear.

Overall question I wanted to check, are we able to call the API early enough on the JS side to avoid fluttering between the settings when starting the app?

I have been doing this in multiple apps with my own native module, persisted with AsyncStorage, and haven’t had any issues with timing or fluttering.

I’ve persisted with UserDefaults in a native swift app, but this doesn’t bring any noticeable timing benefit.

Splash screen is always displayed in the system selected theme anyway.

NickGerleman · 2023-01-29T01:10:24Z

I didn’t want to add unspecified to the flow type because it wouldn’t be relevant in getColorScheme, but I will add it there, makes everything more clear.

I don't think we should add "unspecified" to ColorScheme, since that type is being used for code listening to appearance changes. So they shouldn't need to handle that as a value. But instead we could either make this new API accept ColorScheme | "unspecified", or more idiomatically I think, just ?ColorScheme to allow null to reset.

birkir · 2023-01-30T19:50:29Z

Agree, I updated the PR to reflect this.
Feel free to add revised feedback if needed.

PS. I think the failing test is false positive

NickGerleman

Mostly LGTM. Still a little bit around nullability that isn't quite annotated correctly.

It matters less since it is an internal implementation details, but you would also be able to forward null to the native component, vs "unspecified".

Libraries/Utilities/Appearance.d.ts

Libraries/Utilities/Appearance.js

Libraries/Utilities/NativeAppearance.js

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

birkir · 2023-02-01T09:55:05Z

Thanks @NickGerleman 👍🏻

facebook-github-bot · 2023-02-03T09:47:41Z

@jacdebug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jacdebug · 2023-02-03T13:58:09Z

React/CoreModules/RCTAppearance.mm

@@ -92,6 +106,17 @@ - (dispatch_queue_t)methodQueue
  return std::make_shared<NativeAppearanceSpecJSI>(params);
 }

+RCT_EXPORT_METHOD(setColorScheme : (NSString *)style) {
+  UIUserInterfaceStyle userInterfaceStyle = [RCTConvert UIUserInterfaceStyle:style];
+  NSArray<__kindof UIWindow*>* windows = [[UIApplication sharedApplication] windows];


I am trying to import this and hit this error in our internal CI, can you investigate it please?

xplat/js/react-native-github/React/CoreModules/RCTAppearance.mm:112:59: error: 'sharedApplication' is unavailable: not available on iOS (App Extension) - Use view controller based solutions where appropriate instead.
NSArray<__kindof UIWindow *> *windows = [[UIApplication sharedApplication] windows];

jacdebug

Please see the error got while importing.

birkir · 2023-02-06T23:01:53Z

Fixed. Wasn't aware of this change - see #34787

Using RCTSharedApplication now that I believe is fail-safe.

facebook-github-bot · 2023-02-07T09:53:10Z

@jacdebug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-02-07T14:26:14Z

@jacdebug merged this pull request in 0a4dcb0.

NickGerleman · 2023-02-08T02:56:49Z

ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java

@@ -66,6 +67,17 @@ private String colorSchemeForCurrentConfiguration(Context context) {
    return "light";
  }

+  @Override
+  public void setColorScheme(String style) {
+    if (style == "dark") {


@birkir I missed that birkir@e633876 changed this from .equals() to ==.

== against two Java strings does a reference comparison, instead of a value comparison. So it is not safe for using to compare string value.

Would you be willing to make a followup change to move this back to .equals()?

Good spot and @birkir can you follow up with a test case to cover this please.

Hey there @birkir @jacdebug. Because I think this change may not work, and we don't have any RNTester page or automated tests to verify a fix, I'm going to revert this change.

Do feel free to resubmit this change, with a fix, and test collateral. I do think this API makes sense as part of Appearance.

Hey there @birkir @jacdebug. Because I think this change may not work, and we don't have any RNTester page or automated tests to verify a fix, I'm going to revert this change

Hopefully we'll have it soon (cc @kelset :))

Sure thing, added RNTester and fixed the bug in a new PR #36122

In retrospect, I think the strict equals bug came about because of coding style change (yoda) and the fact Kotlin uses == the same as Java equals.

Summary: See #35989 (comment) Changelog: [General][Fixed] - Back out "Add Appearance.setColorScheme support" Reviewed By: jacdebug Differential Revision: D43148056 fbshipit-source-id: 823ab8276207f243b788ce7757839a3e95bdbe07

Summary: Both Android and iOS allow you to set application specific user interface style, which is useful for applications that support both light and dark mode. With the newly added `Appearance.setColorScheme`, you can natively manage the application's user interface style rather than keeping that preference in JavaScript. The benefit is that native dialogs like alert, keyboard, action sheets and more will also be affected by this change. Implemented using Android X [AppCompatDelegate.setDefaultNightMode](https://developer.android.com/reference/androidx/appcompat/app/AppCompatDelegate#setDefaultNightMode(int)) and iOS 13+ [overrideUserInterfaceStyle](https://developer.apple.com/documentation/uikit/uiview/3238086-overrideuserinterfacestyle?language=objc) ## Changelog [GENERAL] [ADDED] - Added `setColorScheme` to `Appearance` module Pull Request resolved: facebook#35989 Test Plan: This is a void function so testing is rather limited. ```tsx // Lets assume a given device is set to **dark** mode. Appearance.getColorScheme(); // `dark` // Set the app's user interface to `light` Appearance.setColorScheme('light'); Appearance.getColorScheme(); // `light` // Set the app's user interface to `unspecified` Appearance.setColorScheme(null); Appearance.getColorScheme() // `dark` ``` Reviewed By: NickGerleman Differential Revision: D42801094 Pulled By: jacdebug fbshipit-source-id: ede810fe9ee98f313fd3fbbb16b60c84ef8c7204

Summary: See facebook#35989 (comment) Changelog: [General][Fixed] - Back out "Add Appearance.setColorScheme support" Reviewed By: jacdebug Differential Revision: D43148056 fbshipit-source-id: 823ab8276207f243b788ce7757839a3e95bdbe07

birkir added 2 commits January 27, 2023 10:43

add setColorScheme support

516b4a0

fix: duplicates

6be233a

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 27, 2023

add appcompat to buck

1aa2118

NickGerleman reviewed Jan 27, 2023

View reviewed changes

Libraries/Utilities/Appearance.js Outdated Show resolved Hide resolved

NickGerleman reviewed Jan 27, 2023

View reviewed changes

Libraries/Utilities/Appearance.js Outdated Show resolved Hide resolved

NickGerleman reviewed Jan 27, 2023

View reviewed changes

ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java Outdated Show resolved Hide resolved

NickGerleman reviewed Jan 27, 2023

View reviewed changes

ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.java Outdated Show resolved Hide resolved

fix: yoda and flow type

e633876

NickGerleman reviewed Jan 30, 2023

View reviewed changes

Libraries/Utilities/Appearance.d.ts Outdated Show resolved Hide resolved

Libraries/Utilities/Appearance.js Outdated Show resolved Hide resolved

Libraries/Utilities/Appearance.js Outdated Show resolved Hide resolved

Libraries/Utilities/NativeAppearance.js Outdated Show resolved Hide resolved

birkir and others added 4 commits February 1, 2023 09:50

Update Libraries/Utilities/Appearance.d.ts

c0c6068

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Update Libraries/Utilities/Appearance.js

38273e1

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Update Libraries/Utilities/Appearance.js

c923e82

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

fix: non-null and prettier

eafc4c7

fix: prettier

73aaf9b

NickGerleman approved these changes Feb 3, 2023

View reviewed changes

jacdebug reviewed Feb 3, 2023

View reviewed changes

fix: sharedApplication

b8f8876

facebook-github-bot closed this in 0a4dcb0 Feb 7, 2023

facebook-github-bot added the Merged This PR has been merged. label Feb 7, 2023

NickGerleman reviewed Feb 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Appearance.setColorScheme support #35989

Add Appearance.setColorScheme support #35989

birkir commented Jan 27, 2023

facebook-github-bot commented Jan 27, 2023

analysis-bot commented Jan 27, 2023 •

edited

Loading

facebook-github-bot commented Jan 27, 2023

NickGerleman commented Jan 27, 2023 •

edited

Loading

birkir commented Jan 28, 2023

NickGerleman commented Jan 29, 2023 •

edited

Loading

birkir commented Jan 30, 2023 •

edited

Loading

NickGerleman left a comment

birkir commented Feb 1, 2023

facebook-github-bot commented Feb 3, 2023

jacdebug Feb 3, 2023

jacdebug left a comment

birkir commented Feb 6, 2023

facebook-github-bot commented Feb 7, 2023

facebook-github-bot commented Feb 7, 2023

NickGerleman Feb 8, 2023

jacdebug Feb 8, 2023

NickGerleman Feb 9, 2023

cortinico Feb 9, 2023

birkir Feb 10, 2023

Add Appearance.setColorScheme support #35989

Add Appearance.setColorScheme support #35989

Conversation

birkir commented Jan 27, 2023

Summary

Changelog

Test Plan

facebook-github-bot commented Jan 27, 2023

analysis-bot commented Jan 27, 2023 • edited Loading

facebook-github-bot commented Jan 27, 2023

NickGerleman commented Jan 27, 2023 • edited Loading

birkir commented Jan 28, 2023

NickGerleman commented Jan 29, 2023 • edited Loading

birkir commented Jan 30, 2023 • edited Loading

NickGerleman left a comment

Choose a reason for hiding this comment

birkir commented Feb 1, 2023

facebook-github-bot commented Feb 3, 2023

jacdebug Feb 3, 2023

Choose a reason for hiding this comment

jacdebug left a comment

Choose a reason for hiding this comment

birkir commented Feb 6, 2023

facebook-github-bot commented Feb 7, 2023

facebook-github-bot commented Feb 7, 2023

NickGerleman Feb 8, 2023

Choose a reason for hiding this comment

jacdebug Feb 8, 2023

Choose a reason for hiding this comment

NickGerleman Feb 9, 2023

Choose a reason for hiding this comment

cortinico Feb 9, 2023

Choose a reason for hiding this comment

birkir Feb 10, 2023

Choose a reason for hiding this comment

analysis-bot commented Jan 27, 2023 •

edited

Loading

NickGerleman commented Jan 27, 2023 •

edited

Loading

NickGerleman commented Jan 29, 2023 •

edited

Loading

birkir commented Jan 30, 2023 •

edited

Loading