feat(iOS): change Fabric implementation to UIScrollView #672

krozniata · 2022-12-19T23:42:03Z

Summary

This PR introduces PagerView Fabric implementation rewrite from UIPageView to UIScrollView.

It is based on React Native's UIScrollView implementation for Fabric.

Motivation

As described in #673, we wanted to have more control over native side of PagerView which could be achieved by switching from using UIPageViewController to UIScrollView.

This change makes PagerView easier to maintain, because some solutions and workarounds used in previous implementation were adding more complexity and were causing some additional bugs.

Switching to UIScrollView allowed to reduce amount of code and complexity, while maintaining same functionality.
In future it will also be easier to add new features or e.g. custom animations to PagerView.

Test Plan

Every example should work as expected (only Fabric architecture)

Compatibility

OS	Implemented
iOS	✅
Android	❌

Checklist

I have tested this on a device and a simulator

troZee · 2022-12-20T08:18:38Z

common/cpp/react/renderer/components/RNCViewPager/RNCViewPagerShadowNode.cpp

@@ -0,0 +1,35 @@
+#include "RNCViewPagerShadowNode.h"


Suggested change

#include "RNCViewPagerShadowNode.h"

// https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp

#include "RNCViewPagerShadowNode.h"

any way to generate that with codegen or suggest the feature request?

Right now, the codegen generates those files inside a build folder. We can suggest a feature request and request a comment from other developers, on what they think about it.

troZee · 2022-12-20T08:19:04Z

common/cpp/react/renderer/components/RNCViewPager/RNCViewPagerShadowNode.h

@@ -0,0 +1,31 @@
+#pragma once


https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h

troZee · 2022-12-20T08:20:44Z

src/PagerView.tsx

+          this.props.style,
+          {
+            flexDirection:
+              this.props.orientation === 'vertical' ? 'column' : 'row',


Breaking change

src/utils.tsx

troZee · 2023-01-04T09:23:26Z

fabricexample/src/NestPagerView.tsx

@@ -82,7 +82,7 @@ export function NestPagerView() {
        </View>
        <View
          key="3"
-          style={{ backgroundColor: BGCOLOR[3] }}
+          style={[styles.page, { backgroundColor: BGCOLOR[3] }]}


We have to highlight it as a breaking change

okwasniewski · 2023-01-05T20:06:26Z

src/utils.tsx

-          // Override styles so that each page will fill the parent.
-          style: [element.props.style, StyleSheet.absoluteFill],
-        })}
+      <View


I think we need to pass there child.key so the outer view has proper key and react can manage it's views efficiently. I remember that's what @satya164 mentioned on one of our internal meetings.

ios/Fabric/RNCPagerViewComponentView.mm

* feat(iOS): change Fabric implementation to UIScrollView (#672) * feat(iOS): change Fabric implementation to UIScrollView * fix: fix offset values in vertical orientation * feat: add initialPage props support * feat: add RTL language support * feat: add pageMargin prop support * fix: fix typescript error * feat: remove React.cloneElement * feat(ios): add getPageOffset method * fix: fix styles in old example * fix: behavior on page remove * chore: add GH actions (#680) Co-authored-by: Piotr Trocki <[email protected]> * feat(iOS): rewrite old arch to use UIScrollView (#681) * feat: rewrite old arch to use UIScrollView * feat: update example styles * fix: sending event on scrollViewDidEndDecelerating * feat: properly calculate width using orientation * fix: change way of disabing scroll * feat: rename to RNCPagerView * fix: removing last page * fix: remove unused properties, set animated * chore: update release script * Release 7.0.0-rc.0 * wip * wip * fabric example * nit: comment * make init consistent * fix: multiple updates of frame and contentSize * feat: add button to quickly switch layout direction * sync with master (#756) Co-authored-by: Piotr Trocki <[email protected]> * chore: upgrade RN (paper example) * chore: upgrade rn & fix issues (fabric example) * chore: bump versions in package.json * chore: fix eslint issue * chore: exclude example from tsc * feat(next): remove fabric example * chore: update README * fix broken overdrag on notch (#787) Co-authored-by: Kuba Juszczyk <[email protected]> * feat(iOS): Add a `useLegacy` flag to switch between the old/new iOS implementation (#783) * wip: unsuspicious changes * wip: unsuspicious changes v2 * FABRIC NEW IMPL -> OLD IMPL * wip: bring back useLegacy on the RN side * wip: bring back duplicate types to fix codegen issues * wip: remove #705 related code for now * wip: old/new impl division first draft * wip: old/new impl division continued * wip: old/new impl v3 * wip: add a `LEGACY_` prefix to all legacy implementation-related symbols * wip: fix styles for new implementation on Fabric * wip: move old/new impl into separate folders * wip: fix old impl fabric symbol names * wip: xcode changes * wip: clean up & unify the naming convention * wip: fix linter issues * wip: fix styles for new implementation on Paper * wip: make Fabric example run on another port by default to make it possible to run both examples in parallel * wip: implement an abstraction over native commands invocations to reduce branching * refactor: remove the unnecessary value for boolean props * fix: bump react-native-safe-area-context to a Fabric-enabled version * feat: bring back & adjust the `bootstrap-fabric` script * feat: adjust the home screen title depending on the used architecture * chore: update example/Podfile.lock * chore: update an Xcode project file after building * ci: make next branch events trigger ios/android build workflows * chore: remove commented-out code related to #705 for now * chore: add legacy implementation explanation comment * wip: Android fixes * fix: unnecessary comma in MainActivity.java * feat: readme makeover * chore: bump react-native-screens & react-native-gesture-handler in example * refactor(android): extract module name to shared variable, add comment for context * chore: remove unnecessary yarn.lock deps * chore(ios): bring back removed build flags * chore(ios): remove unnecessary concurrentRootEnabled method According to React Native Upgrade Helper, this method is to be removed when updating to RN 0.72: https://react-native-community.github.io/upgrade-helper/?from=0.71.14&to=0.72.0#RnDiffApp-ios-RnDiffApp-AppDelegate.mm * fix(android): adjust incorrect param type on Fabric * chore: remove unnecessary tsconfig.json comment * chore(ios): bring back (currently unused) code related to #712 and #705 * Release 7.0.0-rc.1 * fix(ios): fix freezing when navigating to same index (#804) * Release 7.0.0-rc.2 * fix iOS issue * Change Legacy basic example into the next basic example * revert documentatation --------- Co-authored-by: Kacper Rożniata <[email protected]> Co-authored-by: Piotr Trocki <[email protected]> Co-authored-by: Oskar Kwaśniewski <[email protected]> Co-authored-by: Nishan <[email protected]> Co-authored-by: Kuba Juszczyk <[email protected]> Co-authored-by: Kuba Juszczyk <[email protected]> Co-authored-by: Igor Bejnarowicz <[email protected]>

feat(iOS): change Fabric implementation to UIScrollView

c90e6e1

krozniata requested review from troZee and okwasniewski December 19, 2022 23:42

troZee requested changes Dec 20, 2022

View reviewed changes

troZee mentioned this pull request Dec 20, 2022

☂️ Pager View Improvements #673

Open

krozniata added 5 commits December 20, 2022 21:13

fix: fix offset values in vertical orientation

9e84e5f

feat: add initialPage props support

6113f38

feat: add RTL language support

ff0afab

feat: add pageMargin prop support

5053860

fix: fix typescript error

f458b5d

krozniata changed the base branch from master to refurb December 21, 2022 14:10

krozniata marked this pull request as ready for review December 21, 2022 15:14

satya164 reviewed Jan 1, 2023

View reviewed changes

src/utils.tsx Outdated Show resolved Hide resolved

feat: remove React.cloneElement

b20447f

troZee reviewed Jan 4, 2023

View reviewed changes

okwasniewski reviewed Jan 5, 2023

View reviewed changes

ios/Fabric/RNCPagerViewComponentView.mm Outdated Show resolved Hide resolved

krozniata added 3 commits January 11, 2023 00:26

feat(ios): add getPageOffset method

d725159

fix: fix styles in old example

86020ee

fix: behavior on page remove

5b2238f

troZee merged commit e6bfc6e into next Jan 17, 2023

troZee deleted the feat/fabric-rework branch January 17, 2023 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iOS): change Fabric implementation to UIScrollView #672

feat(iOS): change Fabric implementation to UIScrollView #672

krozniata commented Dec 19, 2022 •

edited

Loading

troZee Dec 20, 2022

thymikee Dec 20, 2022

troZee Dec 20, 2022

troZee Dec 20, 2022

troZee Dec 20, 2022

troZee Jan 4, 2023

okwasniewski Jan 5, 2023

	#include "RNCViewPagerShadowNode.h"
	// https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp
	#include "RNCViewPagerShadowNode.h"

feat(iOS): change Fabric implementation to UIScrollView #672

feat(iOS): change Fabric implementation to UIScrollView #672

Conversation

krozniata commented Dec 19, 2022 • edited Loading

Summary

Motivation

Test Plan

Compatibility

Checklist

troZee Dec 20, 2022

Choose a reason for hiding this comment

thymikee Dec 20, 2022

Choose a reason for hiding this comment

troZee Dec 20, 2022

Choose a reason for hiding this comment

troZee Dec 20, 2022

Choose a reason for hiding this comment

troZee Dec 20, 2022

Choose a reason for hiding this comment

troZee Jan 4, 2023

Choose a reason for hiding this comment

okwasniewski Jan 5, 2023

Choose a reason for hiding this comment

krozniata commented Dec 19, 2022 •

edited

Loading