)]}'
{
  "commit": "b42e34dbe478e582134f85862e080fbfd9eeacf8",
  "tree": "10275485209f51e80fcb27b77bf173bf596b7b5c",
  "parents": [
    "602740c1081893e0d3f4e78a58ac62093c698d9c"
  ],
  "author": {
    "name": "Luke Memet",
    "email": "1598289+lukemmtt@users.noreply.github.com",
    "time": "Thu Aug 07 15:30:24 2025 -0400"
  },
  "committer": {
    "name": "dart-internal-monorepo",
    "email": "dart-internal-monorepo@dart-ci-internal.iam.gserviceaccount.com",
    "time": "Thu Aug 07 13:09:23 2025 -0700"
  },
  "message": "Fix `ReorderableList` proxy animation for partial drag-back (#172380)\n\n_**Note:** Alongside this PR, I\u0027ve also prepared [another\nPR](https://github.com/flutter/flutter/pull/172882) with an alternative\nsolution involving a more substantial refactor that addresses the root\ncause, rather than adding more conditional logic._\n\n## Description\n\nThis PR fixes the proxy animation bug where dragging a `ReorderableList`\nitem downward and then back to its original position causes it to\nanimate to the wrong location (one position too low).\n\n## The Problem\n\nWhen dragging a `ReorderableList` item downward and then back to its\noriginal position, the proxy widget briefly animates to the wrong\nlocation (one position too low) before snapping to the correct spot.\n\n**Reproduction**: Drag any item down past at least one other item, then\ndrag it back to where it started.\n\n\u003cp align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/d0931dff-5600-441c-8536-2c61789767d0\"\nalt\u003d\"demo2\" width\u003d\"250\"\u003e\n\u003c/p\u003e\n\n## Root Cause\n\nThis bug is specific to dragging an item down and then bringing it back\nup to nearly (but not 100% of the way ) to its original position:\n\n1. When the item approaches its original position **from below**,\n`_insertIndex` becomes `item.index + 1`\n- This happens because Flutter\u0027s `ReorderableList` calculates\n`_insertIndex` with the dragged item still present in the list (see\n#24786)\n2. The proxy _should_ animate to the item\u0027s original position at\n`item.index`\n    - _But the proxy actually animates one position too low._\n    - This happens because `_dragEnd` incorrectly calculates\n    `_finalDropPosition \u003d _itemOffsetAt(_insertIndex! - 1) +\n  _extentOffset(...)`\n- The `_extentOffset(...)` addition, designed for items dropping\n_between other items_, shifts the position down by one item\u0027s height\n- The correct calculation for \"returning home from below\" should be just\n`_itemOffsetAt(_insertIndex! - 1)`\n\nNote that this only occurs when returning from below (`_insertIndex \u003e\nitem.index`). Dragging upward (in a vertical list for example) or\ndoesn\u0027t trigger this bug.\n\n## Existing Implementation\n\nThe existing `_dragEnd` method in `reorderable_list.dart`:\n\n```dart\nvoid _dragEnd(_DragInfo item) {\n  setState(() {\n    if (_insertIndex \u003d\u003d item.index) {\n      _finalDropPosition \u003d _itemOffsetAt(_insertIndex!);\n    } else if (_reverse) {\n      if (_insertIndex! \u003e\u003d _items.length) {\n        _finalDropPosition \u003d\n            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);\n      } else {\n        _finalDropPosition \u003d\n            _itemOffsetAt(_insertIndex!) +\n            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);\n      }\n    } else {\n      if (_insertIndex! \u003d\u003d 0) {\n        _finalDropPosition \u003d _itemOffsetAt(0) -\n          _extentOffset(item.itemExtent, _scrollDirection);\n      } else {\n        _finalDropPosition \u003d _itemOffsetAt(_insertIndex! - 1) +\n          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);\n      }\n    }\n  });\n}\n```\n\nWhen returning from below, the code falls through to the final else\nblock, which incorrectly adds `_extentOffset`.\n\n## Fix\n\nDetect when `_insertIndex - item.index \u003d\u003d 1` (indicating a return to\noriginal position from below) and animate to the correct position.\n\n```dart\nif (_insertIndex! - item.index \u003d\u003d 1) {\n  // Drop at the original position when item returns from below\n  _finalDropPosition \u003d _itemOffsetAt(_insertIndex! - 1);\n}\n```\n\nThis fix was proposed by @frankpape in\nhttps://github.com/flutter/flutter/issues/90856#issuecomment-2599368939;\nI\u0027ve merely validated and researched the background of why the fix\nworks, and supported it with tests.\n\n**_Demo of the fixed implementation:_**\n\u003cp align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/a53e8920-ebca-4326-abe9-3b43b34419e5\"\nalt\u003d\"fixed\" width\u003d\"250\"\u003e\n\u003c/p\u003e\n\nFixes #88331\nFixes #90856\nFixes #150843\n\n## A note about a previous PR:\n\nWhile investigating this issue, I found a PR addressing what seemed to\nbe [the same exact\nissue](https://github.com/flutter/flutter/issues/150843): PR #151026; it\nturns out that that PR solved a _portion_ of the edge case: the case\nwhere an item is dragged down and back and slightly **overshoots** its\noriginal position when being dragged back \u0026 dropped—but that PR did not\naccount for the presence of this bug when the dragged item slightly\n**undershoots** its original position on the return drag. This new PR\neffectively addresses the \u0027undershooting\u0027 case.\n\nWith this, I\u0027ve added a new pair of regression tests that are identical\nto the [previous PR\u0027s\ntests](https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/reorderable_list_test.dart#L734),\nexcept for the fact that they simulate an undershoot on the return trip\n(90% of the way back instead of 110% like the original tests). This\ndefinitively captures the issue, failing in the master branch and\npassing in this PR\u0027s branch.\n\nHere is the specific case resolved by the [**old**\nPR](https://github.com/flutter/flutter/pull/151026):\n\u003ctable\u003e\n  \u003ctr\u003e\n    \u003ctd align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/b0ddc745-6e9e-4f12-97da-454e2e76b06d\"\nalt\u003d\"Before\" width\u003d\"200\"\u003e\u003cbr\u003e\n      \u003csub\u003eBefore\u003c/sub\u003e\n    \u003c/td\u003e\n    \u003ctd align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/03e181fa-f43b-4405-b0c0-16d3465ad990\"\nalt\u003d\"After\" width\u003d\"200\"\u003e\u003cbr\u003e\n      \u003csub\u003eAfter\u003c/sub\u003e\n    \u003c/td\u003e\n  \u003c/tr\u003e\n\u003c/table\u003e\n\nHere is the additional case resolved by **this** PR:\n\u003ctable\u003e\n  \u003ctr\u003e\n    \u003ctd align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/9b4bb591-aa2f-4cf0-88b8-a3ec32b0f0ac\"\nalt\u003d\"Before\" width\u003d\"200\"\u003e\u003cbr\u003e\n      \u003csub\u003eBefore\u003c/sub\u003e\n    \u003c/td\u003e\n    \u003ctd align\u003d\"center\"\u003e\n\u003cimg\nsrc\u003d\"https://github.com/user-attachments/assets/31646e9c-78f4-4252-921f-53583193868f\"\nalt\u003d\"After\" width\u003d\"200\"\u003e\u003cbr\u003e\n      \u003csub\u003eAfter\u003c/sub\u003e\n    \u003c/td\u003e\n  \u003c/tr\u003e\n\u003c/table\u003e\n\nTwo final observations worth noting:\n- The fix proposed in this PR seems to **supersede** the previous PR\u0027s\nsolution; it addresses both cases (overshooting and undershooting) even\nin my tests with the [original PR\u0027s changes\n](https://github.com/flutter/flutter/pull/151026/files#diff-23a4bb073009d89f09084bdf5f85232de135b8f11be625e6312bb85900a90e67)\nreverted. Probably best to keep the old PR\u0027s code anyway to be\nconservative, but noteworthy.\n- I also found it notable that neither this PR nor the older PR fix any\nissue with \"reversed lists\", which, in my tests, are simply not subject\nto this edge case as we\u0027ve defined it. The regression tests added for\nthe reverse case are thus purely precautionary.\n\n## Pre-launch Checklist\n\n- [x] I read the [Contributor Guide] and followed the process outlined\nthere for submitting PRs.\n- [x] I read the [Tree Hygiene] wiki page, which explains my\nresponsibilities.\n- [x] I read and followed the [Flutter Style Guide], including [Features\nwe expect every widget to implement].\n- [x] I signed the [CLA].\n- [x] I listed at least one issue that this PR fixes in the description\nabove.\n- [x] I updated/added relevant documentation (doc comments with `///`).\n- [x] I added new tests to check the change I am making, or this PR is\n[test-exempt].\n- [x] I followed the [breaking change policy] and added [Data Driven\nFixes] where supported.\n- [x] All existing and new tests are passing.\n\n\u003c!-- Links --\u003e\n[Contributor Guide]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview\n[Tree Hygiene]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md\n[test-exempt]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests\n[Flutter Style Guide]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md\n[Features we expect every widget to implement]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement\n[CLA]: https://cla.developers.google.com/\n[flutter/tests]: https://github.com/flutter/tests\n[breaking change policy]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes\n[Discord]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md\n[Data Driven Fixes]:\nhttps://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md\nhttps://dart.googlesource.com/external/github.com/flutter/flutter/+/b6e78b9305f28270f2c4e41ea82af54e0ec80da1\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "b27b2e0866ce13e2a4cc4ffe3ca4d9b4aefb3615",
      "old_mode": 33188,
      "old_path": "DEPS",
      "new_id": "ac864f405831fb5c25e5cf65f5ec90e3b53f82ec",
      "new_mode": 33188,
      "new_path": "DEPS"
    },
    {
      "type": "modify",
      "old_id": "2e45b0f2b33ad5a2e907857e0cad430cdb6f2e08",
      "old_mode": 33188,
      "old_path": "commits.json",
      "new_id": "00d49b50f21605b1da6b20192df6e6979b45a6cd",
      "new_mode": 33188,
      "new_path": "commits.json"
    }
  ]
}