forked from facebook/react-native
-
Notifications
You must be signed in to change notification settings - Fork 149
Merge up to commit where view configs are refactored #1620
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
analysis-bot
suggested changes
Jan 5, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code analysis results:
eslint
found some issues. Runyarn lint --fix
to automatically fix problems.
Saadnajmi
commented
Jan 5, 2023
Base commit: 68c0b06 |
Summary: Calls to Inspector::evaluate() and Inspector::executeIfEnabled() take a user-provided callback which are not try-guarded. This means that, should the user code throw, the inspector's process will likely die due to the unhandled exception. This change makes sure that, if the user provided callback throws, then the promise attached to those methods will be fulfilled by an exception. Change: [internal] Reviewed By: avp Differential Revision: D33852073 fbshipit-source-id: 7fbb6662b28d393a5d5b494c004aa9521e23ebb6
Summary: changelog: [internal] React on web uses microtasks to schedule a synchronous update for discrete event. Microtasks are not yet available in React Native and as a fallback, React uses native scheduler and task with immediate priority. Until Microtasks are in place, React Native needs to make sure all immediate tasks are executed after it dispatches each event. I tried to stay as close to [microtask standard](https://developer.mozilla.org/en-US/docs/Web/API/queueMicrotask) as I reasonably could. Once microtasks on RN are shipped, this code can be removed. Reviewed By: mdvacca Differential Revision: D33742583 fbshipit-source-id: eddebb1bd5131ee056252faad48327a70de78a4a
…ig() work on Venice Summary: Changelog: [Internal] Reviewed By: RSNara Differential Revision: D33860658 fbshipit-source-id: 41079b13acef531877c82dc0b2063dbe2b42edcf
Summary: Changelog: [Internal] Remove all the MCs that enable SVCs in Fabric, because we'll only test SVCs in Bridgeless mode to simplify rollout. There were complications with enabling SVCs in Fabric at a previous rollout. Reviewed By: RSNara Differential Revision: D33861243 fbshipit-source-id: fdbfedce77f8bd1bab2a807237017787ae8bf7c1
…e - for codegenNativeComponent Summary: Changelog: [Internal] Since DummyUIManager.getViewManagerConfig() & hasViewManagerConfig() are the same, it's safe to ship this before the native changes in this stack lands. Reviewed By: RSNara Differential Revision: D33832926 fbshipit-source-id: c0f0a169d02397e0f9125bb45d95d395c8bbc492
…e - Remove __fbStaticViewConfig & UIManagerInjection Summary: Changelog: [Internal] Reviewed By: RSNara Differential Revision: D33835081 fbshipit-source-id: ed625de3b2da73f98cdb9c9dc97086aa2c477e3a
Summary: Adds support for Animated.Color with native driver for Android. Reads the native config for the rbga channel AnimatedNodes, and on update(), converts the values into an integer (0xaarrggbb) Followup changes will include support for iOS and platform colors. Changelog: [Android][Added] - Support running animations with AnimatedColor with native driver Reviewed By: javache Differential Revision: D33833600 fbshipit-source-id: 2bf05c9715b603cf014ace09e9308b2bfd67f30a
Summary: JS changes to support D32138347 (facebook@a96bdb7). This was previously reverted due to missing iOS Paper support. Changelog: [Android][Fixed] Enable hitSlop to be set using a single number. Original commit changeset: 91cfcc86582c Original Phabricator Diff: D32559015 (facebook@589b129) Reviewed By: yungsters Differential Revision: D33453327 fbshipit-source-id: d289a0a8b8208bc9c68e6ca537632b745e8196ed
…ok#32989) Summary: Fixes a potential crash was introduced by facebook#30919 that aimed to get the keyboard height on devices with a Notch. The problem is that it considers that any ReactRootView will have an insets available. When using [react-native-navigation](https://github.com/wix/react-native-navigation) and assigning a Navigation button to the TopBar as a component, the component gets registered as a RootView but won't have any insets attach to the view. [getRootWindowInsets()](https://developer.android.com/reference/android/view/View#getRootWindowInsets()) in fact return a `WindowInset` only available if the view is attached, so when executing `checkForKeyboardEvents` method from ReactRootView, is trying to access the `DisplayCutout` of a null object, leading to a crash. ## Changelog [Android] [Fixed] - Fix potential crash if ReactRootView does not have insets attached. Pull Request resolved: facebook#32989 Test Plan: Without the code change: Notice how the second screen being push contains a React Component on the top right of the navigation bar, and when component is unmounted (going back) the app crashes. https://user-images.githubusercontent.com/6757047/151558235-39b9a8b5-be73-4c31-8053-02ce188637b8.mp4 crash log ``` 2022-01-28 10:27:52.902 15600-15600/com.mattermost.rnbeta E/AndroidRuntime: FATAL EXCEPTION: main Process: com.mattermost.rnbeta, PID: 15600 java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.DisplayCutout android.view.WindowInsets.getDisplayCutout()' on a null object reference at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:778) at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:769) at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3214) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2143) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8665) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037) at android.view.Choreographer.doCallbacks(Choreographer.java:845) at android.view.Choreographer.doFrame(Choreographer.java:780) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7839) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) ``` After applying the patch which is only a null check validation and does not change any previous behavior https://user-images.githubusercontent.com/6757047/151558429-9ff1a608-abb6-4168-8db9-df0c3c71d79e.mp4 Reviewed By: cortinico Differential Revision: D33844955 Pulled By: ShikaSD fbshipit-source-id: ed5579ad3afeed009c61cc1851eee45c70087cf5
Summary: Changelog for 0.67.2 ## Changelog [Internal][Changed] - Release Changelog Pull Request resolved: facebook#33007 Test Plan: N/A Reviewed By: cortinico Differential Revision: D33894314 Pulled By: ShikaSD fbshipit-source-id: fc7571e39adc25e2f39db362f35d685cbc10d096
Summary: Android 11 (API 30) introduced a new interface for changing the appearance of the status bars with [`WindowInsetsController#setSystemBarsAppearance`](https://developer.android.com/reference/kotlin/android/view/WindowInsetsController#setsystembarsappearance) and deprecated using the `WindowManager#systemUiVisibility` properties. Apparently, once you call `setSystemBarsAppearance` Android will no longer respect `systemUiVisibility` and if anyone, such as the Android 12 Splash Screen library, happens to call it, it will break status bars. This PR augments the RN StatusBarModule to use the new interface on Android 11+. Also updated the rn-tester app, see video. https://user-images.githubusercontent.com/1124321/151321561-8202e237-cf7d-45ce-b957-18b5bafd17c4.mov ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Changed] - Use new StatusBar API on Android 11 (API 30)+ Pull Request resolved: facebook#32975 Reviewed By: cortinico Differential Revision: D33814853 Pulled By: ShikaSD fbshipit-source-id: c0f2651015dddb4871a3e3b26642f76a46da2a76
…brary with direct call to fb_java_library_android Summary: Replace fb_xplat_platform_specific_rule calls where rule = fb_java_library with direct call to fb_java_library_android Reviewed By: alexmalyshev Differential Revision: D33852709 fbshipit-source-id: 1ff3a1225e681d0924ec04e955b0039724182b1c
… rule = fb_java_library with direct call to fb_java_library_android Differential Revision: D33852709 (facebook@dc507be) Original commit changeset: 1ff3a1225e68 Original Phabricator Diff: D33852709 (facebook@dc507be) fbshipit-source-id: 10db2d1bda1ea69b9a0226041493af06b78c16c4
Summary: ## Impact Fix the Static ViewConfig for <View/>. This diff fixes the base ViewConfig for all HostComponents on both platforms. Consequently, it simplifies SVC reconciliation efforts, by nearly eliminating the first of these classes of SVC errors: 1. Unexpected properties in SVC 2. Missing properties in SVC 3. Not matching properites in SVC ## What is the base ViewConfig on each iOS/Android? **On iOS:** - All props come from ViewManagers - All HostComponent ViewManagers extend <View/> ViewManager https://pxl.cl/1SxdF Therefore, the base ViewConfig for all components should be <View/>'s static ViewConfig. **On Android:** The component model is a bit more complicated: https://pxl.cl/1Vmp5 Takeaways: - Props come from Shadow Nodes **and** ViewManagers - Nearly all HostComponent ViewManagers extend BaseViewManager. But, that's not <View/>'s ViewManager. - <View/>'s ViewManager is [ReactViewManager](https://fburl.com/code/0zalv8zk), which is a descendent of BaseViewManager, and declares its own ReactProps. So, on Android, it's not safe for the base ViewConfig to be <View>'s ViewConfig: 1. No components actualy incorportate <View/>'s props 2. Some components don't even incorporate BaseViewManager's props. So, what should the base ViewConfig be on Android? - Nearly all components extend BaseViewManager. BaseViewManager must have a shadow node [that extends LayoutShadowNode](https://www.internalfb.com/code/fbsource/[47d68ebc06e64d97da9d069f1ab662b392f0df8a]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java?lines=40). Therefore, we'll make the base ViewConfig on Android be generated by BaseViewManager + LayoutShadowNode. ## Changes In this diff, I removed ReactNativeViewViewConfig, and introduced a new view config called PlatformBaseViewConfig. This ViewConfig partial will capture all the props available on all HostComponents on **both** platforms. This may not necessarily be the props made available on <View/>. The only components that don't extend the base platform props are: RCTTextInlineImage. What we do with these components is TBD. Changelog: [Internal] Reviewed By: p-sun, yungsters Differential Revision: D33135055 fbshipit-source-id: 7299f60ae45ed499ce47c0d0a6309a047bff90bb
chiuam
approved these changes
Jan 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary
Merge up to facebook@7b9490b where View configs are refactored. Note that in facebook@033ad83 , these View configs are refactored again, at which point I'll make a proper
BaseViewConfig.macos.js
file.macOS specific commits:
ReactNativeViewViewConfigMacOS.js
, and merge it into the newPlatformBaseViewConfig.js
.Test Plan
CI should pass. Can locally build RNTester and macOS specific props like
cursor
andonMouseEnter
seem to work fine.