Repository navigation
Commit 31f6edd
authored
Fix AnimatedList.separated assert when removing last item mid-removal… (#186389)
## Summary
`AnimatedList.separated.removeItem` used the sliver's total child count
(`_itemsCount`) to decide whether the just-removed item was the last
item in the list. That count still includes children animating out from
earlier `removeItem` calls, so while a previous removal was in flight
the new tail of the visible list was misclassified as a middle item. The
separator removal then targeted the wrong index, and once the item being
removed was added to `_outgoingItems` the next `_indexToItemIndex`
lookup ran past `_itemsCount` and tripped the `itemIndex >= 0 &&
itemIndex < _itemsCount` assertion inside
`_SliverAnimatedMultiBoxAdaptorState.removeItem`.
The fix captures `_itemsCount - _outgoingItems.length` before mutating
the sliver and uses that visible count for both the "are there enough
items to have a separator?" and "is this the new last item?" checks.
When no removal is in flight `_outgoingItems` is empty and behavior is
unchanged, so existing `.separated` coverage and the `removeAllItems`
regression test from #176452 continue to pass.
## Related issues
Fixes #186361.
Related prior fix for the non-separated `removeAllItems` variant:
#176452 (issue #176362).
## Test plan
- [x] Added a regression test in
`packages/flutter/test/widgets/animated_list_test.dart` that reproduces
the assertion on master and passes with this change: starts a 10s
removal on a non-last item, pumps part of the animation, then removes
the new last item, and asserts no exception is thrown.
- [x] `flutter test
packages/flutter/test/widgets/animated_list_test.dart
packages/flutter/test/widgets/animated_grid_test.dart` — 34 tests pass,
including the existing comprehensive `AnimatedList.separated` test and
the `removeAllItems` regression test from #176452.
- [x] `dart analyze` on the touched files: no issues.
- [x] Manually verified with the reproducer from the issue: before the
fix the assertion fires in the debug console; after the fix the second
removal animates out cleanly.
## AI disclosure
Written with assistance from Claude Code (Anthropic). I reviewed every
change, ran the tests, and verified the fix manually before submitting.
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md1 parent d95bae8 commit 31f6edd
2 files changed
Lines changed: 83 additions & 2 deletions
File tree
- packages/flutter
- lib/src/widgets
- test/widgets
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
749 | 749 | | |
750 | 750 | | |
751 | 751 | | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
752 | 756 | | |
753 | 757 | | |
754 | | - | |
755 | | - | |
| 758 | + | |
| 759 | + | |
756 | 760 | | |
757 | 761 | | |
758 | 762 | | |
| |||
816 | 820 | | |
817 | 821 | | |
818 | 822 | | |
| 823 | + | |
| 824 | + | |
819 | 825 | | |
820 | 826 | | |
821 | 827 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1204 | 1204 | | |
1205 | 1205 | | |
1206 | 1206 | | |
| 1207 | + | |
| 1208 | + | |
| 1209 | + | |
| 1210 | + | |
| 1211 | + | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
| 1219 | + | |
| 1220 | + | |
| 1221 | + | |
| 1222 | + | |
| 1223 | + | |
| 1224 | + | |
| 1225 | + | |
| 1226 | + | |
| 1227 | + | |
| 1228 | + | |
| 1229 | + | |
| 1230 | + | |
| 1231 | + | |
| 1232 | + | |
| 1233 | + | |
| 1234 | + | |
| 1235 | + | |
| 1236 | + | |
| 1237 | + | |
| 1238 | + | |
| 1239 | + | |
| 1240 | + | |
| 1241 | + | |
| 1242 | + | |
| 1243 | + | |
| 1244 | + | |
| 1245 | + | |
| 1246 | + | |
| 1247 | + | |
| 1248 | + | |
| 1249 | + | |
| 1250 | + | |
| 1251 | + | |
| 1252 | + | |
| 1253 | + | |
| 1254 | + | |
| 1255 | + | |
| 1256 | + | |
| 1257 | + | |
| 1258 | + | |
| 1259 | + | |
| 1260 | + | |
| 1261 | + | |
| 1262 | + | |
| 1263 | + | |
| 1264 | + | |
| 1265 | + | |
| 1266 | + | |
| 1267 | + | |
| 1268 | + | |
| 1269 | + | |
| 1270 | + | |
| 1271 | + | |
| 1272 | + | |
| 1273 | + | |
| 1274 | + | |
| 1275 | + | |
| 1276 | + | |
| 1277 | + | |
| 1278 | + | |
| 1279 | + | |
| 1280 | + | |
| 1281 | + | |
1207 | 1282 | | |
1208 | 1283 | | |
1209 | 1284 | | |
| |||
0 commit comments