123 #106
123 #106
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
WalkthroughThis pull request primarily removes many configuration, documentation, build, and example files—including various ignore files, project-specific configurations for Android and iOS, and testing templates—to streamline the repository. The most significant functional change is in the Android splash screen implementation, where the Changes
Sequence Diagram(s)sequenceDiagram
participant A as Activity
participant S as SplashScreen
participant D as Dialog
A->>S: show(activity, themeResId, lottieId, fullScreen)
alt fullScreen true
S->>S: setActivityAndroidP(dialog)
end
S->>D: Display splash screen
D-->>S: Animation finished? (isAnimationFinished)
S->>A: Dismiss splash dialog
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
package.json (1)
56-64: Examine Browser Field Mappings.
The"browser"field contains similar mappings to the"react-native"section. If both fields are required to support different bundler behaviors or environments, please ensure that the duplication is intentional and documented. Otherwise, consider consolidating or clarifying the necessity for both to simplify future maintenance.android/src/main/java/org/devio/rn/splashscreen/SplashScreen.java (4)
22-25: Consider thread safety for static boolean flags.The boolean flags
isAnimationFinishedandwaitingare accessed from different threads without synchronization, which could lead to race conditions.Consider making these flags volatile:
- private static Boolean isAnimationFinished = false; - private static Boolean waiting = false; + private static volatile boolean isAnimationFinished = false; + private static volatile boolean waiting = false;Also, use primitive boolean instead of Boolean object for better performance.
46-46: Remove debug print statement.The
System.out.println("asdf")looks like debugging code that should be removed or replaced with proper logging.- System.out.println("asdf"); + // Animation started
73-99: Consider refactoring dialog dismissal logic.There's duplicate code between
setAnimationFinishedandhidemethods for dismissing the dialog. Consider extracting this logic into a separate method.+ private static void tryDismissDialog(Activity activity) { + if (mSplashDialog != null && mSplashDialog.isShowing()) { + boolean isDestroyed = false; + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) { + isDestroyed = activity.isDestroyed(); + } + + if (!activity.isFinishing() && !isDestroyed && + (isAnimationFinished && waiting)) { + mSplashDialog.dismiss(); + mSplashDialog = null; + } + } + }Then update both methods to call this shared implementation.
143-153: Improve method documentation and naming.The method name
setActivityAndroidPdoesn't clearly indicate its purpose. Consider renaming to something more descriptive likeconfigureFullScreenDialog.Also, add more detailed documentation about what this method does and its API level requirements:
- private static void setActivityAndroidP(Dialog dialog) { - //设置全屏展示 + /** + * Configures dialog for full-screen display including display cutout areas + * for devices running Android P (API 28) and above. + * + * @param dialog The dialog to configure for full-screen display + */ + private static void configureFullScreenDialog(Dialog dialog) { + // Configure for full-screen display
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
examples/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngexamples/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexamples/ios/Podfile.lockis excluded by!**/*.lockexamples/yarn.lockis excluded by!**/yarn.lock,!**/*.lockios/.DS_Storeis excluded by!**/.DS_Storescreenshot/Lottie-Splash-Screen-Android.gifis excluded by!**/*.gifscreenshot/Lottie-Splash-Screen-IOS.gifis excluded by!**/*.gifscreenshot/apply-Launchscreen.jpgis excluded by!**/*.jpgscreenshot/apply-image-set.jpgis excluded by!**/*.jpgscreenshot/new-LaunchScreen-image-set.jpgis excluded by!**/*.jpgscreenshot/new-LaunchScreen-storyboard.jpgis excluded by!**/*.jpgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (74)
.gitignore(0 hunks).npmignore(0 hunks)CONTRIBUTING.md(0 hunks)add-LaunchScreen-tutorial-for-ios.md(0 hunks)android/.npmignore(0 hunks)android/proguard-rules.pro(0 hunks)android/src/main/java/org/devio/rn/splashscreen/SplashScreen.java(2 hunks)examples/.buckconfig(0 hunks)examples/.bundle/config(0 hunks)examples/.eslintrc.js(0 hunks)examples/.flowconfig(0 hunks)examples/.gitignore(0 hunks)examples/.prettierrc.js(0 hunks)examples/.ruby-version(0 hunks)examples/.watchmanconfig(0 hunks)examples/App.js(0 hunks)examples/Gemfile(0 hunks)examples/__tests__/App-test.js(0 hunks)examples/android/app/_BUCK(0 hunks)examples/android/app/build.gradle(0 hunks)examples/android/app/build_defs.bzl(0 hunks)examples/android/app/proguard-rules.pro(0 hunks)examples/android/app/src/debug/AndroidManifest.xml(0 hunks)examples/android/app/src/debug/java/com/examples/ReactNativeFlipper.java(0 hunks)examples/android/app/src/main/AndroidManifest.xml(0 hunks)examples/android/app/src/main/java/com/examples/MainActivity.java(0 hunks)examples/android/app/src/main/java/com/examples/MainApplication.java(0 hunks)examples/android/app/src/main/java/com/examples/newarchitecture/MainApplicationReactNativeHost.java(0 hunks)examples/android/app/src/main/java/com/examples/newarchitecture/components/MainComponentsRegistry.java(0 hunks)examples/android/app/src/main/java/com/examples/newarchitecture/modules/MainApplicationTurboModuleManagerDelegate.java(0 hunks)examples/android/app/src/main/jni/Android.mk(0 hunks)examples/android/app/src/main/jni/MainApplicationModuleProvider.cpp(0 hunks)examples/android/app/src/main/jni/MainApplicationModuleProvider.h(0 hunks)examples/android/app/src/main/jni/MainApplicationTurboModuleManagerDelegate.cpp(0 hunks)examples/android/app/src/main/jni/MainApplicationTurboModuleManagerDelegate.h(0 hunks)examples/android/app/src/main/jni/MainComponentsRegistry.cpp(0 hunks)examples/android/app/src/main/jni/MainComponentsRegistry.h(0 hunks)examples/android/app/src/main/jni/OnLoad.cpp(0 hunks)examples/android/app/src/main/res/drawable/rn_edit_text_material.xml(0 hunks)examples/android/app/src/main/res/layout/launch_screen.xml(0 hunks)examples/android/app/src/main/res/raw/loading.json(0 hunks)examples/android/app/src/main/res/values/colors.xml(0 hunks)examples/android/app/src/main/res/values/strings.xml(0 hunks)examples/android/app/src/main/res/values/styles.xml(0 hunks)examples/android/build.gradle(0 hunks)examples/android/gradle.properties(0 hunks)examples/android/settings.gradle(0 hunks)examples/app.json(0 hunks)examples/babel.config.js(0 hunks)examples/index.js(0 hunks)examples/ios/.xcode.env(0 hunks)examples/ios/Dynamic.swift(0 hunks)examples/ios/Podfile(0 hunks)examples/ios/examples-Bridging-Header.h(0 hunks)examples/ios/examples.xcodeproj/project.pbxproj(0 hunks)examples/ios/examples.xcodeproj/xcshareddata/xcschemes/examples.xcscheme(0 hunks)examples/ios/examples.xcworkspace/contents.xcworkspacedata(0 hunks)examples/ios/examples.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist(0 hunks)examples/ios/examples/AppDelegate.h(0 hunks)examples/ios/examples/AppDelegate.mm(0 hunks)examples/ios/examples/Images.xcassets/AppIcon.appiconset/Contents.json(0 hunks)examples/ios/examples/Images.xcassets/Contents.json(0 hunks)examples/ios/examples/Info.plist(0 hunks)examples/ios/examples/LaunchScreen.storyboard(0 hunks)examples/ios/examples/main.m(0 hunks)examples/ios/examplesTests/Info.plist(0 hunks)examples/ios/examplesTests/examplesTests.m(0 hunks)examples/metro.config.js(0 hunks)examples/package.json(0 hunks)ios/SplashScreen.xcodeproj/xcuserdata/penn.xcuserdatad/xcschemes/SplashScreen.xcscheme(0 hunks)ios/SplashScreen.xcodeproj/xcuserdata/penn.xcuserdatad/xcschemes/xcschememanagement.plist(0 hunks)ios/SplashScreen.xcodeproj/xcuserdata/taehyun.xcuserdatad/xcschemes/xcschememanagement.plist(0 hunks)issue_template.md(0 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (72)
- examples/app.json
- examples/.watchmanconfig
- examples/ios/examples.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
- android/proguard-rules.pro
- examples/babel.config.js
- android/.npmignore
- examples/ios/examples-Bridging-Header.h
- examples/metro.config.js
- examples/ios/examples/AppDelegate.h
- examples/ios/examples/Images.xcassets/Contents.json
- examples/android/app/src/main/res/values/colors.xml
- examples/.eslintrc.js
- examples/.buckconfig
- examples/android/app/src/main/res/values/styles.xml
- examples/ios/examplesTests/Info.plist
- .npmignore
- examples/android/build.gradle
- ios/SplashScreen.xcodeproj/xcuserdata/taehyun.xcuserdatad/xcschemes/xcschememanagement.plist
- examples/Gemfile
- examples/tests/App-test.js
- examples/index.js
- .gitignore
- examples/ios/examples.xcworkspace/contents.xcworkspacedata
- examples/android/app/src/main/jni/Android.mk
- examples/android/app/src/main/jni/MainApplicationTurboModuleManagerDelegate.h
- examples/android/app/proguard-rules.pro
- examples/ios/examples/Images.xcassets/AppIcon.appiconset/Contents.json
- examples/.ruby-version
- examples/ios/examples/main.m
- issue_template.md
- examples/android/app/src/main/jni/MainApplicationModuleProvider.h
- examples/android/app/src/main/AndroidManifest.xml
- examples/.bundle/config
- examples/android/app/src/main/jni/MainApplicationModuleProvider.cpp
- ios/SplashScreen.xcodeproj/xcuserdata/penn.xcuserdatad/xcschemes/xcschememanagement.plist
- CONTRIBUTING.md
- examples/ios/Dynamic.swift
- examples/.gitignore
- examples/android/app/src/debug/AndroidManifest.xml
- examples/android/app/src/main/res/layout/launch_screen.xml
- examples/android/settings.gradle
- examples/.prettierrc.js
- examples/android/app/src/main/res/values/strings.xml
- examples/ios/examples.xcodeproj/xcshareddata/xcschemes/examples.xcscheme
- ios/SplashScreen.xcodeproj/xcuserdata/penn.xcuserdatad/xcschemes/SplashScreen.xcscheme
- examples/android/app/src/main/jni/OnLoad.cpp
- examples/ios/Podfile
- examples/package.json
- examples/android/gradle.properties
- examples/ios/examples/LaunchScreen.storyboard
- examples/ios/examples/AppDelegate.mm
- examples/android/app/build.gradle
- examples/android/app/src/main/java/com/examples/MainApplication.java
- examples/android/app/src/main/res/drawable/rn_edit_text_material.xml
- examples/ios/.xcode.env
- examples/android/app/_BUCK
- examples/ios/examplesTests/examplesTests.m
- add-LaunchScreen-tutorial-for-ios.md
- examples/App.js
- examples/android/app/src/main/res/raw/loading.json
- examples/android/app/src/main/java/com/examples/newarchitecture/MainApplicationReactNativeHost.java
- examples/android/app/src/debug/java/com/examples/ReactNativeFlipper.java
- examples/android/app/src/main/java/com/examples/newarchitecture/components/MainComponentsRegistry.java
- examples/ios/examples/Info.plist
- examples/android/app/build_defs.bzl
- examples/android/app/src/main/java/com/examples/MainActivity.java
- examples/ios/examples.xcodeproj/project.pbxproj
- examples/android/app/src/main/java/com/examples/newarchitecture/modules/MainApplicationTurboModuleManagerDelegate.java
- examples/.flowconfig
- examples/android/app/src/main/jni/MainApplicationTurboModuleManagerDelegate.cpp
- examples/android/app/src/main/jni/MainComponentsRegistry.h
- examples/android/app/src/main/jni/MainComponentsRegistry.cpp
🔇 Additional comments (6)
package.json (2)
3-3: Version Bump Verified.
The version has been updated to"1.1.2". Please ensure that the changelog and any release notes are updated accordingly to reflect all new changes and potential impacts from this version bump.
46-55: Review React Native Module Mappings.
The new"react-native"field now adds explicit mappings for modules like"crypto"and several stream components. Verify that these mappings properly provide polyfills for Node.js modules in React Native environments. Additionally, confirm that these changes are fully tested across the target platforms to avoid runtime issues.android/src/main/java/org/devio/rn/splashscreen/SplashScreen.java (4)
7-8: LGTM: Added necessary import for full-screen functionality.The WindowManager import is correctly added to support the new full-screen display mode.
62-64: LGTM: Proper implementation of full-screen mode.Good approach to conditionally apply full-screen settings only when requested.
101-104: LGTM: Good method overload with theme selection.This method provides a convenient way to show the splash screen with appropriate theme selection based on the full-screen parameter.
134-137: Verify animation finish condition.The dialog is only dismissed if
isAnimationFinishedis true, which is good for ensuring the animation completes before dismissal. However, consider adding a timeout mechanism in case the animation never completes or fails.Does the animation have a fixed duration? If not, consider adding a timeout to prevent the splash screen from staying indefinitely if animation fails to complete.
1 participant
Summary by CodeRabbit
New Features
Chores