[WIP] Opt-in for smooth pointer scrolling #115314
Conversation
flutter-dashboard
bot
added
a: text input
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| assert(delta != 0.0); | ||
|
|
||
| if (animatePointerScroll) { | ||
| double currentPosition = _outerPosition?.pixels ?? 0.0; |
There was a problem hiding this comment.
Another point for #102003, this kind of info should be more accessible.
| if (animatePointerScroll) { | ||
| double currentPosition = _outerPosition?.pixels ?? 0.0; | ||
| for (final _NestedScrollPosition innerPosition in _innerPositions) { | ||
| currentPosition += innerPosition.pixels; |
yjbanov
left a comment
yjbanov
left a comment
There was a problem hiding this comment.
From some manual Q/A:
- If scroll wheel event are high enough frequency, the scrolling velocity is reset to zero on every click.
- In the Gallery home page, the horizontally scrollable carousel of demos overscrolls the target item then scrolls back to it. This happens when using the arrow button to scroll. I don't think we support horizontal scrolling using the mouse wheel (?), so I didn't test it.
| _animating = true; | ||
| moveTo( | ||
| newTargetPixels, | ||
| duration: Duration(milliseconds: updatedDuration.round()), | ||
| curve: Curves.easeInOut, | ||
| ).whenComplete(() { | ||
| _animating = false; | ||
| _lastVelocity = 0.0; | ||
| }); |
There was a problem hiding this comment.
These lines can be mostly deduplicated with the ones in the if block, saving a few bytes in download size.
Is this a case where the scroll direction is changing? I recall you mentioned testing this with the Gallery app, did you opt-in for each individual demo? I think you said you saw it in action on the home page, if you set MaterialApp.scrollBehavior to opt in at that level, the demoes with their own MaterialApps override it. They each need to set it in the Gallery app since they are designed as stand alone apps within the gallery app. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| from: pixels, | ||
| to: newTargetPixels, | ||
| duration: Duration(milliseconds: newDuration.round()), | ||
| curve: Cubic(0.42, carriedVelocity, 0.58, 1.0), |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
@yjbanov now that flutter/engine#36346 has landed, can you try this again? Since we can now identify trackpads, I have filtered them out. :)
|
Closing for now since there has been no feedback. Will look at returning to this in a few weeks. |
|
I wonder if you can pick this back up @Piinks? |
|
Please look into picking this back up @Piinks 😢, as of now, I've been using my own fork of pub.dev/smooth_scroll_multiplatform, which breaks with NotificationListeners. |
4 participants
WIP
Fixes #32120
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.