ajsmth / react-native-pager

controllable pager component for react native
MIT License
334 stars 28 forks source link

Android scrollable area isn't calculated correctly #9

Closed gringocl closed 5 years ago

gringocl commented 5 years ago

First thanks for putting this pager out there!

It seems that recent changes to reanimated have changed how the scrollable are is being calculated on android. Our use case is using this library to horizontally page through smaller visible cards. On android the scrollable / swipeable are is only as large as the Page width, active indexed element, ie the green date below.

Screenshot 2019-10-03 16 53 34

We have solved this by adding page width and height to the Page element. I can push up a PR with these changes, if this is how this should be solved. I am unaware of another solution.

ajsmth commented 5 years ago

hey there -- thanks for this. i'm wondering if you have a small reproducible example i could test out. thanks!

i suspect we could add an additional property to configure the PanGestureHandler container as an alternative solution

gringocl commented 5 years ago

I do not have an example currently but I will put one together.

ajsmth commented 5 years ago

okay great, thank you! just want to make sure we're on the same page with this

gringocl commented 5 years ago

Related in our current implementation. Is this issue on reanimated. I saw a recent commit that does some dimensional setting.

gringocl commented 5 years ago

https://snack.expo.io/@gringocl/android-pager

ajsmth commented 5 years ago

thanks for this -- i've come up with a solution that I think will work. will post updates on this soon, just want to test it out a little bit first. unfortunately your PR breaks some of the existing functionality for pages, so i'll be closing it in favour of something else

ajsmth commented 5 years ago

also i was curious about this use case in general - i would think that these indicators would be served better as pagination components responding to a pager component with your views for each date - no?

gringocl commented 5 years ago

I think you are correct! I’ll have to think about that for a bit!

On Sun, Oct 6, 2019 at 8:43 PM andy notifications@github.com wrote:

also i was curious about this use case in general - i would think that these indicators would be served better as pagination components responding to a pager component with your views for each date - no?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CrowdLinker/react-native-pager/issues/9?email_source=notifications&email_token=AAYFA4TMDFMGNCXQEDDXW3LQNJZZ7A5CNFSM4I5Q6MB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAOXD6Y#issuecomment-538800635, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYFA4WETWSYCBB76AIUXYDQNJZZ7ANCNFSM4I5Q6MBQ .

ajsmth commented 5 years ago

okay cool, regardless i think this issue is important to resolve with the upcoming fix so thanks!

ajsmth commented 5 years ago

@gringocl i've pushed a new release (0.1.15) with the fixes, the panhandler dimensions will expand to the width of the container which should capture the pan gestures without any additional config. let me know if this helps - cheers!

ajsmth commented 5 years ago

after some testing, i realize some of the changes have broken flex a little bit, i'm adding an addition containerStyle configuration that should give us the flexibility to define the pan handler area with some sensible defaults. again, your config should work out of the box but the prop will be there if you need any extra configurations

gringocl commented 5 years ago

@ajsmth This solution worked great! Thanks for getting this out so quick! I'm going to close this as its resolved!