[fabric] Fixes to core fabric components #2117

shwanton · 2024-05-01T06:42:32Z

Summary:

Upstream Fabric fixes for Views, Text & TextInput

NOTE: TouchHandler & ScrollView changes will be added once this PR lands.

Test Plan:

Fabric

View

CleanShot.2024-05-01.at.16.53.12.mp4

Paper

View

CleanShot.2024-05-01.at.17.02.56.mp4

Summary: Fix AppKit exception throws when focusing text inputs by calling becomeFirstResponder directly on the backing text input view. Making a view first responder has to happen through the window using makeFirstResponder. Test Plan: - Run Zeratul with Fabric - Focus the search text input above the message threads - Click inside the active message thread to trigger the auto-focus of the composer - The composer gets focus without AppKit throwing an exception. https://pxl.cl/3dVMx Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D48696690

…Component Summary: Hit testing in RN macOS uses the view coordinate space instead of the macOS superview coordinate space. The RCTView hitTest implementation gets called from Fabric when Paper components are loaded through the `LegacyViewManagerInteropComponent`. When the Paper component has Fabric child components, the hitTest implementation would treat those as native macOS views. This diff adds a selector check to detect Fabric RCTViewComponentView instances and apply the right point conversion. The selector check allows to have the right behavior without having to import Fabric classes in Paper code. Test Plan: - Run Workplace Chat with Fabric enabled - Click on a thread title in the thread list - With the fix, the thread gets selected Reviewers: shawndempsey, ericroz, chpurrer, #rn-desktop Reviewed By: shawndempsey, ericroz Differential Revision: https://phabricator.intern.facebook.com/D49083593 Tasks: T163162857

Summary: This diff conditionally sets the CALayer masksToBounds based on the clipsToBounds RN property so that the content of the view would be clipped by the view border. Test Plan: - Run Zeratul with Fabric enabled - Check that the profile pictures with no status indicator are clipped by the half size border radius set on the parent view. | Before | After | |--| | {F1090251649} | {F1090249259} | Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D49239973

…yout manager Summary: To render text using an NSTextView, we need to gain access to a fully configured NSTextStorage to configure the text view to render the text with the right configuration. Test Plan: Tested later in this stack. Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49465173 Tasks: T163838519

Summary: Use an NSTextView to render the text in RCTParagraphComponentView so that we would get UX interactions specific to desktop for free (e.g. text selection) Test Plan: - Run Zeratul with Fabric enabled - Check that text is being rendered correctly with the right size and positioning. {F1097505779} Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49465175 Tasks: T163838519

Summary: Override the hitTest and mouseDown handler in `RCTParagraphComponentView` to forward mouse drag events to the underlying NSTextView to get text selection support in Fabric with correct rendering. Test Plan: - Run Zeratul with Fabric enabled. - Select text https://pxl.cl/3q3b3 Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49465174 Tasks: T163838519

…er components Summary: The tag value is stored in the `reactTag` property on macOS. The adapter used by the RCTLegacyViewManagerInteropComponentView was being provided the `tag` property value which set all interceptors to tag -1 leading to incorrect event dispatching on the JS side. Test Plan: - Open the settings pane in Zeratul and select the appearance tab. - Toggle the theme PopUpButton With the fix, the theme changes while before the zoom would change. https://pxl.cl/3vZRd Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49897662

Summary: See title Test Plan: Build RNTester Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49924996

Summary: This overrides the correct `hitTest` method to conditionally forward NSResponder events to the component or the underlying native text view to handle onPress and text selection. RN will use `hitTest:withEvent:` while native components use `hitTest:`. Without overriding the method used by RN, the conditional forwarding is being bypassed and mouse clicks would get forwarded to the underlying native text view component. Test Plan: - Allowlist TU for the zeratul_enable_fabric_preferences_surface GK - Run Zeratul - Open the settings - Select *Active status* - Click on the *Learn more* link With the fix the link loads in the browser: https://pxl.cl/3z70J Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D50087800 Tasks: T166059211

Summary: **Context** - Our Fabric ViewComponentView supports the focus prop but it is not being sent via native props **Change** - Add the native prop to `ReactCommon/.../ViewProps.h` - Updated `focusable` property on `RCTViewComponentView` Test Plan: |RNTester - Fabric| | https://pxl.cl/3wLwF | |RNTester - Paper| || Reviewers: lefever, #rn-desktop Differential Revision: https://phabricator.intern.facebook.com/D49998664 [upstream][fabric] Add enableFocusRing for Fabric ViewComponentView Summary: **Context** - `focusable` prop was added to Fabric ViewComponent in D49998664 - `enableFocusRing` is available on Paper view **Change** - Add the native prop to `ReactCommon/.../ViewProps.h` - Updated `enableFocusView` property on RCTViewComponentView Test Plan: NOTE: Since views are flattened, the focusable view w/ enable focus ring won't be visible till D50102747 |Fabric| | https://pxl.cl/3z9Wm| Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D50102547

Summary: **Context** - Focusable props were added in D49998664 and D50102547 - The ViewComponentView is flattened so when the component had children, the focus view would be flattened away. **Change** - Do not flatten the view if `focusable` or `enableFocusView` are props on the Fabric View Test Plan: |Fabric| | https://pxl.cl/3z9X4| Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D50102747

Summary: On macOS, Core Animation layers have their anchor point set to (0,0) which is the top-left. On iOS the anchor point is set to (0.5, 0.5) which matches the center. Transforms assigned to views are built based on having the anchor point at the center. This diff updates the transform matrix to apply to transform from the center. Test Plan: Check zoom/rotation animations in Zeratul. | Before | After | |--| | https://pxl.cl/3G8HG | https://pxl.cl/3G8HQ | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D50628807 Tasks: T161413049

…stance Summary: Fix AppKit exception throws when blurring text inputs by calling `resignFirstResponder` directly on the backing text input view. Resigning the first responder state has to happen through the window by calling `[window makeFirstResponder:nil]` which will: - call `resignFirstResponder` on the current first responder - if successful, the window will become the first responder Test Plan: - Run Zeratul with Fabric - Focus the search text input above the message threads - Click inside the active message thread to trigger the auto-focus of the composer and the blur on the search field - The focused field resigns the first responder status without throwing an exception https://pxl.cl/3GvZD Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D50700782

Summary: Scroll views set up with an inverted vertical axis can't support view flattening due to the flattening not taking the axis inversion in consideration while repositioning the views. This diff disables view flattening on the cells of the virtualized list so that the layout would be correct in inverted scroll views when using Fabric. The change is not being applied to ScrollView directly because we can safely assume that vertical axis inversion will only be enabled on VirtualizedList/FlatList. Test Plan: Run Zeratul with Fabric and check that the vertical order of grouped bubble messages is correct. | Before | After | |--| | {F1136386200} | {F1136386364} | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D50846483 Tasks: T167539420

…hit testing Summary: Native views use the AppKit api for hit testing which requires the point to be in the superview coordinate system. This diff updates the hit testing in `RCTViewComponentView` to conditionally converts the point to the target view coordinate system only if the tested view is a react view. Test Plan: Run Zeratul with Fabric and select text inside message bubbles. The scroll view being a native view, the hit testing does not require a point conversion. With this change, the text selection works as expected. | Before | After | |--| | https://pxl.cl/3Mlpb | https://pxl.cl/3MllN | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D51129375 Tags: uikit-diff

Summary: Picking out this from D51015931, will import to fbsource. Goal is upgrading visual studio tools on windows from the good ol' ancient vs2017. Test Plan: CI on D46923145 Reviewers: lyahdav, ericroz Reviewed By: ericroz Differential Revision: https://phabricator.intern.facebook.com/D51140141

…ents of VirtualizedList Summary: View flattening was already disabled in the cell renderer used by the Virtualized List in this diff D50846483 This diff disables view flattening in the header, footer, empty and spacer cells to fix the layout being broken because of the vertical axis flipping used by the reverse order virtualized list. Test Plan: Run Zeratul with Fabric enabled and scroll to the top of a message thread to show the participants summary header. | Before | After | |--| | {F1145726580} | {F1145726618} | Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51182545 Tasks: T167539420

Summary: **Context** - Port Paper TextInput api's to Fabric https://www.internalfb.com/code/fbsource/[dab91113a70a2f967fa2996f4aca0f49a58aea17]/third-party/microsoft-fork-of-react-native/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m?lines=440-444 **Change** - Automatic spelling correction works on Fabric TextInput Test Plan: **Enable "Correct Spelling Automatically"** |https://pxl.cl/3MFjJ| https://pxl.cl/3MFx6| {F1146645411} Reviewers: chpurrer, lefever, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51158136

Summary: **Context** - Port Paper TextInput api's to Fabric https://www.internalfb.com/code/fbsource/[25297dddce1b]/third-party/microsoft-fork-of-react-native/react-native-macos/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.mm?lines=500-505 **Change** - Automatic spell checking works on Fabric TextInput Test Plan: **Enable "Check Spelling While Typing"** https://pxl.cl/3MG01 Reviewers: chpurrer, lefever, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51177294

Summary: **Context** - Port Paper TextInput api's to Fabric https://www.internalfb.com/code/fbsource/[25297dddce1b]/third-party/microsoft-fork-of-react-native/react-native-macos/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.mm?lines=507-512 **Change** - Automatic grammar checking works on Fabric TextInput Test Plan: Enable "Check Grammar With Spelling" https://pxl.cl/3MWj6 {F1146663064} Reviewers: chpurrer, lefever, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51179163

Summary: `BOOL` being a char type, when building with our current BUCK config we need to static cast it to `bool` in Obj-C++ files when assigning those to a `bool`. Test Plan: Build Zeratul with BUCK. ``` BUILDING: FINISHED IN 5m 13.7s (100%) 11302/11302 JOBS, 6/11302 UPDATED BUILD SUCCEEDED ``` Reviewers: shawndempsey Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D51258122

Summary: **Context** - Text selection for Fabric should match Paper **Change** - Use correct position for selection range - Maintain cursor position with selection Test Plan: https://pxl.cl/3NW99 Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D51282825

Summary: Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text. This change will allow to select UI elements based on the text contents. Test Plan: Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText: {F1162808272} Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51736932 Tasks: T170938725 Tags: uikit-diff

Summary: Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text. This change will allow to select UI elements based on the text contents. Test Plan: Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText: {F1162808272} Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51736931 Tasks: T170938725

Summary: Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text. This change will allow to select UI elements based on the text contents. Test Plan: Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText: {F1162808272} Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51736933 Tasks: T170938725

Summary: Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text. This change will allow to select UI elements based on the text contents. Test Plan: Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText: {F1162808272} Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51736954 Tasks: T170938725

Summary: The multiline text input view on macOS needs its own view hierarchy, wrapping the RCTUITextView in a scroll view to support all the features offered by the React TextInput component. This diff adds a wrapper class for RCTUITextView that provides the appropriate view hierarchy while still supporting the text input protocols required for text input. The wrapper forwards all unimplemented methods to the RCTUITextView so that it can be used as a direct substitute for the RCTUITextView. This allows us to reduce the custom changes need for macOS in RCTTextInputComponentView while re-using all the logic in RCTUITextView. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D51962394 Tasks: T167538822, T157889591 Tags: uikit-diff

Summary: Add a `responder` property to support assigning the first responder to the actual textfield/textview if the view is wrapped or not. The wrapped text view already implements this property. This diff brings the same functionality to the text field and declares it on the common protocol. Test Plan: Tested later in this stack. Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey, chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51962395 Tasks: T167538822, T157889591

Summary: We're using wrapped text views with method forwarding, which enables a view to have fully supported text input interfaces. The text input copy helper method signature was limiting its use to RCTUITextView. To support wrapped text views the typing was changed to RCTPlatformView. All properties used in the implementation of the copy method are declared on RCTBackedTextInputViewProtocol. Test Plan: Tested later in this stack. Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey, chpurrer Differential Revision: https://phabricator.intern.facebook.com/D51962397 Tasks: T167538822, T157889591

Summary: This diff updates the core TextInput RN component to use the wrapped text view for multiline TextInput. Switching to `RCTWrappedTextView` enables correct `borderWidth` and `contentInsets` support for multi line text inputs while maintaining the same functionality for single line text input. Scrolling text views are also supported correctly, with vertical height dependent scrollers. Test Plan: - Build Zeratul with Fabric enabled. - Type in the search field to test the layout of the text contents - Type in the composer to test multi line support and the layout of the text contents | Before | After | |--| | https://pxl.cl/3Xrrx | https://pxl.cl/3Xrr9 | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D51962396 Tasks: T167538822, T157889591

Summary: See title. Test Plan: Sandcastle build D53695263 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53711568

Summary: See title. Test Plan: Sandcastle build D53695263 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53715088

Summary: This diff fixes a crash happening when a key up/down event was processed by a non-focusable view with no "handled keys" configured on the view. This would lead to an iteration over an optional vector that had no values set. Test Plan: * Run Zeratul with Fabric enabled * Focus the search field * Tab to toggle the focus until it reaches the message thread list * Tab to the next focusable view | Before | After | |--| | https://pxl.cl/4n91l | https://pxl.cl/4n910 | | Crash when tabbing out of thread list | No more crash | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53930899

Summary: See title. Test Plan: Sandcastle build. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53931156

Summary: This diff extends the macOS view event emitter to support sending blur and focus events. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53931155

Summary: This diff overrides the first responder tracking methods to submit focus/blur events when the native view becomes the first responder (focus) and resigns the first responder state (blur). Test Plan: * Update a view to log to the console when it is focused/blurred * Run Zeratul with Fabric enabled * Focus/blur the view and check the logs to verify that the event handlers are being called https://pxl.cl/4n99N Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53931157

Summary: This diff adds the `doubleClick` event emitter to the macOS view event emitter with tracking of handler assignment on the component to test if a handler is set or not on the component to conditionally enable double click event forwarding. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54007807

Summary: This diff adds support for double click event handling to the `View` component by overriding the `mouseUp` NSResponder method and track when a double click happened for a component that has JS double click handler set. The mouse event generation method was refactored to use an enum for the event type to support enter/leave/double-click event emissions. Test Plan: * Run Zeratul with Fabric enabled * Double click on the window drag handler area on the top of the window * The JS-based window zoom action should be triggered https://pxl.cl/4nwrl Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54007810

…tsUpdateLayer Summary: As title Test Plan: TBD I believe this is tested after cloning changes to fbsource and then testing there? Reviewers: shawndempsey, lefever, helenistic Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D53783607 Tasks: T163838856

Summary: See title. Test Plan: Tested later in this stack. Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D54045718

Summary: This diff adds the macOS `tooltip` property to the View component and assigns the provided text to the native NSView `toolTip` property. Test Plan: * Run Zeratul with Fabric enabled * Hover over buttons and check that a tool tip is displayed. https://pxl.cl/4nM7h Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54045717

Summary: This diff adds the `cursor` prop to the View component in Fabric. The supported cursor types are converted to the `facebook::react::Cursor` type, supported the same set of values in Paper for the same prop. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54098203 Tasks: T157889735

Summary: This diff implements the conversion of the react cursor value to a native `NSCursor` instance which is used by the native view to update the current cursor for the surface covered by the view bounds. Test Plan: * Run Zeratul with Fabric enabled * Move the cursor to the vertical splitter * Check that the cursor changes depending on the possible resize direction. https://pxl.cl/4p28W Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54098204 Tasks: T157889735

Summary: **Context** - TextInput focus prop was not wired up for Fabric **Change** - Focus the text input if `autofocus={true}` Test Plan: Tested in top of stack Reviewers: lefever, #rn-desktop Reviewed By: lefever Differential Revision: https://phabricator.intern.facebook.com/D54880714

Summary: This diff resets the string content assigned to the internal text view on recycle for the Text component. Because the text view update exits early when no text storage is defined on the state of the component, the previous text would still be set on the text view and displayed when using a recycled component view with undefined content assigned to it. By resetting the text view to an empty string on recycle, that edge case will display an empty text component when being assigned undefined content. Test Plan: - Run Cosmo Studio and open the UI Reference - Update an example to include a Text component with undefined content - Check that no text is being rendered | Before | After | |--| | https://pxl.cl/4zTW2 | {F1473555501} | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D55368305 Tasks: T170085811

Summary: **Context** - Configuring paste options was never implemented in our Fabric TextInput - Paper Implementation: https://www.internalfb.com/code/fbsource/[93ff90a797da]/third-party/microsoft-fork-of-react-native/react-native-macos/packages/react-native/Libraries/Text/TextInput/Multiline/RCTMultilineTextInputViewManager.m?lines=29-35 https://www.internalfb.com/code/fbsource/[563690cf01f1]/third-party/microsoft-fork-of-react-native/react-native-macos/packages/react-native/React/Base/RCTConvert.m?lines=1239 **Change** - Add c++ enum/prop - Add conversion - Add `pastedTypes` to Fabric component Test Plan: |Fabric|Paper| | https://pxl.cl/4LcPw | https://pxl.cl/4LcQL| Reviewers: lefever, #rn-desktop Differential Revision: https://phabricator.intern.facebook.com/D56558929 Tasks: T186086013

Saadnajmi · 2024-05-01T06:49:17Z

packages/react-native/React/Fabric/Mounting/ComponentViews/Text/RCTParagraphComponentView.mm

+@interface RCTParagraphComponentView () <NSTextViewDelegate>
+@end
+#endif // macOS]
+
 #if !TARGET_OS_OSX // [macOS]
 @interface RCTParagraphComponentView () <UIEditMenuInteractionDelegate>

 @property (nonatomic, nullable) UIEditMenuInteraction *editMenuInteraction API_AVAILABLE(ios(16.0));

 @end


Prefer the code blocks that are the same class to be in the same ifdef group. Something like:

Suggested change

@interface RCTParagraphComponentView () <NSTextViewDelegate>

@end

#endif // macOS]

#if !TARGET_OS_OSX // [macOS]

@interface RCTParagraphComponentView () <UIEditMenuInteractionDelegate>

@property (nonatomic, nullable) UIEditMenuInteraction *editMenuInteraction API_AVAILABLE(ios(16.0));

@end

#endif // macOS]

#if !TARGET_OS_OSX // [macOS]

@interface RCTParagraphComponentView () <UIEditMenuInteractionDelegate>

@property (nonatomic, nullable) UIEditMenuInteraction *editMenuInteraction API_AVAILABLE(ios(16.0));

@end

#else // [macOS

@interface RCTParagraphComponentView () <NSTextViewDelegate>

@end

#endif //macOS]

Saadnajmi · 2024-05-01T06:50:56Z

...erer/components/view/platform/macos/react/renderer/components/view/HostPlatformViewProps.cpp

+      validKeysDown(
+          CoreFeatures::enablePropIteratorSetter
+              ? sourceProps.validKeysDown
+              : convertRawProp(context, rawProps, "validKeysDown", sourceProps.validKeysDown, {})),
+      validKeysUp(
+          CoreFeatures::enablePropIteratorSetter
+              ? sourceProps.validKeysUp
+              : convertRawProp(context, rawProps, "validKeysUp", sourceProps.validKeysUp, {})),


uuuuuh any chance we can not use validKeysDown 😅

shwanton · 2024-05-01T19:02:31Z

...ents/textinput/iostextinput/react/renderer/components/iostextinput/TextInputEventEmitter.cpp

+  });
+}
+#endif // macOS]
+


These platform specific files were refactored in core. If we rebase on 0.74, I can move them to their correct folders.
facebook#41600
packages/react-native/ReactCommon/react/renderer/components/textinput/platform/macos/react/renderer/components/iostextinput/...

Saadnajmi · 2024-05-01T21:39:11Z

packages/react-native/React/Fabric/RCTConversions.h

+inline NSAccessibilityRole RCTUIAccessibilityRoleFromAccessibilityTraits(
+    facebook::react::AccessibilityTraits accessibilityTraits)
+{
+  using AccessibilityTraits = facebook::react::AccessibilityTraits;


We should probably align to the w3c-compliant roles that @FalseLobster updated our paper implementation to:
#2101

… for View component Summary: This diff fixes an invariant that wasn't valid anymore after having enabled `wantsUpdateLayer` statically for the View component in Fabric. `RCTUIView` in RCTUIKit enables `wantsUpdateLayer` only if the instance implements the `displayLayer:` method. Because the View component always wants to have `wantsUpdateLayer` enabled, the assumption that `displayLayer:` exists wasn't valid anymore. This diff only calls the `displayLayer:` method if the instance effectively responds to it. To avoid a perf hit on the check, we only test for it in the initialization and cache the result. Test Plan: * Run the Cosmo app ``` ~/fbsource/xplat/arfx/cosmo/mac/run.sh ``` | Before | After | |--| | {F1460101180} | {F1460101226} | Reviewers: shawndempsey, jorgecab, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D54090975 # Conflicts: # packages/react-native/React/Base/macOS/RCTUIKit.m

…uch handler is merged

shwanton · 2024-05-01T23:05:47Z

packages/react-native/Libraries/Text/TextInput/Multiline/RCTWrappedTextView.m

+    [self addSubview:_scrollView];
+
+    // a register for those notifications on the content view.
+  #if !TARGET_OS_OSX // [macOS]


Remove mobile code since this file is macOS only. @lenaic should we nest these files in a macos specific folder?

The file is macOS specific, it could be placed in its own macOS subfolder to make it more obvious.

Nick Lefever and others added 30 commits April 30, 2024 23:24

[fabric] Fix missing import

53f631f

Summary: See title Test Plan: Build RNTester Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D49924996

Nick Lefever and others added 16 commits April 30, 2024 23:24

[fabric] Fix missing method declaration of View component

7ecb2f6

Summary: See title. Test Plan: Sandcastle build D53695263 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53711568

[fabric] Fix namespace in View component header

1b5364e

Summary: See title. Test Plan: Sandcastle build D53695263 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53715088

[fabric] Add missing pragma marks in macOS view event emitter

8b630fb

Summary: See title. Test Plan: Sandcastle build. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53931156

[fabric] Clean up explicit namespace references in View component

b0e32bb

Summary: See title. Test Plan: Tested later in this stack. Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D54045718

Saadnajmi reviewed May 1, 2024

View reviewed changes

shwanton commented May 1, 2024

View reviewed changes

Saadnajmi reviewed May 1, 2024

View reviewed changes

Nick Lefever and others added 3 commits May 1, 2024 15:45

[fabric][touch-handler] Remove cancelTouchWithEvent till surface to…

a430e28

…uch handler is merged

[fabric] Refactor MacOS view props & emitters

cc052f5

shwanton force-pushed the shwanton/fabric-core-fixes branch from 909c136 to e722e95 Compare May 1, 2024 23:01

shwanton commented May 1, 2024

View reviewed changes

shwanton force-pushed the shwanton/fabric-core-fixes branch from e722e95 to b991978 Compare May 1, 2024 23:13

shwanton added 2 commits May 1, 2024 16:42

[fabric] Revert to Core Cursor implementation

fe4989a

[fabric] Fix warning & formatting

6b0fd9c

shwanton force-pushed the shwanton/fabric-core-fixes branch from b991978 to 6b0fd9c Compare May 1, 2024 23:42

Saadnajmi mentioned this pull request Nov 15, 2024

fix(fabric): cherry-pick some View related fixes #2281

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fabric] Fixes to core fabric components #2117

[fabric] Fixes to core fabric components #2117

shwanton commented May 1, 2024 •

edited

Loading

Saadnajmi May 1, 2024

Saadnajmi May 1, 2024

shwanton May 1, 2024

Saadnajmi May 1, 2024

shwanton May 1, 2024

lenaic May 2, 2024

[fabric] Fixes to core fabric components #2117

Are you sure you want to change the base?

[fabric] Fixes to core fabric components #2117

Conversation

shwanton commented May 1, 2024 • edited Loading

Summary:

Test Plan:

Fabric

View

Paper

View

Saadnajmi May 1, 2024

Choose a reason for hiding this comment

Saadnajmi May 1, 2024

Choose a reason for hiding this comment

shwanton May 1, 2024

Choose a reason for hiding this comment

Saadnajmi May 1, 2024

Choose a reason for hiding this comment

shwanton May 1, 2024

Choose a reason for hiding this comment

lenaic May 2, 2024

Choose a reason for hiding this comment

shwanton commented May 1, 2024 •

edited

Loading