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

feat: make pager controlled #693

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

feat: make pager controlled #693

wants to merge 16 commits into from

Conversation

krozniata
Copy link
Member

@krozniata krozniata commented Feb 13, 2023

Summary

This PR makes Pager View a controlled component, giving users the ability to control the current page with new page and animated properties.

It also changes all examples to use the controlled version of Pager View, as this will be the recommended way of using it.

This feature will only be available from version 7.x.x

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)

krozniata and others added 7 commits January 17, 2023 17:57
* 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
Co-authored-by: Piotr Trocki <[email protected]>
* 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
@krozniata krozniata requested review from troZee and okwasniewski and removed request for troZee February 13, 2023 23:45
@@ -24,6 +24,8 @@ export type OnPageScrollStateChangedEventData = Readonly<{
}>;

export interface NativeProps extends ViewProps {
page?: Int32;
Copy link
Member

Choose a reason for hiding this comment

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

Could you extract those props to a separate interface?

@krozniata krozniata marked this pull request as ready for review February 19, 2023 21:37
val view = PagerViewViewManagerImpl.getViewPager(host)
Assertions.assertNotNull(view)
Assertions.assertNotNull(pageIndex)
val childCount = view.adapter?.itemCount
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val childCount = view.adapter?.itemCount
val childCount = view.adapter?.itemCount ?: -1

Assertions.assertNotNull(pageIndex)
val childCount = view.adapter?.itemCount
val animated = PagerViewViewManagerImpl.animated
val canScroll = childCount != null && childCount > 0 && pageIndex >= 0 && pageIndex < childCount
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val canScroll = childCount != null && childCount > 0 && pageIndex >= 0 && pageIndex < childCount
val canScroll = childCount > 0 && pageIndex >= 0 && pageIndex < childCount

@troZee
Copy link
Member

troZee commented Mar 24, 2023

RN this PR is on hold, bc the sync mechanism in case of failure does not work

@troZee troZee added the on hold label Mar 24, 2023
Base automatically changed from next to master March 31, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants