Fix ViewPager being stuck in some scenarios #15376
Conversation
In some scenarios, the view pager gets stuck in and it's not possible to update the pages. I did some digging and found the following. In native `ViewPager`, the `onAttachedToWindow` method gets called on start, which sets a variable called `mFirstLayout` to `true`. Then `onLayout` method gets called, which sets `mFirstLayout` to `false`, since first layout has occurred now. But then `onAttachedToWindow` method gets called again for some reason which resets `mFirstLayout` to `true`. This makes the `ViewPager` stuck since `mFirstLayout` variable never updates again, and to be able to animate the pager, the native component expects it to be `false`. I also discovered that @rauliyohmc arrived at the same conclusions here, which describes it in more detail - expo/ex-navigation#415 (comment) This PR is a hacky attempt to prevent `onAttachedToWindow` being called again after first layout since I am unsure about the correct fix. Fixes #13463 cc @astreet
facebook-github-bot
added
CLA Signed
facebook-github-bot
added
the
Import Started
|
@mmmulani has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
This doesn't seem like a safe change. Why is this only a problem in React Native if it all has to do with the native android ViewPager? |
It's a problem with React Native's ViewPager wrapper. Not the native Android component. |
|
I'm not working on RN anymore so I've got limited time I can give to help out, but I saw this go by internally and swallowing This issue needs more digging to figure out why it only affects RN and not standard Android (e.g. all the explanation above only references things in ViewPager) -- there must be something weird or unexpected RN is doing that's manifesting itself this way. |
|
I also attempted fixing this in another way by having another layout pass. It was inspired by a similar fix in the Switch component. |
|
@satya164 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
|
Closing in favor of #14867 which looks more reasonable. |
In some scenarios, the view pager gets stuck in and it's not possible to update the pages. I did some digging and found the following.
In native
ViewPager, theonAttachedToWindowmethod gets called on start, which sets a variable calledmFirstLayouttotrue. ThenonLayoutmethod gets called, which setsmFirstLayouttofalse, since first layout has occurred now. But thenonAttachedToWindowmethod gets called again for some reason which resetsmFirstLayouttotrue.This makes the
ViewPagerstuck sincemFirstLayoutvariable never updates again, and to be able to animate the pager, the native component expects it to befalse.I also discovered that @rauliyohmc arrived at the same conclusions here, which describes it in more detail -
expo/ex-navigation#415 (comment)
This PR is a hacky attempt to prevent
onAttachedToWindowbeing called again after first layout since I am unsure about the correct fix.Fixes #13463
cc @astreet