-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix the layout calculation in sliver list where the scroll offset cor… #59888
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
Conversation
…rection accidently set the child with layout offset of zero
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| testWidgets( | ||
| 'SliverList can handle inaccurate scroll offset due to changes in children list', | ||
| (WidgetTester tester) async { | ||
| bool skip = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add one of those issue references in case it gets lost in history?
goderbauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| }, | ||
| ); | ||
|
|
||
| testWidgets( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add: "Regression test for " here?
|
|
||
| assert(childScrollOffset(firstChild) > -precisionErrorTolerance); | ||
|
|
||
| // If the scroll offset is at the zero, we should make sure we are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // If the scroll offset is at the zero, we should make sure we are | |
| // If the scroll offset is at zero, we should make sure we are |
| assert(childScrollOffset(firstChild) > -precisionErrorTolerance); | ||
|
|
||
| // If the scroll offset is at the zero, we should make sure we are | ||
| // actually at the beginning of the list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: end with a .
|
Google testing passed! |
1 similar comment
|
Google testing passed! |

…rection accidently set the child with layout offset of zero
Description
consider following example:
we have sliver list with items where each item has 10 paint extent. the screen is at item 10
-------start of screen----------
item 10 -- layout offset 100
item 11 -- layout offset 110
item 12 -- layout offset 120
item 13 -- layout offset 130
-------end of screen-----------
now, the sliver list children updated, and the each item has 20 paint extent. The RenderSliverList is smart to update the children scroll offset after the first child. So it become like this
-------start of screen----------
item 10 -- layout offset 100
item 11 -- layout offset 120
-------end of screen-----------
item 12 and 13 are not in the screen so it will be destroy.
Now, start scrolling back
-------start of screen----------
item 9 -- layout offset 80
item 11 -- layout offset 100
-------end of screen-----------
...keep scrolling
-------start of screen----------
item 5 -- layout offset 0
item 11 -- layout offset 20
-------end of screen-----------
oops, the sliver list think item 5 is the start of the list, so you cannot scroll any further.
Related Issues
Fixes #59819
Tests
I added the following tests:
see files
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.