-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Allow for rubber band overscrolling on iOS #616
Conversation
Untested Resolution for follow up by @maxsalven on bvaughn#566
Thanks a ton @toddtarsi! I'm going to tweak it slightly to also cover |
:D Glad I can help. Thanks again! |
9.4.0 was just released with this feature. Thank you for your contribution! I listed you in the 9.4.0 release notes. |
Unfortunately this seems to have caused a fairly serious regression that I didn't notice until earlier tonight. You can see it here, for example. Scroll to the bottom of the this._scrollingContainer.scrollHeight - this._scrollingContainer.clientHeight // 1367
totalRowsHeight - height + scrollbarSize // 1365 For now, I'm going to revert this change and we can fix it forward if we find a way around this issue. |
(I realize that my "repro" steps may no longer work once I've released the fix, but you could repro them locally by running this branch locally) |
Ah, smart. Sorry about causing that @bvaughn. We use list pretty
aggressively where I'm at so i missed that case.
I will try and take a look later, using that example to test.
Perhaps we can reduce the footprint of the constraint to skip rerenders if
we are outside the constraints on one axis and the other axis is unchanged
or outside constraints on both axes.
On Mar 31, 2017 12:49 AM, "Brian Vaughn" <[email protected]> wrote:
Unfortunately this seems to have caused a fairly serious regression that I
didn't notice until earlier tonight. You can see it here
<https://bvaughn.github.io/react-virtualized/#/components/Grid>, for
example. Scroll to the bottom of the Grid and then scroll right. Grid won't
update when you scroll to the right- because the eventScrollTop is slightly
greater than scrollTop. The case I'm seeing is:
this._scrollingContainer.scrollHeight -
this._scrollingContainer.clientHeight // 1367
totalRowsHeight - height + scrollbarSize // 1365
For now, I'm going to revert this change and we can fix it forward if we
find a way around this issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#616 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEfUsQ1KKX3Cj8tTNFoxSbwSp5eunhVNks5rrJPogaJpZM4MezkG>
.
|
No worries! I appreciate the work you've done to help on this. 😊
…On Mar 31, 2017 5:50 AM, "toddtarsi" ***@***.***> wrote:
Ah, smart. Sorry about causing that @bvaughn. We use list pretty
aggressively where I'm at so i missed that case.
I will try and take a look later, using that example to test.
Perhaps we can reduce the footprint of the constraint to skip rerenders if
we are outside the constraints on one axis and the other axis is unchanged
or outside constraints on both axes.
On Mar 31, 2017 12:49 AM, "Brian Vaughn" ***@***.***> wrote:
Unfortunately this seems to have caused a fairly serious regression that I
didn't notice until earlier tonight. You can see it here
<https://bvaughn.github.io/react-virtualized/#/components/Grid>, for
example. Scroll to the bottom of the Grid and then scroll right. Grid won't
update when you scroll to the right- because the eventScrollTop is slightly
greater than scrollTop. The case I'm seeing is:
this._scrollingContainer.scrollHeight -
this._scrollingContainer.clientHeight // 1367
totalRowsHeight - height + scrollbarSize // 1365
For now, I'm going to revert this change and we can fix it forward if we
find a way around this issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#616#
issuecomment-290622341>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/
AEfUsQ1KKX3Cj8tTNFoxSbwSp5eunhVNks5rrJPogaJpZM4MezkG>
.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#616 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABznZGQppTsyfKTvyrSVdQ9k1nvIkFrks5rrPaTgaJpZM4MezkG>
.
|
Untested resolution for follow up by @maxsalven on #566
Needs testing, and if no one is able to, I may be able to this weekend.