Repository navigation
Fix TreeSliver first node clipping during expand/collapse animation - #188626
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates RenderTreeSliver to ensure that the parent node's extent is always added during animation, even when its index is 0, preventing children from painting over it. It also adds regression tests to verify that animating nodes clip their children below their trailing edge. The feedback suggests a more idiomatic and robust way to retrieve the RenderTreeSliver in the tests by using tester.renderObject instead of searching through tester.allRenderObjects.
| final RenderTreeSliver renderTree = tester.allRenderObjects | ||
| .whereType<RenderTreeSliver>() | ||
| .first; |
There was a problem hiding this comment.
Instead of searching through tester.allRenderObjects, you can use tester.renderObject to directly retrieve the RenderTreeSliver associated with the TreeSliver widget. This is more idiomatic and robust.
final RenderTreeSliver renderTree = tester.renderObject<RenderTreeSliver>(find.byType(TreeSliver<String>));There was a problem hiding this comment.
These are good suggestions. :)
| final RenderTreeSliver renderTree = tester.allRenderObjects | ||
| .whereType<RenderTreeSliver>() | ||
| .first; |
There was a problem hiding this comment.
Instead of searching through tester.allRenderObjects, you can use tester.renderObject to directly retrieve the RenderTreeSliver associated with the TreeSliver widget. This is more idiomatic and robust.
final RenderTreeSliver renderTree = tester.renderObject<RenderTreeSliver>(find.byType(TreeSliver<String>));
Piinks
left a comment
There was a problem hiding this comment.
Thanks for this! Apologies on the review delay, we are finally catching up. 🙏
| // parentIndex is always a real animating node (the unclipped first | ||
| // segment is already painted), so its extent is added even when it is | ||
| // index 0. Otherwise the clip starts at the parent's leading edge and its | ||
| // children paint over it. See https://github.com/flutter/flutter/issues/188305. |
There was a problem hiding this comment.
We can remove this reference to the issue, it will be out of date in the future since this change will close it!
| // Regression test for https://github.com/flutter/flutter/issues/188305. | ||
| // While a node expands, its children must be clipped beneath the node (at the | ||
| // node's trailing edge) - including the first node, which used to clip at its | ||
| // leading edge and let its children paint over it. |
There was a problem hiding this comment.
Nit: This can go inside of the group body.
|
Hi @Piinks , documentation has been updated :) |
e35f57f to
d9fb52f
Compare
…12325) Manual roll requested by stuartmorgan@google.com flutter/flutter@c83f80b...2a230d1 2026-07-29 zerouali.bardai.omar@gmail.com Clarify CustomScrollView use cases (flutter/flutter#189692) 2026-07-29 brunocorona.alcantar@gmail.com Fix TreeSliver first node clipping during expand/collapse animation (flutter/flutter#188626) 2026-07-29 chris@bracken.jp iOS: Migrate VSyncClient tests to Swift Testing (flutter/flutter#190054) 2026-07-29 32538273+ValentinVignal@users.noreply.github.com Remove no shuffle from flutter_driver extension_test.dart and mock flutter.process channel (flutter/flutter#187559) 2026-07-29 52160996+FMorschel@users.noreply.github.com Adds missing await on `instantiateImageCodecFromBuffer` (flutter/flutter#188910) 2026-07-29 1961493+harryterkelsen@users.noreply.github.com [web] Provide Content-Length header for CanvasKit files in flutter test (flutter/flutter#190155) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Description
While a
TreeSlivernode expands/collapses, its children should be clipped beneath the node (at the node's trailing edge). This worked for every node except the first: node index 0 clipped at its leading edge, so its children painted over the node during the animation.The cause was a special case in
RenderTreeSliver.paintthat conflated "no parent" with "parent is index 0":This branch only runs for the clipped segments after the first (the first segment is painted unclipped beforehand), so
parentIndexis always a real animating parent whose extent must be added — including index 0. The fix always adds the parent's extent.Demo
Video showing the bug and the fix:
Screen.Recording.2026-06-26.at.7.20.49.a.m.mov
Related Issues
Fixes #188305
Tests
I added the following tests:
Regression tests in
sliver_tree_test.dartthat pump the toggle animation to its midpoint and assert the clip-rect top edge:rowExtent), not0.0.rowExtent * 2), confirming no regression for other nodes.Pre-launch Checklist
///).