-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: expose shrinkWrap of CustomScrollView #71
base: main
Are you sure you want to change the base?
Conversation
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, left a few small adjustments regarding documentation.
Any chance we can add some tests to ensure the property gets proxied into CustomScrollView
and ensure false
is the default?
@@ -76,6 +77,12 @@ class InfiniteList extends StatelessWidget { | |||
/// {@endtemplate} | |||
final bool reverse; | |||
|
|||
/// {@template shrinkWrap} |
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: If the @template
is not being used elsewhere in a @macro
we should remove the @template
.
/// should be determined by the contents being viewed. | ||
/// {@endtemplate} |
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: consider referencing the property that the value is proxying to.
/// should be determined by the contents being viewed. | |
/// {@endtemplate} | |
/// should be determined by the contents being viewed. | |
/// | |
/// See also: | |
/// | |
/// * [CustomScrollView.shrinkWrap], for more details about this flag. |
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 you waiting for my response?
I not sure how to "add some tests to ensure the property gets proxied into CustomScrollView and ensure false is the default?"
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.
Hi @limcheekin sorry for the late reply, you can try having a look at a recent PR: #76, to get inspired on how to test it.
Status
READY
Description
I exposed the
shrinkWrap
option of theCustomScrollView
widget to support the top alignment of list items. Please see section "3. Aligning Chat Messages to the Top" of the article at https://medium.com/@ximya/tips-and-tricks-for-implementing-a-successful-chat-ui-in-flutter-190cd81bdc64.If you think the changes are unnecessary, please let me know the alternative solution.
Type of Change