Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP - Fabric ScrollContentView 0_73 #10

Draft
wants to merge 4 commits into
base: 0.73-stable
Choose a base branch
from
Draft

WIP - Fabric ScrollContentView 0_73 #10

wants to merge 4 commits into from

Commits on Apr 28, 2024

  1. [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
    @shwanton
    shwanton committed Apr 28, 2024
    Configuration menu
    Copy the full SHA
    6a4abd8 View commit details
    Browse the repository at this point in the history

  2. [fabric] ScrollContentView should be mounted as document view of Scro… 

    …llView

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
    @shwanton
    shwanton committed Apr 28, 2024
    Configuration menu
    Copy the full SHA
    b824fa0 View commit details
    Browse the repository at this point in the history

  3. [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
    @shwanton
    Nick Lefever authored and shwanton committed Apr 28, 2024
    Configuration menu
    Copy the full SHA
    15f29d6 View commit details
    Browse the repository at this point in the history

  4. [fabric] Add view flipping propagation to ScrollView/ScrollContentVie… 

    …w 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
    @shwanton
    Nick Lefever authored and shwanton committed Apr 28, 2024
    Configuration menu
    Copy the full SHA
    e9aff24 View commit details
    Browse the repository at this point in the history