Repository navigation
SliverFillRemaining accounts for child size when hasScrollBody is false - #35810
Conversation
|
Will incorporate sample code & visuals to better illustrate the use cases of this feature. ref: #35753 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| } | ||
| } | ||
|
|
||
| assert(extent.isFinite); |
There was a problem hiding this comment.
Let's add a test covering this, and ideally have a message here that explains what can be done if anything (e.g. wrapping the child in something else or changing the child to some other type?).
There was a problem hiding this comment.
(post-offline chat) I haven't been able to trigger this assert in a test, as it is currently covered by the child's layout method. I will leave this assertion in place thought to guard against future changes that may result in triggering this assert.
I have added a message to explain why this assertion would fail.
| /// defer to the child's size rather than overriding it. | ||
| bool hasScrollBody; | ||
|
|
||
| /// Indicates whether the child should stretch to fill the overscroll area |
There was a problem hiding this comment.
nit: the first paragraph of a doc comment should be a single sentence summary.
…se (flutter#35810) Fixes the hasScrollBody flag not accounting for child size. Adds the ability to specify over-scroll behavior.
Description
SliverFillRemainingstipulates that it will size its child to fill the remaining space in the viewport in the main axis.Its actual default behavior has been to fill the entire viewport in the main axis, which is desirable behavior for applications such as NestedScrollView, but is not the expected behavior for most cases.
#33627 added the flag
hasScrollBody, which allows developers to specify if the child is a scrollable, like in a NestedScrollView, or not. However, this implementation did not take into account cases when the child's size should be used instead of the calculated extent of the sliver.This PR modifies the original implementation of #33627 to include this consideration.
When
hasScrollBodyis false, the child's size will be compared to that of the calculated extent to choose the appropriate size.Also in this PR is an additional flag -
fillOverscroll. This give developers even more power over the layout of this widget, as they can specify what the behavior should be for scroll physics that utilize an overscroll feature, such as with the default scroll behavior on iOS platforms.fillOverscrolldefaults to false, meaning the child widget will maintain its size during overscroll. When set to true, it will cause the child's size to stretch and fill the overscroll area.An executable study app is available here that illustrates use cases and behaviors: gist
Related Issues
Fixes #35758
Related to #35753, will follow-up with documentation and diagrams.
Related to #17444 (#33627)
Tests
SliverFillRemaining - hasScrollBody
iOS/similar scroll physics when hasScrollBody: false & fillOverscroll: true
Android/other scroll physics when hasScrollBody: false, ignores fillOverscroll: true
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
Does your PR require Flutter developers to manually update their apps to accommodate your change?