Fix #1159: CollectionView scrollTop/scrollLeft #1260
Conversation
|
Has this PR slipped through the cracks? We would love to be able to switch to the latest version, but without this fix we either have to patch or stick on 9.18.5 |
Props scrollTop/scrollLeft are only applied to CollectionView when this.state.scrollPositionChangeReason is REQUESTED, but getDerivedStateFromProps was not setting scrollPositionChangeReason. This meant that these props were not being applied to the view.
dawnmist
force-pushed
the
bugfix/1159-collection-scrollTop-scrollLeft
branch
from
17ea8f3
to
f40bc0d
Compare
To check that scrollPositionChangeReason is being set properly when setting scrollLeft or scrollTop parameters on Collection - if it is not set, the view does not actually apply the given scrollLeft or scrollTop parameters.
|
As far as I know, no-one has ever reviewed it - I've certainly never received any form of feedback ever since the initial labelling of the bug 2 years ago. The only reason I can think of for the issue was that at the time of creating the pull request I hadn't been able to get puppeteer working, hence couldn't run the tests properly locally, and the count of checkboxes on the pull requests list page therefore wasn't "correct". I've updated the pull request to current master, and updated the tests that were incorrectly passing on master (because they were not checking the It is my hope that after 18 months waiting for a review (or almost 2 years waiting for any action on the bug report) that the test changes will finally trigger this into being looked at. It feels really awful to try to contribute and simply be ignored entirely despite a critical bug having been introduced that even with a fully worked example doesn't get any action and a 2 line pull request fixing it doesn't get any feedback as to why it is being left untouched/unreviewed. We're still stuck on react-virtualized 9.18.5 because the implementation of |
|
We have also been stuck on 9.18.5 for a while now because of this issue, and we want to be prepared for concurrent mode when it comes along. @wuweiweiwu Can this be looked into? It's a very blocking issue and seems to be going unnoticed. |
|
Will release and update! |
|
released! https://www.npmjs.com/package/react-virtualized/v/9.22.2 hope the update process is smooth |
Props scrollTop/scrollLeft are only applied when this.state.scrollPositionChangeReason is REQUESTED, but getDerivedStateFromProps was not setting scrollPositionChangeReason. This meant that these props were not being applied to the view.
Thanks for contributing to react-virtualized!
Before submitting a pull request, please complete the following checklist:
npm test) all passnpm run prettier).npm run typecheck).closes #1159