devtools icon indicating copy to clipboard operation
devtools copied to clipboard

Scrolling for treetable needs position clamping to prevent overshooting

Open carolynqu opened this issue 3 years ago • 2 comments
trafficstars

https://github.com/flutter/devtools/pull/4226/files#diff-7f63e0b64be4d80b632e5eefe1cac93308dff718f53fc1fe6e1db774ffd4e7a9R198

Current Problem: When autoscrolling for the treetable, if the index is near the maxScrollingExtent, it will overshoot and therefore create a bouncing animation.

Solution: By clamping the position to be between 0.0 - maxScrollExtent, this will make sure the scroll will stay in the appropriate range. However, maxScrollExtent is not updating after parents are expanded and before scrolling happens. Therefore, if an index is greater than the previous maxScrollExtent even if it is in range after parents are expanded, it still believes that the index is not in range and therefore not scroll to the correct position.

carolynqu avatar Jul 20 '22 00:07 carolynqu

@Piinks this sounds similar to the scrolling issue with the DevTools flame chart that we tried to debug: https://github.com/flutter/devtools/issues/2012

kenzieschmoll avatar Jul 20 '22 15:07 kenzieschmoll

If it is like the flame chart issue, IIRC, trying to jump to the end of the list at that point caused issues because the max scroll extent had not been updated yet. This was expected because the length of the list had changed, but did not require rebuilding, which would update the extent. Since scrolling widgets ideally build lazily, if you add something to the end that is not in view (or outside of the cache extent), Flutter won't build it in order to be efficient.

Did #2012 ever try using the ScrollMetricsNotification instead of a ScrollNotification? We added that feature some time after discussing that issue. I have also seen folks work around this by triggering an empty scroll event to force an update. Something along the lines of:

scrollController!.position.didUpdateScrollPositionBy(0);

Piinks avatar Jul 20 '22 16:07 Piinks