Add scroll-physics logging in scroll_overlay #16
Conversation
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Sharing on all 3 for consistency: I was so very excited to see #13 that I forgot myself and made an error, actually 2. I should have sought a second reviewer on the PR as is our review guidelines. And I should have ensured tests were included, or that test exemption had been granted. 😅
Can you add a test for this PR? It ensures we do not end up removing your change accidentally. :)
|
Test added! PTAL. |
This has been very convenient for experimenting with different scroll physics in my recent investigations, providing a way to do so without creating rebase conflicts with other changes to this `ListView.builder` call. This is especially useful in combination with the next commit, adding a piece of debug logging.
This has been extremely convenient in my recent scrolling investigations. Because the ScrollPhysics protocol is a bit subtle to hook into correctly, and requires some boilerplate, inserting debug logging for it is a bit more complicated than just writing down a `debugPrint` call of one's choice. So it benefits from having this permanent infrastructure in the app.
And move the customization point to a widget field so it can be tested.
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Also LGTM! Thank you for all of these improvements. I think I managed the merge conflict this time. :)
|
Thank you! It looks like the merge on this one needs a fixup: #17. (Which I guess reminds us of why it'd be great to have these new tests run automatically on PRs, and/or to have the autosubmit infra running on this repo.) |
2 participants
This is the last part of the further enhancements after #13 that I've been using for my investigations the past couple of weeks. It consists of two related commits:
[scroll_overlay] Add a place to customize the scroll physics
This has been very convenient for experimenting with different
scroll physics in my recent investigations, providing a way to
do so without creating rebase conflicts with other changes to
this
ListView.buildercall.This is especially useful in combination with the next commit,
adding a piece of debug logging.
[scroll_overlay] Add debug print on createBallisticSimulation
This has been extremely convenient in my recent scrolling
investigations.
Because the ScrollPhysics protocol is a bit subtle to hook into
correctly, and requires some boilerplate, inserting debug logging
for it is a bit more complicated than just writing down a
debugPrintcall of one's choice. So it benefits from havingthis permanent infrastructure in the app.
The second commit leaves the debug logging it adds enabled by default. That's not what one would do in the framework or in a normal app; but I think it makes sense here, given that this is a test app and the logging is right at the core of what the app is made to test.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.