Repository navigation
Fix gap flipping when dragging variable height reorderable items - #194478
Merged
auto-submit[bot] merged 5 commits intoNov 5, 2026
Merged
auto-submit[bot] merged 5 commits into
auto-submit[bot] merged 5 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates SliverReorderableListState to skip the dragged item when determining the insertion index, preventing its original slot from competing with other items, and simplifies the index calculation logic. It also adds a test to verify gap stability when dragging variable-height items. The review feedback recommends removing the package:flutter/material.dart import and the MaterialApp wrapper from the test to maintain proper layering within the widgets package.
github-actions
Bot
added
the
framework
Contributor
There was a problem hiding this comment.
Hi @suryaKommana2662, thank you for picking this up!
Report below was generated by gemini and manually verified by me:
The framework change is correct and I'd like to land it. I wrote my own regression test for #194436 (below) and it fails on master with exactly the symptom in the issue (Expected: <482.0> Actual: <376.0> move 0) and passes with your change. I also checked the mirrored case (dragging a taller item down over a shorter one) and it's stable too. Removing the own-slot branch from #151026 is safe now that _dragEnd handles _insertIndex == item.index + 1 (#172380), and the two regression tests that #151026 added for #150843 ('SliverReorderableList - properly animates the drop at starting position' and its reverse: true twin) still pass with this change. 👍
There are two problems with the test, though, and they share a root cause, so I'd ask for a change there before we proceed. Details are in the inline comments; the short version:
1. The test isn't exercising the fix
The itemBuilder returns a bare SizedBox with no ReorderableDragStartListener, so a reorder never starts. The long-press + moveBy(0, -80) simply scrolls the CustomScrollView by 80 px — every item moves up by 80 (that's where the "~296.0" in the comment comes from: 376 − 80). The lessThan(376 − 50) assertion is therefore satisfied by scrolling alone, and the test passes against master without your fix:
# with master's reorderable_list.dart + this PR's test
00:00 +1: All tests passed!
You can see it yourself by logging if onReorderStart is hit.
2. CI failure: Windows framework_tests_widgets_leak_tracking (not a flake)
The hand-rolled WidgetsApp → Overlay → OverlayEntry scaffolding leaks the OverlayEntry (it's never disposed), which is what the leak-tracking shard reports:
notDisposed:
ValueNotifier<_OverlayEntryWidgetState?>: test: SliverReorderableList keeps gap stable when dragging variable height items
OverlayEntry: test: SliverReorderableList keeps gap stable when dragging variable height items
Reproducible locally with flutter test --dart-define=LEAK_TRACKING=true test/widgets/reorderable_list_test.dart.
Suggested replacement
Both issues go away if you use TestWidgetsApp (like the rest of this file) and wrap items in a ReorderableDragStartListener. This is what I used to verify the fix; feel free to take it as-is or fix your own test — it's leak-free under LEAK_TRACKING=true:
testWidgets('SliverReorderableList gap does not flip when dragging a taller item over a shorter one', (
WidgetTester tester,
) async {
// Regression test for https://github.com/flutter/flutter/issues/194436.
const itemHeights = <double>[68, 85, 126, 97, 54, 106, 110, 119];
final items = List<int>.generate(itemHeights.length, (int index) => index);
await tester.pumpWidget(
TestWidgetsApp(
home: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
return CustomScrollView(
slivers: <Widget>[
SliverReorderableList(
itemCount: items.length,
itemBuilder: (BuildContext context, int index) {
final int item = items[index];
return SizedBox(
key: ValueKey<int>(item),
height: itemHeights[item],
child: ReorderableDragStartListener(index: index, child: Text('item $item')),
);
},
onReorderItem: (int fromIndex, int toIndex) {
setState(() {
items.insert(toIndex, items.removeAt(fromIndex));
});
},
),
],
);
},
),
),
);
final Finder item4 = find.text('item 4');
final Finder item5 = find.text('item 5');
expect(tester.getTopLeft(item4).dy, 376.0);
expect(tester.getTopLeft(item5).dy, 430.0);
// Pick up item 5 (106px) and move it up into the top half of item 4 (54px).
final TestGesture drag = await tester.startGesture(tester.getCenter(item5));
await tester.pump(kPressTimeout);
await drag.moveBy(const Offset(0, -48.5));
await tester.pumpAndSettle();
// Item 4 is pushed down by item 5's extent: 376 + 106.
final double pushedDown = tester.getTopLeft(item4).dy;
expect(pushedDown, 482.0);
// Creep upward half a pixel at a time; item 4 must not flip back to 376.
for (int move = 0; move < 8; move++) {
await drag.moveBy(const Offset(0, -0.5));
await tester.pumpAndSettle();
expect(tester.getTopLeft(item4).dy, pushedDown);
}
await drag.up();
await tester.pumpAndSettle();
});Next steps
- Replace or fix the test as described above and inline in comments (and re-check with
flutter test --dart-define=LEAK_TRACKING=true test/widgets/reorderable_list_test.dart). - Address the inline nits.
- Push, and I'll re-run CI and review promptly — the fix itself is good to go.
Thanks again!
Renzo-Olivares
approved these changes
Nov 4, 2026
Renzo-Olivares
left a comment
Renzo-Olivares
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thank you @suryaKommana2662!
Renzo-Olivares
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
This was referenced Nov 5, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Nov 6, 2026
Roll Flutter from 3623d2d34543 to 09175cdeac89 (61 revisions) flutter/flutter@3623d2d...09175cd 2026-11-06 [email protected] Roll Skia from ae35b7b2221f to 793b8ac73d4b (3 revisions) (flutter/flutter#196091) 2026-11-06 [email protected] [web] Rasterize scenes at the size passed to `FlutterView.render` (flutter/flutter#196052) 2026-11-06 [email protected] Roll Skia from 4aa043a81e0c to ae35b7b2221f (1 revision) (flutter/flutter#196086) 2026-11-06 [email protected] Roll Fuchsia Linux SDK from Fqe3UlLwAohsPbsCS... to KIc0XZbhLmvDpTluZ... (flutter/flutter#196085) 2026-11-06 [email protected] Allow a zero scale change in InteractiveViewer (flutter/flutter#194949) 2026-11-06 [email protected] Roll Skia from dd5c2553a856 to 4aa043a81e0c (2 revisions) (flutter/flutter#196079) 2026-11-06 [email protected] [web] Keep ImageFilter.blur bounds in equality and description (flutter/flutter#196013) 2026-11-06 [email protected] iOS: Remove unused PlatformViewIOS setup from FlutterPlatformViewsTest (flutter/flutter#196033) 2026-11-06 [email protected] Serialize LoggerTests that mutate the shared Logger (flutter/flutter#196028) 2026-11-06 [email protected] Use reverseDuration for the AnimatedCrossFade size animation (flutter/flutter#194860) 2026-11-05 [email protected] Roll Skia from a5aacd103e68 to dd5c2553a856 (8 revisions) (flutter/flutter#196073) 2026-11-05 [email protected] Fix ListWheelScrollView null check error when it reads itemIndex before content dimensions are set (flutter/flutter#194859) 2026-11-05 [email protected] Allow zero cross axis extent in SliverGridDelegateWithMaxCrossAxisExtent (flutter/flutter#194858) 2026-11-05 [email protected] Revert: Allow color delta tolerance in in-memory fuzzy image comparator (flutter/flutter#196067) 2026-11-05 [email protected] Revert: Mitigate snapshot race in 3D transform test by reducing loop density (flutter/flutter#196066) 2026-11-05 [email protected] Delegate createStream in ResizeImage and ScrollAwareImageProvider (flutter/flutter#194756) 2026-11-05 [email protected] [iOS] Introduce FlutterShell and move thread host and profiler (flutter/flutter#195388) 2026-11-05 [email protected] [web] Support FragmentShader.getImageSampler on CanvasKit and Skwasm (flutter/flutter#195854) 2026-11-05 [email protected] Deprecates setFloat API (flutter/flutter#195505) 2026-11-05 [email protected] [flutter_tools] Remove public GlobalToolContext and globals from runner (flutter/flutter#195561) 2026-11-05 [email protected] Defer TLHC PlatformViewWrapper frame post and avoid initial 1x1 buffer resize (flutter/flutter#195861) 2026-11-05 [email protected] Roll Skia from b5abed66da86 to a5aacd103e68 (17 revisions) (flutter/flutter#196054) 2026-11-05 [email protected] Made `FlutterWindowControllerSizeTest.SizedToContentNotResizable` more robust (flutter/flutter#195845) 2026-11-05 [email protected] Update docs for contributor ladder (flutter/flutter#196011) 2026-11-05 [email protected] [Impeller] Use the transfer stage for the Vulkan readback barrier (flutter/flutter#194991) 2026-11-05 [email protected] Remove Mac mac_unopt builder (flutter/flutter#196027) 2026-11-05 [email protected] Updated Instructions for Determining Gradle Version for `gradle_dists` in CIPD (flutter/flutter#193310) 2026-11-05 [email protected] Move ios_app_with_extensions_test into the tool integration shard (flutter/flutter#195609) 2026-11-05 [email protected] [web] Respect Uint8List view bounds when detecting animated WebP/GIF (flutter/flutter#196010) 2026-11-05 [email protected] Add platform_views_layout to build_tests so iOS/Android build breaks are caught in presubmit (flutter/flutter#192985) 2026-11-05 [email protected] [flutter_tools] Detect macOS TCC permission crashes and provide diagnostic guidance (flutter/flutter#192432) 2026-11-05 [email protected] Roll Packages from 670b30e to 0478162 (15 revisions) (flutter/flutter#196047) 2026-11-05 [email protected] [web] Support Skwasm rendering without bitmap capture (flutter/flutter#195103) 2026-11-05 [email protected] [flutter_tools] Reland fix for local CanvasKit asset paths on Windows (flutter/flutter#195770) 2026-11-05 [email protected] Roll Skia from f96be51af95d to b5abed66da86 (14 revisions) (flutter/flutter#196017) 2026-11-05 [email protected] Split chrome web engine test drone (flutter/flutter#195873) 2026-11-05 [email protected] Roll Dart SDK from c9153304136a to 6623b84628f3 (1 revision) (flutter/flutter#196026) 2026-11-05 [email protected] Roll ICU from 4fe960221710 to a0ea93436c52 (6 revisions) (flutter/flutter#196025) 2026-11-05 [email protected] Add more guidance in README and AGENTS for contributing to flutter_tools (flutter/flutter#195998) 2026-11-05 [email protected] Roll Fuchsia Linux SDK from p0HDXLISPIElJfSz7... to Fqe3UlLwAohsPbsCS... (flutter/flutter#196019) 2026-11-05 [email protected] Roll Dart SDK from e362a23e9738 to c9153304136a (18 revisions) (flutter/flutter#196014) 2026-11-05 [email protected] [Impeller] Fix dilation input bounds when clipping saveLayers (flutter/flutter#195652) 2026-11-05 [email protected] Add `variantOf` to the M3E migration checklist (flutter/flutter#195099) 2026-11-05 [email protected] [flutter_tools] Eliminate globals from remaining subsystem files (flutter/flutter#195555) 2026-11-05 [email protected] Fix gap flipping when dragging variable height reorderable items (flutter/flutter#194478) 2026-11-05 [email protected] [A11y] Send un-merged SemanticsNodes in the same update that first references them (flutter/flutter#195829) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When dragging an item in a SliverReorderableList containing items of different heights, the gap can flicker between positions as the pointer moves in small increments. This results in unstable reorder animations.
Changes
Testing
Targeted Regression Test:
flutter test packages/flutter/test/widgets/reorderable_list_test.dart --plain-name "SliverReorderableList keeps gap stable when dragging variable height items"Full Test Suite:
flutter test packages/flutter/test/widgets/reorderable_list_test.dart(All 50 tests pass)Code Hygiene:
git diff --checkpassed cleanly.Issue
#194436
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.