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.

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

Migrate the ViewPager module from React Native core to the community repo #1

Merged
merged 75 commits into from Feb 11, 2019

Conversation

ferrannp
Copy link
Contributor

@ferrannp ferrannp commented Feb 9, 2019

Done

  • Keep git history from RN core
  • Changed module name to RNCViewPager
  • Fix bug on the example: This from RNTester should be like here

ToDo after merging this

  • Add eslint + prettier
  • Add CI
  • Deprecate and remove from RN core repo
  • Refactor JS code a bit?
  • Fix warnings and deprecations from Android
  • Modernize Android code (even migrate to Kotlin if we want to)

Example

viewpager

Martin Konicek and others added 30 commits October 6, 2015 20:01
Reviewed By: @foghina

Differential Revision: D2513014

fb-gh-sync-id: d9bb668d76939ad85b657233c8b7beac9b244fab
Reviewed By: astreet

Differential Revision: D2590921

fb-gh-sync-id: cf870c96f772c06e1a8b69014ebd906978ea8c00
Differential Revision: D2610700

fb-gh-sync-id: b59dfc581d9ca8d29203b5915fb743f3270989ab
Differential Revision: D2620741

fb-gh-sync-id: 9f35e94efb25c689661bf2775a7d0624b3c586f4
Summary: In some cases it's desirable to set the page in the ViewPager without animating it -- we have this in ScrollView with `scrollWithoutAnimationTo`. This PR adds `setPageWithoutAnimation` on ViewPager.

cc ide kmagiera
Closes facebook/react-native#3621

Reviewed By: svcscm

Differential Revision: D2652056

Pulled By: mkonicek

fb-gh-sync-id: 6f1f38558c41ffdd863c0ebb2f046c75b5c0392c
Reviewed By: sahrens

Differential Revision: D2663780

fb-gh-sync-id: 94a7e2265f6f869a2bdf1576dd8ea76b4c5f0b34
Summary: public

RCTUIManager is a public module with several useful methods, however, unlike most such modules, it does not have a JS wrapper that would allow it to be required directly.

Besides making it more cumbersome to use, this also makes it impossible to modify the UIManager API, or smooth over differences between platforms in the JS layer without breaking all of the call sites.

This diff adds a simple JS wrapper file for the UIManager module to make it easier to work with.

Reviewed By: tadeuzagallo

Differential Revision: D2700348

fb-gh-sync-id: dd9030eface100b1baf756da11bae355dc0f266f
Summary:
Fixed few typos in `./Examples` and `./Libraries` folders.
Closes facebook/react-native#4788

Reviewed By: svcscm

Differential Revision: D2759918

Pulled By: androidtrunkagent

fb-gh-sync-id: d692b5c7f561822353e522f9d4dfde7e60b491cf
Summary:
Fixes #5195
Closes facebook/react-native#5236

Reviewed By: svcscm

Differential Revision: D2819197

Pulled By: androidtrunkagent

fb-gh-sync-id: cea451802c659512f64c1e90905647b8fbe4490b
Summary:
I have an issue when combining `PullToRefreshViewAndroid` and `ViewPagerAndroid`.
`ViewPagerAndroid` will not able to scroll that gesture handler is being taken by `PullToRefreshViewAndroid`
One solution is to disable `PullToRefreshViewAndroid` if `ViewPagerAndroid` is scrolling (i.e. not idle).
[Reference solution here](http://stackoverflow.com/a/29946734/2590265)

So here need to expose the `onPageScrollStateChanged` event.
Some code referenced from  DrawerLayoutAndroid, especially the `VIEWPAGER_PAGE_SCROLL_STATES` array.
Please feel free give me comments.
Thanks.
Closes facebook/react-native#5026

Reviewed By: svcscm

Differential Revision: D2830623

Pulled By: andreicoman11

fb-gh-sync-id: c2a6920c6f4c7daab0115f13864db83b93b31abf
Reviewed By: foghina

Differential Revision: D2953917

fb-gh-sync-id: effd09849a5504c9eb7c684a510e616fdcfcdf6e
shipit-source-id: effd09849a5504c9eb7c684a510e616fdcfcdf6e
Reviewed By: sahrens

Differential Revision: D3145366

fb-gh-sync-id: 6412d51a698b2c932c915e405e4bbc35e96060dc
fbshipit-source-id: 6412d51a698b2c932c915e405e4bbc35e96060dc
Summary:Since the React 0.14 split of modules, the findNodeHandle feature is part of the
renderer and not the generic React API.

This just greps for React.findNodeHandle and replace them with ReactNative.findNodeHandle. I fixed up the imports manually.

I also found two callers each of ReactNative.createClass and React.render with the exception of downstream and examples will fix them separately.

I'll need to find more things like `var { PropTypes } = ReactNative;` separately. I think this is a good start though.

Reviewed By: vjeux

Differential Revision: D3149356

fb-gh-sync-id: 50ed60bc67270b16f561d4c641f2f19e85724d3b
fbshipit-source-id: 50ed60bc67270b16f561d4c641f2f19e85724d3b
Summary: Similar to ScrollView, adds ability to set scrollEnabled={false}, which prevents dragging. Paging is still possible by updating initialPage.

Reviewed By: AaaChiuuu

Differential Revision: D3209743

fb-gh-sync-id: ce4140323a03f2257a9bb310c7285418b01abae7
fbshipit-source-id: ce4140323a03f2257a9bb310c7285418b01abae7
Summary:
This is a nice feature to have.

I've tested this by copying and renaming the ViewPager java and javascript files from the react-native repo and including them in a project. Whats the best way to test this directly from the repo?
Closes facebook/react-native#5968

Differential Revision: D3240651

Pulled By: mkonicek

fbshipit-source-id: 5f1d157216df4f3314915496188a92aec1b85e91
Summary: Closes facebook/react-native#7456

Differential Revision: D3299061

fbshipit-source-id: 417995191d675e2ae625926df93412c660d1931a
Summary: This removes `node_modules/react` from the list of directories that are used for haste module resolutions. Modules required from React are now imported with `require('react/lib/…')`.

Reviewed By: astreet

Differential Revision: D3509863

fbshipit-source-id: 32cd34e2b8496f0a6676dbe6bb1eacc18124c01e
…sistent with iOS

Summary:
So `PanReponder.onPanResponderRelease/onPanResponderTerminate` receive a `gestureState` object containing a `onPanResponderTerminate.vx/vy` property. On Android and iOS, they appear to be orders of magnitude different, which appear to be due to the different scale of timestamps that are used when generating touch events.

This pull request fixes the timestamps to be milliseconds on both platforms (since I assume iOS is the more authoritative one, and is the one that `react-native-viewpager`'s vx thresholds written written to compare against.)

As far as I can tell, the RN code doesn't use the `vx/vy` properties, so they should be okay. And looks like the RN code only cares about relative values of `startTimestamp/currentTimestamp/previousTimestamp` though, so should be fine too. it's quite possible there will be downstream android breakage with this change, particularly for those who are already compensating for the RN discrepancy.
Closes facebook/react-native#8199

Differential Revision: D3528215

Pulled By: dmmiller

fbshipit-source-id: cbd25bb7e7bb87fa77b661a057643a6ea97bc3f1
Summary:
So `PanReponder.onPanResponderRelease/onPanResponderTerminate` receive a `gestureState` object containing a `onPanResponderTerminate.vx/vy` property. On Android and iOS, they appear to be orders of magnitude different, which appear to be due to the different scale of timestamps that are used when generating touch events.

This pull request fixes the timestamps to be milliseconds on both platforms (since I assume iOS is the more authoritative one, and is the one that `react-native-viewpager`'s vx thresholds written written to compare against.)

As far as I can tell, the RN code doesn't use the `vx/vy` properties, so they should be okay. And looks like the RN code only cares about relative values of `startTimestamp/currentTimestamp/previousTimestamp` though, so should be fine too. it's quite possible there will be downstream android breakage with this change, particularly for those who are already compensating for the RN discrepancy.
Closes facebook/react-native#8199

Differential Revision: D3528215

Pulled By: davidaurelio

fbshipit-source-id: d81732e50a5ece2168e8347309d8d52a0db42951
Summary:
Two things in this diff:

1. Implemented `getItemPosition` in our adapter; the default implementation always returns POSITION_UNCHANGED, which is incorrect, and causes `destroyItem` to never (sometimes?) be called.
2. Fix `destroyItem`: this never worked. `destroyItem` is always called by the ViewPager after a `notifyDataSetChanged()`, so after `removeViewAt`, which removes the view from `mViews`, causing `destroyItem` to throw `IndexOutOfBoundsException` when it tries to get the view. Since our item objects are just views, use that instead of checking `mViews`.

Reviewed By: ahmedre

Differential Revision: D3555427

fbshipit-source-id: 900c2696162d07f507e850517d483b943ce39a35
Summary: This is pure cleanup so that we can make sure that all events are living in the same time space (currently nano seconds).

Reviewed By: foghina

Differential Revision: D3593884

fbshipit-source-id: 71b084362008f1c93c21880630acf11f5c058355
Summary: Add a batch addition operation for ViewPager.

Differential Revision: D3597840

fbshipit-source-id: 1c9c42e03da2492444298220e75f547b6567b4e5
Reviewed By: cpojer

Differential Revision: D3619143

fbshipit-source-id: e14e81468d467437ee3d79c34c34b7780a46ca1c
Reviewed By: bestander

Differential Revision: D3683952

fbshipit-source-id: 9484d0b0e86859e8edaca0da1aa13a667f200905
Summary:
Explain the **motivation** for making this change. What existing problem does the pull request solve?

Under certain scenario, `PageScrollEvent.offset` was initialized to `NaN`, which cause `folly::toJson` failed, and FIX #9750

<https://github.com/android/platform_frameworks_base/blob/e71ecb2c4df15f727f51a0e1b65459f071853e35/core/java/com/android/internal/widget/ViewPager.java#L1689>

![image](https://cloud.githubusercontent.com/assets/104052/18266416/2a01f882-744d-11e6-86c4-3a2de3a1ca25.png)

**Test plan (required)**

<http://stackoverflow.com/questions/39327429/reactnative-viewpagerandroid-rcteventemitter>
Closes facebook/react-native#9755

Differential Revision: D3841674

Pulled By: andreicoman11

fbshipit-source-id: d4cd9f4b2f61daad9005a098161ad7f75555345d
Reviewed By: sebmarkbage

Differential Revision: D4024375

fbshipit-source-id: cd2226a3580a7a4ff319d6a93b67b68f2942eb00
Reviewed By: sebmarkbage

Differential Revision: D4025448

fbshipit-source-id: 4e9b6ee002a86f638fc57a4bbeb45bf35fabf74c
Reviewed By: sebmarkbage

Differential Revision: D4025461

fbshipit-source-id: bdb35bd8948bf96e791e2c0d567ad6bd42806188
TheSavior and others added 21 commits May 11, 2018 13:52
Reviewed By: yungsters

Differential Revision: D7974340

fbshipit-source-id: 5fe457a8a9be4bd360fc3af9acb5c1136b2be0d7
Reviewed By: yfeldblum

Differential Revision: D8073585

fbshipit-source-id: 12322aebc09b89d5af9cc257b16c1bc0fbb066c1
Reviewed By: shergin

Differential Revision: D7832079

fbshipit-source-id: 263a2f8ff96ab6e14b91395644710b4d5f36dc50
Reviewed By: achen1

Differential Revision: D8216184

fbshipit-source-id: 3b188804e2dad2b112f566da49a939eb4338713d
Summary:
Bump Prettier to use version 1.13.4
All code changes are caused by running Prettier and should only affect files that have an `format` header.
All other changes caused by yarn.

Reviewed By: ryanmce

Differential Revision: D8251255

fbshipit-source-id: 0b4445c35f1269d72730f2000002a27c1bc35914
…iles

Reviewed By: sahrens

Differential Revision: D8345921

fbshipit-source-id: 187048ad4c1b361f0b99b993052bdcaf47a266db
Summary:
ag -L --ignore __snapshots__ 'flow strict$|noflow|generated|The controller you requested could not be found.' | ag '\.js$' | xargs ag -l 'flow' | sort > ~/temp
  cat ~/temp | xargs ag -L 'flow strict' | xargs sed -i '' 's/flow$/flow strict/'
  cat ~/temp | xargs ag -L 'flow strict$' | xargs sed -i '' 's/flow strict-local$/flow strict/'
  until flow; do flow check --json | jq -r '.errors[].message[0].path' | sort | uniq | xargs hg revert; done

allow_many_files
The controller you requested could not be found.
The controller you requested could not be found.

Reviewed By: yungsters

Differential Revision: D9003523

fbshipit-source-id: d0c9fbfe3c32e65d57819fa040d06cd6ebbd59cc
Summary:
Flow doesn't check .android.js files yet anyway.

I'm going to be adding suppressions in a followup diff. It would be nice to not have >1k suppressions saying that we can't do certain things in `flow strict` when we don't even typecheck with regular `flow` just yet

I ran these commands to produce this diff:
`find . -name '*.android.js' -exec sed -i 's/flow strict-local/flow/g' {} +`
`find . -name '*.android.js' -exec sed -i 's/flow strict/flow/g' {} +`

Followed https://unix.stackexchange.com/questions/112023/how-can-i-replace-a-string-in-a-files to do it.

The controller you requested could not be found.

Reviewed By: TheSavior

Differential Revision: D9143783

fbshipit-source-id: e9af4fe695ebdba4db4083de1697cc248d48eb0d
Summary:
.android.js files may be checked (when the next version of flow is released) by using `flow start --flowconfig-name .flowconfig.android` and `flow status --flowconfig-name .flowconfig.android`

This diff adds suppressions to the errors that are in .android.js files, which flow does not check right now.

When site is `react_native_fb` or `react_native_android_fb`, error will be suppressed when checking with .flowconfig.android
When site is `react_native_fb` or `react_native_ios_fb`, error will be suppressed when checking with .flowconfig.

You can use `react_native_fb` when it should be suppressed for both.

The controller you requested could not be found.

Reviewed By: TheSavior

Differential Revision: D9122178

fbshipit-source-id: 0ec9d3cae3d887f58645e6585b2a3f6c3889b13e
Summary: This diff moves the prop-type definitions for View out into it's own file. We will be able to do this with a bunch of the prop-type definitions and then move them out into a deprecated npm package.

Reviewed By: yungsters

Differential Revision: D9444394

fbshipit-source-id: 4fd0a78533211b598ba2da4eb5015ffcc20bb675
Reviewed By: mzlee

Differential Revision: D9553765

fbshipit-source-id: cb65081668ea2726f24d2c9c02661e859cc7a994
Summary: This change drops the year from the copyright headers and the LICENSE file.

Reviewed By: yungsters

Differential Revision: D9727774

fbshipit-source-id: df4fc1e4390733fe774b1a160dd41b4a3d83302a
Summary: Replaced each view manager access with a getViewManager() function call. This will later be used to lazily load view manager classes by allowing java to avoid sending the entire list of view managers to JS.

Reviewed By: QueryConnectionException

Differential Revision: D9695788

fbshipit-source-id: 949858aa2f0b0b00b68e260461ba8f1d085cf07f
Summary:
related #21342

This is a first PR for this repo.
So, if there are any problem, please tell me 🙇

TODO
* delete props types
* apply read only interface

CheckList
 - [x] `yarn prettier`
 - [x] `yarn flow-check-android`
Pull Request resolved: facebook/react-native#21347

Differential Revision: D10095503

Pulled By: TheSavior

fbshipit-source-id: fd1adb5edf19234ae8ae9f3fe732b03a521eb82b
Summary:
I removed `var` in RNTester.

- [x] npm run prettier
- [x] npm run flow-check-ios
- [x] npm run flow-check-android

[GENERAL] [ENHANCEMENT] [RNTester] - remove `var`
Pull Request resolved: facebook/react-native#22014

Differential Revision: D12843150

Pulled By: TheSavior

fbshipit-source-id: 593adf141164cffe0ddc2db756721df26e38b4f5
Summary:
Related to #22100

Turn Flow strict mode on for Libraries/Components/ViewPager/ViewPagerAndroid.android.js

- [x] npm run prettier
- [x] npm run flow-check-ios
- [x] npm run flow-check-android

[GENERAL] [ENHANCEMENT] [Libraries/Components/ViewPager/ViewPagerAndroid.android.js] - Flow strict mode
Pull Request resolved: facebook/react-native#22134

Differential Revision: D12930719

Pulled By: TheSavior

fbshipit-source-id: 9519f31b7af27f0497e42fd51f18c19be3692823
…rs and Native Modules

Summary: Using strings to refer to Native Modules and View Managers in ReactPackages are prone to error. The compiler replaces the constants with the actual strings anyway, so using constants gives us  better type safety

Reviewed By: achen1

Differential Revision: D12843649

fbshipit-source-id: 7a7c6c854c8689193f40df92dc8426a3b37f82c8
Summary: Catch `IllegalArgumentException: ReactViewPager.onTouchEvent` and ignore it (but log something) to work around this known bug in the Android SDK per Baseflow/PhotoView#31 (comment). Note that `onInterceptTouchEvent()` is already doing the same thing.

Reviewed By: yungsters

Differential Revision: D13550425

fbshipit-source-id: 9fa3a7a0ca0cd95aafa3bcae6a59e0d1e690b564
Summary:
This PR adds flow types for the RNTester examples, and updates all of the RNTester examples to match the flow type consistently.

Previously, there was a mix of static class definitions and whether or not pages exported examples or a component. Now we will always export the same way, enforced by flow types

Note: I also fixed most of the $FlowFixMe in changed components
Pull Request resolved: facebook/react-native#22829

Reviewed By: cpojer

Differential Revision: D13563191

Pulled By: rickhanlonii

fbshipit-source-id: b697e3346a863d1b130881592b0522a96c202b63
Summary:
[Android][Changed] - All the imports connected to `requireNativeComponent` in `ViewPager` was moved to  a separate file.
Issue in focus: #22990
Pull Request resolved: facebook/react-native#22995

Differential Revision: D13760459

Pulled By: cpojer

fbshipit-source-id: fca1633ce933ea4909ef81d7bbe8123d654e24fb
js/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid work, thank you!

Let's discuss next steps up to publishing on Discord.

@ferrannp ferrannp merged commit d0a3116 into callstack:master Feb 11, 2019
@ferrannp ferrannp deleted the init-with-history branch February 11, 2019 13:22
@cpojer cpojer mentioned this pull request Feb 14, 2019
64 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet