-
Notifications
You must be signed in to change notification settings - Fork 29.1k
Add SliverGrid.list convenience constructor #173925
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
Add SliverGrid.list constructor that accepts List<Widget> children for cleaner API when working with predefined widget lists. Fixes: flutter#173018
Add test coverage for SliverGrid.list including basic functionality and empty children edge case.
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.
Code Review
This pull request introduces a SliverGrid.list
convenience constructor, which simplifies creating a SliverGrid
from a list of widgets. The implementation is clean and follows existing patterns in the framework. The accompanying tests cover the basic functionality and edge cases well.
My main feedback is to enhance the documentation for the new constructor to fully align with the Flutter Style Guide by adding a code example and documenting all parameters. This will improve its usability for other developers.
FYI @loic-sharma since you opened the issue asking for this (#173018). |
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.
This looks good to me - thanks for the wonderful contribution!
Thank you! Excited to keep contributing 🙌 |
/// | ||
/// |
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.
Are these two newlines intentional?
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.
Thanks, that was unintentional. I’ve fixed it 👍
@ahmeddhus Looks like there are some formatting issues, can you run |
Fixed 👍🏾 |
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. Thanks for the PR
autosubmit label was removed for flutter/flutter/173925, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
This PR adds a new
SliverGrid.list
convenience constructor that accepts aList<Widget> children
parameter, providing a cleaner API compared to manually creatingSliverChildListDelegate
.What was added
The constructor uses
SliverChildListDelegate
internally and follows the same pattern as existing convenience constructors likeSliverGrid.count
andSliverGrid.extent
. This addresses the use case where developers want to create sliver grids with predefined widget lists without the verbosity of manually constructing the delegate.Before/After comparison
Before:
After:
Implementation details
SliverGrid.list
constructor inpackages/flutter/lib/src/widgets/sliver.dart
List<Widget> children
and all necessary delegate configuration parametersSliverChildListDelegate
internally for consistent behaviorpackages/flutter/test/widgets/slivers_test.dart
Issue addressed
Fixes: #173018
Pre-launch Checklist
///
).