[DRAFT] feat(fabric): merge the rest of the fabric core fixes branch #2647
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
Saadnajmi
commented
Aug 21, 2025
Comment on lines
65
to
71
| HostPlatformViewTraitsInitializer::formsStackingContext(viewProps) | ||
| #if TARGET_OS_OSX // [macOS | ||
| || viewProps.focusable | ||
| || viewProps.enableFocusRing | ||
| || viewProps.macOSViewEvents[MacOSViewEvents::Offset::MouseEnter] | ||
| || viewProps.macOSViewEvents[MacOSViewEvents::Offset::MouseLeave] | ||
| #endif // macOS] | ||
| ; |
There was a problem hiding this comment.
Feels like these checks should move into HostPlatformViewTraitsInitializer
Saadnajmi
commented
Aug 27, 2025
Comment on lines
+38
to
+45
| validKeysDown( | ||
| CoreFeatures::enablePropIteratorSetter | ||
| ? sourceProps.validKeysDown | ||
| : convertRawProp(context, rawProps, "validKeysDown", sourceProps.validKeysDown, {})), | ||
| validKeysUp( | ||
| CoreFeatures::enablePropIteratorSetter | ||
| ? sourceProps.validKeysUp | ||
| : convertRawProp(context, rawProps, "validKeysUp", sourceProps.validKeysUp, {})), |
There was a problem hiding this comment.
Theses should be keyDownEvents and keyUpEvents
Saadnajmi
commented
Aug 27, 2025
| @@ -8,6 +8,7 @@ | |||
| #include "SurfaceTelemetry.h" | |||
|
|
|||
| #include <algorithm> | |||
| #include <iterator> | |||
Saadnajmi
commented
Aug 27, 2025
Comment on lines
+129
to
+130
| OpenHand, | ||
| OperationNotAllowed, |
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
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: 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
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/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: This diff propagates the View `enableFocusRing` property to the backing textfield/textview to correctly set up the NSTextField/NSScrollView with the latest provided property value for single and multiline TextInput components. Test Plan: - Run Zeratul with Fabric enabled - Focus the composer in a message thread - The blue focus ring should not show up around the composer | Before | After | |--| | https://pxl.cl/3XHP8 | {F1169320851} | Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D52005858 Tasks: T167538822
Summary: Pressing the escape key down in a TextInput on macOS will be captured on the native side as a cancel event. This diff adds support for sending the "Escape" key as a key pressed event and wires the delegate method of the backing text input to call the key press event handler with the correct payload. Test Plan: - Run Zeratul with Fabric enabled - Add a key press handler on the textfield logging the received data (e.g. on MDSTextInput) - Focus the textfield - Press the escape key - The key press handler should be called and print that the "Escape" key was pressed. https://pxl.cl/3XHR2 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D52005857 Tasks: T167538822
Summary: This diff implements the scrollViewDidScroll delegate method call on the mutli line text input (`RCTWrappedTextView`). The text metrics are updated with the right values read directly from the emitting scroll view so that the TextInput would submit the right payload for scrolling multi line TextInput components at all times. Test Plan: - Run Zeratul with Fabric enabled - Log the onScroll callback data for TextInput used by the composer (e.g. MDSTextInput) - Type a message spanning over multiple lines in the composer until a scroll bar is visible - Scroll the composer contents - The logging shows the text metrics provided as payload for the onScroll handler. https://pxl.cl/3XHRl Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D52005859 Tasks: T167538822
Summary: When text completions or special character symbols are displayed ahead of time, the content is present in the native text view attributed string. If a state update is applied, the cursor position will not be read as being at the end of the string while typing, which will shift the cursor position after the update. This diff disables native text view content updates while temporary context is displayed to fix the wrong cursor position updates. Test Plan: Run Zeratul with Fabric and type text in the message composer until macOS renders a text completion/suggestion. Keep typing and see if the cursor stays at the end of the string. | Before | After | |--| | https://pxl.cl/49PW4 | https://pxl.cl/49PWh | Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D52787094 Tasks: T174286406
Summary: This diff adds the macOS-only property to the TextInput component called `submitKeyEvents` which allows the user to provide key combinations that will trigger a submit action. This is being used to implement message sending on hitting the return key. Test Plan: Tested later in this stack Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Subscribers: taskcreeper Differential Revision: https://phabricator.intern.facebook.com/D52860413
Summary: This diff implements the key down event processing to check if any of the provided submit key combinations were entered in the TextInput. If a match is found, the submit callback is triggered. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D52860412
Summary: This diff adds the macOS-only `clearTextOnSubmit` property for the TextInput component. When set to `true`, the TextInput will clear its content after a submit was triggered through the configured `submitKeyEvents` key combinations. Test Plan: - Run Zeratul with Fabric enabled and enter a message in the composer. - Submit the message by pressing the `Return` key. - Check that `Shift + Return` still works without submitting. https://pxl.cl/4bw3D Reviewers: shawndempsey, chpurrer, #rn-desktop Reviewed By: shawndempsey, chpurrer Differential Revision: https://phabricator.intern.facebook.com/D52860414
Summary: This diff adds support for the secure text entry mode of the single line TextInput component. The backing text field is switched for an extension of the NSSecureTextField on macOS. This was implemented on the single line text input implementation in Paper. This logic is now combined with the multi line text input on the RCTTextInputComponentView on Fabric. The secure text field uses a macro define to re-include the text field source files with minor changes, this to avoid code duplication. To support the inclusion magic happening here, the `import` statement of the text field header had to be converted to an `include` so that the macro-toggling-magic of the secure text field would work. Test Plan: - Run Zeratul with Fabric enabled and use the auth screen of Messenger to test the secure text entry with the password field. https://pxl.cl/4c2CD Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D52924481
…ng view
Summary: This diff adds the copy of the a11y attributes when switching the TextInput to the secure text entry which triggers the switch to a NSSecureTextField and runs the copy of the NSTextField attributes to the new backing text field.
Test Plan:
- Run Zeratul with Fabric enabled and inspect the Messenger password field on the auth screen using the Accessability Inspector to check that the attributes are passed down to the native text field.
{F1332886925}
Reviewers: shawndempsey, #rn-desktop
Reviewed By: shawndempsey
Differential Revision: https://phabricator.intern.facebook.com/D52924480
Summary: This diff updates `ViewProps` with a reference to the `HostPlatformViewProps`, allowing to move macOS specific properties in a separate class without having to change `ViewProps`. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241732 Tasks: T157889406, T154618477
Summary: This diff updates the `ViewEventEmitter` with a reference to the `HostPlatformViewEventEmitter`, allowing to move macOS specific event emitters in a separate file without having to change `ViewEventEmitter`. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241735 Tasks: T157889406, T154618477
Summary: Moving the macOS specific props from ViewProps to the macOS specific HostPlatformViewProps extension. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241734 Tasks: T157889406, T154618477
Summary: This diff adds the `validKeyDown` and `validKeyUp` props to the View component to provide a list of key event matchers used to filter out key event that have to be handled manually by the JS side. These properties were added through the HostPlatformViewProps class to keep the macOS specific properties in a separate file from the iOS ViewProps. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241733 Tasks: T157889406, T154618477
Summary: This diff adds the event emitters that will be used by the keyboard handler of the View component. The `KeyEvent` struct provides the same fields that were previously submitted by Paper. An equals operator between `KeyEvent` and `HandledKey` allows to check if a key event matches the criteria defined by the handled key description. The event emitters were added using the HostPlatformEventEmitter so that the macOS specific changes would be kept separate from the iOS version. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53241730 Tasks: T157889406, T154618477
Summary: Add support for `draggedTypes` to the View component, enabling the configuration of a native view as a drop target. Test Plan: * Run Zeratul with Fabric enabled * Drag a file over the messages view and check that the cursor is changing indicating a drop target https://pxl.cl/4ldkW Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Subscribers: taskcreeper Differential Revision: https://phabricator.intern.facebook.com/D53674739
Summary: Add the drag and drop event emitters to View with the payload conversion matching the Paper API for dragEnter/dragLeave/drop events. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53674738
Summary: Add support for drag and drop to the `ViewComponent` for files and images. The implementation converts the dropped pasteboard items the same way as in Paper. The `DataTransferItem` resulting from the pasteboard conversion of the dragged items is used to build the JS payload for the dragEnter/dragLeave/drop events. Test Plan: * Run Zeratul with Fabric enabled * Drag and drop an image on the messages view * Send the attachment https://pxl.cl/4ldB7 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53674742
Summary: Add support for drag and drop to the TextInput component. This re-uses the drag and drop handlers implemented on the View component to support dropping images/files on TextInput views directly. Test Plan: * Run Zeratul with Fabric enabled * Drag and drop an image on the message composer * Send the attachment https://pxl.cl/4ldBX Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53674740
Summary: Update draggedType prop to use enums and limit input to supported options. Test Plan: * Run Zeratul with Fabric enabled * Drag an image over the messages view and send the image https://pxl.cl/4ljBt Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53694154 Tasks: T178960804
Summary: This diff refactors the data transfer item payload conversion logic to share it with `TextInputEventEmitter` paste event, allowing to implement the onPaste event emitters building the same payload as generated by the drag and drop implementation. Test Plan: Tested later in this stack. Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53694921
Summary: Implement the file/image pasteboard pasting event for the `TextInput` component in Fabric. The payload for the event is the same as for the drag and drop events. Test Plan: * Run Zeratul with Fabric enabled * Copy an image/file on the pasteboard * Focus the message composer * Paste the image/file and send the message https://pxl.cl/4lk92 Reviewers: shawndempsey, #rn-desktop Reviewed By: shawndempsey Differential Revision: https://phabricator.intern.facebook.com/D53694922
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
… 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
Saadnajmi
force-pushed
the
fabric/more
branch
from
bfd9584 to
2ee90c1
Compare
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.
This is part of a series of PRs where we are cherry-picking fixes from #2117 to update our Fabric implementation on macOS.
Summary:
Merge the rest of the branch at once.
Test Plan:
TODO: Manual pass over all of the code.
CI should pass.