feat(iOS): Add a useLegacy flag to switch between the old/new iOS implementation
#783
Conversation
This file contains 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
…ssible to run both examples in parallel
troZee
requested changes
Oct 26, 2023
example/ios/PagerViewExample.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
Show resolved
Hide resolved
okwasniewski
reviewed
Nov 6, 2023
android/src/fabric/java/com/reactnativepagerview/LEGACY_PagerViewViewManager.kt
Outdated
Show resolved
Hide resolved
troZee
reviewed
Nov 11, 2023
troZee
requested changes
Nov 11, 2023
tsconfig.json
Outdated
| @@ -25,5 +25,5 @@ | |||
| "strict": true, | |||
| "target": "esnext" | |||
| }, | |||
| "exclude": ["example"] | |||
| "exclude": ["example"] // why?? | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Sorry about this one, it was meant as a reminder for me to investigate this exclude further, but it slipped my mind and ended up in the PR 😁 I removed it, but to answer your comment — thanks for clarifying, I understand where the value is coming from, but I was wondering about why we're not type checking the example folder, time savings? (If you remove this exclude there are some 200+ new TS errors emitted.)
troZee
requested changes
Nov 11, 2023
android/src/fabric/java/com/reactnativepagerview/PagerViewViewManager.kt
Outdated
Show resolved
Hide resolved
troZee
requested changes
Nov 11, 2023
troZee
requested changes
Nov 12, 2023
6 tasks
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
troZee
added a commit
that referenced
this pull request
Mar 31, 2024
* 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]>
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.
Continues #763
To have a more complete understanding of the issues @troZee ran into while working on #763 and of the scope of the task, I started the implementation from scratch, basing my work on the two (old/new architecture)
UIPageViewController->UIScrollViewre-writes by @krozniata and @okwasniewski directly, and ended up using the previous PR only as a reference. Because of that, to make things more readable, I'm leaving #763 as is and I'm creating a new PR that continues the work started there.Summary
useLegacyflag to switch between the used implementation on iOS (old/legacy -UIPageViewController, new -UIScrollView).Test Plan
Compatibility
(*Just stub code to make it work, the feature doesn't affect Android per se.)
Checklist
README.md