Skip to content
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

[core] fix splash flickering in dark mode #26015

Merged
merged 3 commits into from
Dec 19, 2023
Merged

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Dec 19, 2023

Why

aside from #25946 and #25971, i also noticed a splash flickering without expo-system-ui. this one is coming from
RCTRootView setup.

How

we didn't setup backgroundColor as react-native. in dark mode after splash screen and before App fully loaded, it will show white screen in between. this pr tries to set the RCTRootView's background color as system background color.

  • Note that with expo-system-ui, it may override the RCTRootView's background color from it's OnCreate lifecycle.
  • The downside of this pr's approach is that, ReactDelegate subscribers have no way to change the background color. i think it is okay if we don't support the use case.

Test Plan

as #25971

$ yarn create expo -t blank@sdk-50 sdk50
$ cd sdk 50
$ patch << EOF
--- a/App.js
+++ b/App.js
@@ -13,7 +13,7 @@ export default function App() {
 const styles = StyleSheet.create({
   container: {
     flex: 1,
-    backgroundColor: '#fff',
+    backgroundColor: '#000',
     alignItems: 'center',
     justifyContent: 'center',
   },
--- a/app.json
+++ b/app.json
@@ -5,11 +5,11 @@
     "version": "1.0.0",
     "orientation": "portrait",
     "icon": "./assets/icon.png",
-    "userInterfaceStyle": "light",
+    "userInterfaceStyle": "dark",
     "splash": {
       "image": "./assets/splash.png",
       "resizeMode": "contain",
-      "backgroundColor": "#ffffff"
+      "backgroundColor": "#000000"
     },
     "assetBundlePatterns": [
       "**/*"
EOF
$ npx expo run:ios

# should not keep all screen in black

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Dec 19, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Dec 19, 2023
@Kudo Kudo marked this pull request as ready for review December 19, 2023 08:23
@Kudo Kudo merged commit d615e1f into main Dec 19, 2023
@Kudo Kudo deleted the @kudo/fix-ios-background-color2 branch December 19, 2023 14:23
brentvatne pushed a commit that referenced this pull request Dec 19, 2023
# Why

#26015 breaks the build on tvos:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/a2f1abba-1712-4121-a6b0-d69c179d00a4

# How

only set systemBackgroundColor on non-tvos

# Test Plan

ci passed:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/2f54cf61-ae0d-4890-a7e7-64d907ae10e1

# Checklist

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
brentvatne pushed a commit that referenced this pull request Dec 19, 2023
# Why

#26015 breaks the build on tvos:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/a2f1abba-1712-4121-a6b0-d69c179d00a4

# How

only set systemBackgroundColor on non-tvos

# Test Plan

ci passed:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/2f54cf61-ae0d-4890-a7e7-64d907ae10e1

# Checklist

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Dec 19, 2023
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why

aside from expo#25946 and expo#25971, i also noticed a splash flickering without
expo-system-ui. this one is coming from RCTRootView setup.

# How

we didn't setup backgroundColor [as react-native](https://github.com/facebook/reactnative/blob/43826facfab8eed7d01a801778d1c477e60730a6/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm#L154).
in dark mode after splash screen and before App fully loaded, it will show white screen in between. this pr tries to set the RCTRootView's background color as system background color.

- Note that with expo-system-ui, it may override the RCTRootView's
background color from it's OnCreate lifecycle.
- The downside of this pr's approach is that, ReactDelegate subscribers
have no way to change the background color. i think it is okay if we
don't support the use case.
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why

expo#26015 breaks the build on tvos:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/a2f1abba-1712-4121-a6b0-d69c179d00a4

# How

only set systemBackgroundColor on non-tvos

# Test Plan

ci passed:
https://expo.dev/accounts/expo-ci/projects/updates-e2e/builds/2f54cf61-ae0d-4890-a7e7-64d907ae10e1

# Checklist

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants