From 6a4abd8805196e0e99f658a96c52875cdca64a7c Mon Sep 17 00:00:00 2001 From: Shawn Dempsey Date: Tue, 26 Sep 2023 13:06:24 -0400 Subject: [PATCH 1/4] [fabric] Add RCTScrollContentComponentView Fabric Native View Summary: **Context** - On Fabric, the ScrollView content Fabric component was never added for macOS. - We need this content view to manage the `inverted` flag on our scrollviews. Mobile does not have this requirement. **Change** - Add Scroll Content Fabric component view - Fix the js spec w/ codegen support & Paper fallback - Remove ScrollContentView from the RCTLegacyViewManagerInteropComponent fallback list Test Plan: |Paper|| |{F1102578530}| https://pxl.cl/3sqgW| |Fabric|| | {F1102600561} | https://pxl.cl/3sqn5 | Reviewers: lefever, #rn-desktop Subscribers: ramanpreet Differential Revision: https://phabricator.intern.facebook.com/D49645029 Tags: uikit-diff --- .../ScrollContentViewNativeComponent.js | 30 ++++------- .../RCTFabricComponentsPlugins.h | 3 ++ .../RCTFabricComponentsPlugins.mm | 3 ++ .../RCTScrollContentComponentView.h | 20 +++++++ .../RCTScrollContentComponentView.mm | 54 +++++++++++++++++++ 5 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.h create mode 100644 packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js b/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js index bd7278c93d0dee..b07cc82dbae124 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js @@ -8,25 +8,17 @@ * @flow */ -import type { - HostComponent, - PartialViewConfig, -} from '../../Renderer/shims/ReactNativeTypes'; -import type {ViewProps as Props} from '../View/ViewPropTypes'; +import type {HostComponent} from '../../Renderer/shims/ReactNativeTypes'; +import type {ViewProps} from '../View/ViewPropTypes'; -import * as NativeComponentRegistry from '../../NativeComponent/NativeComponentRegistry'; +import codegenNativeComponent from '../../Utilities/codegenNativeComponent'; -export const __INTERNAL_VIEW_CONFIG: PartialViewConfig = { - uiViewClassName: 'RCTScrollContentView', - bubblingEventTypes: {}, - directEventTypes: {}, - validAttributes: {}, -}; +type NativeProps = $ReadOnly<{| + ...ViewProps, + inverted?: ?boolean, +|}>; -const ScrollContentViewNativeComponent: HostComponent = - NativeComponentRegistry.get( - 'RCTScrollContentView', - () => __INTERNAL_VIEW_CONFIG, - ); - -export default ScrollContentViewNativeComponent; +export default (codegenNativeComponent('ScrollContentView', { + paperComponentName: 'RCTScrollContentView', + excludedPlatforms: ['android'], +}): HostComponent); diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h b/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h index ca5f395c3141a8..f01bc9c29d77d2 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h @@ -36,6 +36,9 @@ Class RCTParagraphCls(void) __attribute__((used)); Class RCTPullToRefreshViewCls(void) __attribute__((used)); Class RCTSafeAreaViewCls(void) __attribute__((used)); Class RCTScrollViewCls(void) __attribute__((used)); +#if TARGET_OS_OSX // [macOS +Class RCTScrollContentViewCls(void) __attribute__((used)); +#endif // macOS] Class RCTSwitchCls(void) __attribute__((used)); Class RCTTextInputCls(void) __attribute__((used)); Class RCTUnimplementedNativeViewCls(void) __attribute__((used)); diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm index 02d1dc1a125af7..fac9f32e44a281 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm @@ -24,6 +24,9 @@ {"PullToRefreshView", RCTPullToRefreshViewCls}, {"SafeAreaView", RCTSafeAreaViewCls}, {"ScrollView", RCTScrollViewCls}, +#if TARGET_OS_OSX // [macOS + {"ScrollContentView", RCTScrollContentViewCls}, +#endif // macOS] {"Switch", RCTSwitchCls}, {"TextInput", RCTTextInputCls}, {"UnimplementedNativeView", RCTUnimplementedNativeViewCls}, diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.h b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.h new file mode 100644 index 00000000000000..e35e190d016382 --- /dev/null +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.h @@ -0,0 +1,20 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import // [macOS] + +#import + +NS_ASSUME_NONNULL_BEGIN + +@interface RCTScrollContentComponentView : RCTViewComponentView + +@property (nonatomic, assign, getter=isInverted) BOOL inverted; + +@end + +NS_ASSUME_NONNULL_END diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm new file mode 100644 index 00000000000000..2f2966f877f4a7 --- /dev/null +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm @@ -0,0 +1,54 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "RCTScrollContentComponentView.h" + +#import +#import +#import + +#import "RCTFabricComponentsPlugins.h" + +using namespace facebook::react; + +@implementation RCTScrollContentComponentView + + - (instancetype)init + { + if (self = [super init]) { + _props = std::make_shared(); + } + + return self; + } + +- (void)updateProps:(Props::Shared const&)props oldProps:(Props::Shared const&)oldProps { + const auto& newViewProps = *std::static_pointer_cast(props); + + [self setInverted:newViewProps.inverted]; + + [super updateProps:props oldProps:oldProps]; +} + +- (BOOL)isFlipped +{ + return !self.inverted; +} + +#pragma mark - RCTComponentViewProtocol + ++ (ComponentDescriptorProvider)componentDescriptorProvider +{ + return concreteComponentDescriptorProvider(); +} + +Class RCTScrollContentViewCls(void) +{ + return RCTScrollContentComponentView.class; +} + +@end From b824fa001aba7ca6b9077ae71edc804b98745b42 Mon Sep 17 00:00:00 2001 From: Shawn Dempsey Date: Fri, 13 Oct 2023 11:37:30 -0700 Subject: [PATCH 2/4] [fabric] ScrollContentView should be mounted as document view of ScrollView Summary: **Context** - D49645029 Added a Fabric ScrollContentView - ScrollView.js adds this `contentContainer` view from RN https://www.internalfb.com/code/fbsource/[381a6f37779b]/third-party/microsoft-fork-of-react-native/react-native/Libraries/Components/ScrollView/ScrollView.js?lines=1742-1759 - When the view is mounted, it was being inserted as subview instead of being added as the `documentView` which is required by AppKit. https://www.internalfb.com/code/fbsource/[381a6f37779b]/third-party/microsoft-fork-of-react-native/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm?lines=372-376 - This was inserting a `RCTUIView` in the view hierarchy and the ScrollContentView was not working correctly {F1119521762} **Change** - ScrollContentView is added as `documentView` on mount - Document view is reset to initial value (Empty RCTUIView) on unmount Test Plan: |Fabric| | {F1119532976} | Reviewers: chpurrer, lefever, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D50276834 --- .../ScrollView/RCTScrollViewComponentView.mm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 81a0b79d6fd98e..fe3f3eeabacdee 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -382,19 +382,27 @@ - (void)_preserveContentOffsetIfNeededWithBlock:(void (^)())block - (void)mountChildComponentView:(RCTUIView *)childComponentView index:(NSInteger)index // [macOS] { +#if !TARGET_OS_OSX // [macOS] [_containerView insertSubview:childComponentView atIndex:index]; if (![childComponentView conformsToProtocol:@protocol(RCTCustomPullToRefreshViewProtocol)]) { _contentView = childComponentView; } +#else // [macOS + [_scrollView setDocumentView:childComponentView]; +#endif // macOS] } - (void)unmountChildComponentView:(RCTUIView *)childComponentView index:(NSInteger)index // [macOS] { +#if !TARGET_OS_OSX // [macOS] [childComponentView removeFromSuperview]; if (![childComponentView conformsToProtocol:@protocol(RCTCustomPullToRefreshViewProtocol)] && _contentView == childComponentView) { _contentView = nil; } +#else // [macOS + [_scrollView setDocumentView:_containerView]; +#endif // macOS] } /* From 15f29d6d20fde02e612dc91e63b4e1a77324f11c Mon Sep 17 00:00:00 2001 From: Nick Lefever Date: Fri, 15 Mar 2024 01:37:46 +0100 Subject: [PATCH 3/4] [fabric] Add a custom shadow node to ScrollContentComponentView Summary: This diff adds a custom shadow node for the ScrollContentView component, allowing to propagate the view flipping state to the shadow tree. The ScrollContentView was using a generated shadow node. The generation of the descriptor and shadow node was disabled in the spec and a custom implementation was added for the component. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Subscribers: ramanpreet Differential Revision: https://phabricator.intern.facebook.com/D54931340 Tasks: T182039265, T182037720 --- .../ScrollContentViewNativeComponent.js | 1 + .../RCTScrollContentComponentView.mm | 2 +- .../ScrollContentViewComponentDescriptor.h | 20 +++++++++++++ .../ScrollContentViewShadowNode.cpp | 17 +++++++++++ .../scrollview/ScrollContentViewShadowNode.h | 30 +++++++++++++++++++ 5 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewComponentDescriptor.h create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js b/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js index b07cc82dbae124..eb5199158b079e 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollContentViewNativeComponent.js @@ -21,4 +21,5 @@ type NativeProps = $ReadOnly<{| export default (codegenNativeComponent('ScrollContentView', { paperComponentName: 'RCTScrollContentView', excludedPlatforms: ['android'], + interfaceOnly: true, }): HostComponent); diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm index 2f2966f877f4a7..636dc7dde67897 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollContentComponentView.mm @@ -7,7 +7,7 @@ #import "RCTScrollContentComponentView.h" -#import +#import #import #import diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewComponentDescriptor.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewComponentDescriptor.h new file mode 100644 index 00000000000000..87973d811fff5a --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewComponentDescriptor.h @@ -0,0 +1,20 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook { +namespace react { + +using ScrollContentViewComponentDescriptor = + ConcreteComponentDescriptor; + +} // namespace react +} // namespace facebook diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp new file mode 100644 index 00000000000000..abdeb903712347 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp @@ -0,0 +1,17 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "ScrollContentViewShadowNode.h" + +#include +#include + +namespace facebook::react { + +const char ScrollContentViewComponentName[] = "ScrollContentView"; + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h new file mode 100644 index 00000000000000..126a3ef8476197 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h @@ -0,0 +1,30 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include + +namespace facebook { +namespace react { + +extern const char ScrollContentViewComponentName[]; + +/* + * `ShadowNode` for component. + */ +class ScrollContentViewShadowNode final : public ConcreteViewShadowNode< + ScrollContentViewComponentName, + ScrollContentViewProps> { + public: + using ConcreteViewShadowNode::ConcreteViewShadowNode; +}; + +} // namespace react +} // namespace facebook From e9aff24fc205a53119a7bea627df8adae6c4225e Mon Sep 17 00:00:00 2001 From: Nick Lefever Date: Fri, 15 Mar 2024 01:42:51 +0100 Subject: [PATCH 4/4] [fabric] Add view flipping propagation to ScrollView/ScrollContentView shadow nodes Summary: To enable the correct layout metrics transformation with flipped views, we override the `LayoutableShadowNode` function `getIsVerticalAxisFlipped` to return the current configuration of the component. This enables the shadow tree to correctly convert coordinates to the right axis when going through these component instances that have their axis flipped. Test Plan: - Run Zeratul with Fabric enabled - Hover over a reaction - Check that the reaction summary popup is positioned above the reaction bubble - Hover over a message bubble - Check that the timestamp popup is positioned level with the center of the message bubble - Test this out at the top/bottom of the scroll view clipping area. | Before | After | |--| | https://pxl.cl/4vKvG | https://pxl.cl/4vKvg | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54931342 Tasks: T182039265, T182037720 --- .../scrollview/ScrollContentViewShadowNode.cpp | 6 ++++++ .../scrollview/ScrollContentViewShadowNode.h | 10 ++++++++-- .../components/scrollview/ScrollViewShadowNode.cpp | 6 ++++++ .../components/scrollview/ScrollViewShadowNode.h | 4 ++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp index abdeb903712347..8207e1067a651a 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.cpp @@ -14,4 +14,10 @@ namespace facebook::react { const char ScrollContentViewComponentName[] = "ScrollContentView"; +#if TARGET_OS_OSX // [macOS +bool ScrollContentViewShadowNode::getIsVerticalAxisFlipped() const { + return getConcreteProps().inverted; +} +#endif // macOS] + } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h index 126a3ef8476197..c33048d612258c 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollContentViewShadowNode.h @@ -20,10 +20,16 @@ extern const char ScrollContentViewComponentName[]; * `ShadowNode` for component. */ class ScrollContentViewShadowNode final : public ConcreteViewShadowNode< - ScrollContentViewComponentName, - ScrollContentViewProps> { + ScrollContentViewComponentName, + ScrollContentViewProps> { public: using ConcreteViewShadowNode::ConcreteViewShadowNode; + +#pragma mark - LayoutableShadowNode + +#if TARGET_OS_OSX // [macOS + bool getIsVerticalAxisFlipped() const override; +#endif // macOS] }; } // namespace react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp index a8ecce51c482f7..9a4771b7f1458c 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp @@ -69,4 +69,10 @@ Point ScrollViewShadowNode::getContentOriginOffset() const { return {-contentOffset.x, -contentOffset.y + stateData.scrollAwayPaddingTop}; } +#if TARGET_OS_OSX // [macOS +bool ScrollViewShadowNode::getIsVerticalAxisFlipped() const { + return getConcreteProps().inverted; +} +#endif // macOS] + } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h index cc4d7e119cc2d1..0157c6f2c14b72 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h @@ -39,6 +39,10 @@ class ScrollViewShadowNode final : public ConcreteViewShadowNode< void layout(LayoutContext layoutContext) override; Point getContentOriginOffset() const override; +#if TARGET_OS_OSX // [macOS + bool getIsVerticalAxisFlipped() const override; +#endif // macOS] + private: void updateStateIfNeeded(); void updateScrollContentOffsetIfNeeded();