search

flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
166.27k stars 27.51k forks source link

Consider option to defer painter updates during drag on scrollbar thumb #121574

Open Piinks opened 1 year ago

Piinks commented 1 year ago

Updated:

https://github.com/flutter/flutter/pull/121786 resolved the original issue in accordance with native behavior, but I am leaving this open to see if folks want the option for the first idea we had for this. If you would like the option to have the scrollbar update after the drag is released, add a :+1:

For more context see: https://github.com/flutter/flutter/pull/121786#discussion_r1129803106

Also related: https://github.com/flutter/flutter/issues/97676

OP:

Investigating https://github.com/flutter/samples/issues/1680

I suspect (if the scrollbar is being dragged) that the scroll metrics are being updated while drag is active on the scrollbar, leading to the jumpy behavior in the video. The list is not a fixed extent list, so as items are being added, the max scroll extent is changing in real time.

If so, we should defer updating the scrollbar with new metrics until after dragging ends to the user does not get flung around while holding onto the scrollbar.

HansMuller commented 1 year ago

I believe that's correct. I traced the code a little and the jumpiness corresponds to big updates to the ScrollMetrics minScrollExtents property which occur while the thumb is being dragged.

Piinks commented 1 year ago

I think the behavior is more pronounced since https://github.com/flutter/flutter/pull/111250

IIRC, before then, it was possible for the mouse pointer to be separated from the thumb. Since that change, they are kept together.

I think just deferring an update to the scrollbar painter until the drag gesture ends is the right approach here. @TahaTesser do you agree? If so I can send a PR right quick for this.

TahaTesser commented 1 year ago

My PR https://github.com/flutter/flutter/pull/111250 was reverted and then @xu-baolin took it over in https://github.com/flutter/flutter/pull/112434.

I think just deferring an update to the scrollbar painter until the drag gesture ends is the right approach here. @TahaTesser do you agree? If so I can send a PR right quick for this.

Sounds good

Piinks commented 1 year ago

Oh right, no wonder I had a hard time finding it via the git blame. Thanks for weighing in!

xu-baolin commented 1 year ago

image When scrolling from 1318.9 to 1337.0, the scroll metrics suddenly shrink, causing the wrong new position value to be calculated.

Maybe we should adjust the value in this case.

xu-baolin commented 1 year ago

Just came out with a solution and will check whether it suits all the metrics change cases.

Piinks commented 1 year ago

Thanks @xu-baolin!