Skip to content

Conversation

@lukemmtt
Copy link
Contributor

@lukemmtt lukemmtt commented Jul 18, 2025

Note: Alongside this PR, I've also prepared another PR with an alternative solution involving a more substantial refactor that addresses the root cause, rather than adding more conditional logic.

Description

This PR fixes the proxy animation bug where dragging a ReorderableList item downward and then back to its original position causes it to animate to the wrong location (one position too low).

The Problem

When dragging a ReorderableList item downward and then back to its original position, the proxy widget briefly animates to the wrong location (one position too low) before snapping to the correct spot.

Reproduction: Drag any item down past at least one other item, then drag it back to where it started.

demo2

Root Cause

This bug is specific to dragging an item down and then bringing it back up to nearly (but not 100% of the way ) to its original position:

  1. When the item approaches its original position from below, _insertIndex becomes item.index + 1
  2. The proxy should animate to the item's original position at item.index
    • But the proxy actually animates one position too low.
    • This happens because _dragEnd incorrectly calculates
      _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) + _extentOffset(...)
    • The _extentOffset(...) addition, designed for items dropping between other items, shifts the position down by one item's height
    • The correct calculation for "returning home from below" should be just _itemOffsetAt(_insertIndex! - 1)

Note that this only occurs when returning from below (_insertIndex > item.index). Dragging upward (in a vertical list for example) or doesn't trigger this bug.

Existing Implementation

The existing _dragEnd method in reorderable_list.dart:

void _dragEnd(_DragInfo item) {
  setState(() {
    if (_insertIndex == item.index) {
      _finalDropPosition = _itemOffsetAt(_insertIndex!);
    } else if (_reverse) {
      if (_insertIndex! >= _items.length) {
        _finalDropPosition =
            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition =
            _itemOffsetAt(_insertIndex!) +
            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
      }
    } else {
      if (_insertIndex! == 0) {
        _finalDropPosition = _itemOffsetAt(0) - 
          _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);
      }
    }
  });
}

When returning from below, the code falls through to the final else block, which incorrectly adds _extentOffset.

Fix

Detect when _insertIndex - item.index == 1 (indicating a return to original position from below) and animate to the correct position.

if (_insertIndex! - item.index == 1) {
  // Drop at the original position when item returns from below
  _finalDropPosition = _itemOffsetAt(_insertIndex! - 1);
}

This fix was proposed by @frankpape in #90856 (comment); I've merely validated and researched the background of why the fix works, and supported it with tests.

Demo of the fixed implementation:

fixed

Fixes #88331
Fixes #90856
Fixes #150843

A note about a previous PR:

While investigating this issue, I found a PR addressing what seemed to be the same exact issue: PR #151026; it turns out that that PR solved a portion of the edge case: the case where an item is dragged down and back and slightly overshoots its original position when being dragged back & dropped—but that PR did not account for the presence of this bug when the dragged item slightly undershoots its original position on the return drag. This new PR effectively addresses the 'undershooting' case.

With this, I've added a new pair of regression tests that are identical to the previous PR's tests, except for the fact that they simulate an undershoot on the return trip (90% of the way back instead of 110% like the original tests). This definitively captures the issue, failing in the master branch and passing in this PR's branch.

Here is the specific case resolved by the old PR:

Before
Before
After
After

Here is the additional case resolved by this PR:

Before
Before
After
After

Two final observations worth noting:

  • The fix proposed in this PR seems to supersede the previous PR's solution; it addresses both cases (overshooting and undershooting) even in my tests with the original PR's changes reverted. Probably best to keep the old PR's code anyway to be conservative, but noteworthy.
  • I also found it notable that neither this PR nor the older PR fix any issue with "reversed lists", which, in my tests, are simply not subject to this edge case as we've defined it. The regression tests added for the reverse case are thus purely precautionary.

Pre-launch Checklist

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 18, 2025
@lukemmtt lukemmtt force-pushed the fix-reorderable-list-drag-feedback branch 3 times, most recently from 8b81001 to 4291801 Compare July 19, 2025 12:48
@lukemmtt lukemmtt requested a review from matanlurey as a code owner July 19, 2025 12:48
@github-actions github-actions bot added the a: animation Animation APIs label Jul 19, 2025
@lukemmtt lukemmtt force-pushed the fix-reorderable-list-drag-feedback branch from 4291801 to af88190 Compare July 20, 2025 00:01
@justinmc justinmc requested a review from Piinks July 22, 2025 22:16
@lukemmtt lukemmtt force-pushed the fix-reorderable-list-drag-feedback branch from c2f53e3 to a3b1870 Compare July 23, 2025 01:54
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lukemmtt! Welcome. Thank you for contributing. Rather than an integration test, this should be easier to test with a simple unit test in packages/flutter/test/widgets/reorderable_list_test.dart
Can you update to test this there? I think most of CI is failing since the changes here are not quite right for setting up a new integration test, but reverting and adding a unit test should resolve those. :)

@lukemmtt
Copy link
Contributor Author

lukemmtt commented Jul 23, 2025

Hey @lukemmtt! Welcome. Thank you for contributing. Rather than an integration test, this should be easier to test with a simple unit test in packages/flutter/test/widgets/reorderable_list_test.dart Can you update to test this there? I think most of CI is failing since the changes here are not quite right for setting up a new integration test, but reverting and adding a unit test should resolve those. :)

Thanks @Piinks for looking into this and for your reply!

I certainly agree that an integration test is a last resort, and I'm not surprised I set it up wrong (sorry 🥲!) but I'm afraid I was unable to create a comparable unit test despite hours of trying. The thing is, no matter what I tried, I could not manage to create a unit test that properly captured the direction of movement of the proxy as it was animating to its final position upon being released (which is the basis for the integration test). Every time I tried to 'observe' the animation during a unit test (even one frame later), I was finding that the proxy had already 'settled' into its final position, as if it wasn't animating at all. I asked AI models about it, and they just said that maybe the animation frames were being 'collapsed' in the unit test, which would make any "observing the movement of the proxy over time" an impossible task.

I'd love to be proven wrong, but I'm afraid I'm out of ideas. Open to any suggestions!

@Piinks
Copy link
Contributor

Piinks commented Jul 23, 2025

Thanks for the update. Can you share some of the unit test code you tried to reproduce with? Its hard to say without it what might have been going on.

@lukemmtt
Copy link
Contributor Author

@Piinks I scrapped it, but I trust that if you're asking, it must be possible, so I'll be glad to investigate the existing tests and and keep trying to figure it out. I appreciate the feedback!

@lukemmtt
Copy link
Contributor Author

lukemmtt commented Jul 24, 2025

Hey @Piinks, some updates:

In looking through the existing widget tests for ReorderableList, I was surprised to find one that sounded extremely similar to the condition I'm trying to test, called "properly animates the drop at starting position", added in PR #151026, which was a fix for #150843, which, in its filing video, is shown to look like this:

...which looks strikingly similar to the issue in my own PR here. Perhaps that issue was subtly different in some way, or perhaps it simply regressed, but it certainly seems to persist in my own tests in the master branch, despite that PR being merged in December 2024 and rolled into Flutter 3.27.0 the same month).

  • Moreover, that PR's widget test seems to be adequate for reproducing this issue at a glance, so I'm curious as to why it's not failing today. This makes me wonder of the suitability of using a widget test or unit test to reliably protect against the regression of this issue.

All that is to say: I understand that we would like to avoid an integration test if at all possible, but I remain unsure of how to make that happen effectively. I remain eager to learn how I can do better here.

Thanks for your time and for any additional feedback!

@lukemmtt lukemmtt changed the title Fix ReorderableList proxy animation when returning to original position Fix ReorderableList item animating one position too low when returned to start Jul 27, 2025
@lukemmtt lukemmtt force-pushed the fix-reorderable-list-drag-feedback branch 2 times, most recently from 54aea89 to 13bdb6d Compare July 28, 2025 20:43
@lukemmtt lukemmtt changed the title Fix ReorderableList item animating one position too low when returned to start Fix ReorderableList return-to-origin animation (minimal approach) Jul 28, 2025
@github-actions github-actions bot removed the framework flutter/packages/flutter repository. See also f: labels. label Jul 28, 2025
@lukemmtt lukemmtt force-pushed the fix-reorderable-list-drag-feedback branch 2 times, most recently from 0e1149f to 7a353b0 Compare July 28, 2025 21:32
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. and removed a: animation Animation APIs labels Jul 28, 2025
@lukemmtt
Copy link
Contributor Author

lukemmtt commented Aug 4, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an animation bug in ReorderableList that occurs when an item is dragged and then partially moved back to its original position. The fix introduces a specific condition to handle this "undershoot" scenario, preventing the item from animating to an incorrect offset. The change is well-explained and includes new regression tests that cover the fixed behavior. My review confirms the logic of the fix and the added tests. I have one minor suggestion to improve the clarity of a comment in one of the new tests for better maintainability.

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@Piinks
Copy link
Contributor

Piinks commented Aug 6, 2025

Note: Alongside this PR, I've also prepared #172882 with an alternative solution involving a more substantial refactor that addresses the root cause, rather than adding more conditional logic.

LOL! I too was like, I feel like we keep adding conditional cases and was drafting a simpler flow over here. 😀
Let's land this, and then maybe we can compare notes to refactor?

Copy link
Contributor

@victorsanni victorsanni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for the fix!

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 7, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 7, 2025
Merged via the queue into flutter:master with commit b6e78b9 Aug 7, 2025
74 of 75 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 7, 2025
@lukemmtt lukemmtt deleted the fix-reorderable-list-drag-feedback branch August 7, 2025 20:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 8, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 8, 2025
flutter/flutter@92a6bfb...3821790

2025-08-08 [email protected] Use LLDB as the default debugging method for iOS 17+ and Xcode 26+ (flutter/flutter#173443)
2025-08-08 [email protected] Roll Fuchsia Linux SDK from i4vsuEGyP8Xeb5tiy... to HclTm0V8hgSpfqmtG... (flutter/flutter#173462)
2025-08-08 [email protected] Support launching a HTTPS URL (flutter/flutter#164720)
2025-08-08 [email protected] Roll Dart SDK from c48772a79e1f to 4b7b565eb468 (1 revision) (flutter/flutter#173454)
2025-08-08 [email protected] Roll Dart SDK from ba58b96a80d7 to c48772a79e1f (3 revisions) (flutter/flutter#173451)
2025-08-07 [email protected] Web dev proxy (flutter/flutter#172175)
2025-08-07 [email protected] Roll ICU from b929596baebf to 1b2e3e8a421e (7 revisions) (flutter/flutter#173436)
2025-08-07 [email protected] [A11y] TextField prefix icon and suffix icon create a sibling node' (flutter/flutter#173312)
2025-08-07 [email protected] [Android templates] Remove jetifier usage (flutter/flutter#173431)
2025-08-07 [email protected] Remove a couple of asserts from display_list_unittest (flutter/flutter#173381)
2025-08-07 [email protected] Fix ScaffoldGeometry null scale with noAnimation FAB (flutter/flutter#172914)
2025-08-07 [email protected] Prepare for iOS debugging with lldb and devicectl (flutter/flutter#173417)
2025-08-07 [email protected] Fix `ReorderableList` proxy animation for partial drag-back (flutter/flutter#172380)
2025-08-07 [email protected] [ios26]Do not report error for Info.plist key not found (flutter/flutter#172913)
2025-08-07 [email protected] Manual roll to 3.10.0-75.1.beta (flutter/flutter#173423)
2025-08-07 [email protected] [web] add --static-assets-url argument to build web (flutter/flutter#171638)
2025-08-07 [email protected] Adds deprecation for impeller opt out on android (flutter/flutter#173375)

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 [email protected],[email protected] 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
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…172380)

_**Note:** Alongside this PR, I've also prepared [another
PR](flutter#172882) with an alternative
solution involving a more substantial refactor that addresses the root
cause, rather than adding more conditional logic._

## Description

This PR fixes the proxy animation bug where dragging a `ReorderableList`
item downward and then back to its original position causes it to
animate to the wrong location (one position too low).

## The Problem

When dragging a `ReorderableList` item downward and then back to its
original position, the proxy widget briefly animates to the wrong
location (one position too low) before snapping to the correct spot.

**Reproduction**: Drag any item down past at least one other item, then
drag it back to where it started.

<p align="center">
<img
src="https://github.com/user-attachments/assets/d0931dff-5600-441c-8536-2c61789767d0"
alt="demo2" width="250">
</p>

## Root Cause

This bug is specific to dragging an item down and then bringing it back
up to nearly (but not 100% of the way ) to its original position:

1. When the item approaches its original position **from below**,
`_insertIndex` becomes `item.index + 1`
- This happens because Flutter's `ReorderableList` calculates
`_insertIndex` with the dragged item still present in the list (see
flutter#24786)
2. The proxy _should_ animate to the item's original position at
`item.index`
    - _But the proxy actually animates one position too low._
    - This happens because `_dragEnd` incorrectly calculates 
    `_finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
  _extentOffset(...)`
- The `_extentOffset(...)` addition, designed for items dropping
_between other items_, shifts the position down by one item's height
- The correct calculation for "returning home from below" should be just
`_itemOffsetAt(_insertIndex! - 1)`

Note that this only occurs when returning from below (`_insertIndex >
item.index`). Dragging upward (in a vertical list for example) or
doesn't trigger this bug.

## Existing Implementation

The existing `_dragEnd` method in `reorderable_list.dart`:

```dart
void _dragEnd(_DragInfo item) {
  setState(() {
    if (_insertIndex == item.index) {
      _finalDropPosition = _itemOffsetAt(_insertIndex!);
    } else if (_reverse) {
      if (_insertIndex! >= _items.length) {
        _finalDropPosition =
            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition =
            _itemOffsetAt(_insertIndex!) +
            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
      }
    } else {
      if (_insertIndex! == 0) {
        _finalDropPosition = _itemOffsetAt(0) - 
          _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);
      }
    }
  });
}
```

When returning from below, the code falls through to the final else
block, which incorrectly adds `_extentOffset`.

## Fix

Detect when `_insertIndex - item.index == 1` (indicating a return to
original position from below) and animate to the correct position.

```dart
if (_insertIndex! - item.index == 1) {
  // Drop at the original position when item returns from below
  _finalDropPosition = _itemOffsetAt(_insertIndex! - 1);
}
```

This fix was proposed by @frankpape in
flutter#90856 (comment);
I've merely validated and researched the background of why the fix
works, and supported it with tests.

**_Demo of the fixed implementation:_**
<p align="center">
<img
src="https://github.com/user-attachments/assets/a53e8920-ebca-4326-abe9-3b43b34419e5"
alt="fixed" width="250">
</p>

Fixes flutter#88331
Fixes flutter#90856
Fixes flutter#150843

## A note about a previous PR: 

While investigating this issue, I found a PR addressing what seemed to
be [the same exact
issue](flutter#150843): PR flutter#151026; it
turns out that that PR solved a _portion_ of the edge case: the case
where an item is dragged down and back and slightly **overshoots** its
original position when being dragged back & dropped—but that PR did not
account for the presence of this bug when the dragged item slightly
**undershoots** its original position on the return drag. This new PR
effectively addresses the 'undershooting' case.

With this, I've added a new pair of regression tests that are identical
to the [previous PR's
tests](https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/reorderable_list_test.dart#L734),
except for the fact that they simulate an undershoot on the return trip
(90% of the way back instead of 110% like the original tests). This
definitively captures the issue, failing in the master branch and
passing in this PR's branch.

Here is the specific case resolved by the [**old**
PR](flutter#151026):
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/b0ddc745-6e9e-4f12-97da-454e2e76b06d"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/03e181fa-f43b-4405-b0c0-16d3465ad990"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Here is the additional case resolved by **this** PR:
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/9b4bb591-aa2f-4cf0-88b8-a3ec32b0f0ac"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/31646e9c-78f4-4252-921f-53583193868f"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Two final observations worth noting:
- The fix proposed in this PR seems to **supersede** the previous PR's
solution; it addresses both cases (overshooting and undershooting) even
in my tests with the [original PR's changes
](https://github.com/flutter/flutter/pull/151026/files#diff-23a4bb073009d89f09084bdf5f85232de135b8f11be625e6312bb85900a90e67)
reverted. Probably best to keep the old PR's code anyway to be
conservative, but noteworthy.
- I also found it notable that neither this PR nor the older PR fix any
issue with "reversed lists", which, in my tests, are simply not subject
to this edge case as we've defined it. The regression tests added for
the reverse case are thus purely precautionary.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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.md
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…172380)

_**Note:** Alongside this PR, I've also prepared [another
PR](flutter#172882) with an alternative
solution involving a more substantial refactor that addresses the root
cause, rather than adding more conditional logic._

## Description

This PR fixes the proxy animation bug where dragging a `ReorderableList`
item downward and then back to its original position causes it to
animate to the wrong location (one position too low).

## The Problem

When dragging a `ReorderableList` item downward and then back to its
original position, the proxy widget briefly animates to the wrong
location (one position too low) before snapping to the correct spot.

**Reproduction**: Drag any item down past at least one other item, then
drag it back to where it started.

<p align="center">
<img
src="https://github.com/user-attachments/assets/d0931dff-5600-441c-8536-2c61789767d0"
alt="demo2" width="250">
</p>

## Root Cause

This bug is specific to dragging an item down and then bringing it back
up to nearly (but not 100% of the way ) to its original position:

1. When the item approaches its original position **from below**,
`_insertIndex` becomes `item.index + 1`
- This happens because Flutter's `ReorderableList` calculates
`_insertIndex` with the dragged item still present in the list (see
flutter#24786)
2. The proxy _should_ animate to the item's original position at
`item.index`
    - _But the proxy actually animates one position too low._
    - This happens because `_dragEnd` incorrectly calculates 
    `_finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
  _extentOffset(...)`
- The `_extentOffset(...)` addition, designed for items dropping
_between other items_, shifts the position down by one item's height
- The correct calculation for "returning home from below" should be just
`_itemOffsetAt(_insertIndex! - 1)`

Note that this only occurs when returning from below (`_insertIndex >
item.index`). Dragging upward (in a vertical list for example) or
doesn't trigger this bug.

## Existing Implementation

The existing `_dragEnd` method in `reorderable_list.dart`:

```dart
void _dragEnd(_DragInfo item) {
  setState(() {
    if (_insertIndex == item.index) {
      _finalDropPosition = _itemOffsetAt(_insertIndex!);
    } else if (_reverse) {
      if (_insertIndex! >= _items.length) {
        _finalDropPosition =
            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition =
            _itemOffsetAt(_insertIndex!) +
            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
      }
    } else {
      if (_insertIndex! == 0) {
        _finalDropPosition = _itemOffsetAt(0) - 
          _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);
      }
    }
  });
}
```

When returning from below, the code falls through to the final else
block, which incorrectly adds `_extentOffset`.

## Fix

Detect when `_insertIndex - item.index == 1` (indicating a return to
original position from below) and animate to the correct position.

```dart
if (_insertIndex! - item.index == 1) {
  // Drop at the original position when item returns from below
  _finalDropPosition = _itemOffsetAt(_insertIndex! - 1);
}
```

This fix was proposed by @frankpape in
flutter#90856 (comment);
I've merely validated and researched the background of why the fix
works, and supported it with tests.

**_Demo of the fixed implementation:_**
<p align="center">
<img
src="https://github.com/user-attachments/assets/a53e8920-ebca-4326-abe9-3b43b34419e5"
alt="fixed" width="250">
</p>

Fixes flutter#88331
Fixes flutter#90856
Fixes flutter#150843

## A note about a previous PR: 

While investigating this issue, I found a PR addressing what seemed to
be [the same exact
issue](flutter#150843): PR flutter#151026; it
turns out that that PR solved a _portion_ of the edge case: the case
where an item is dragged down and back and slightly **overshoots** its
original position when being dragged back & dropped—but that PR did not
account for the presence of this bug when the dragged item slightly
**undershoots** its original position on the return drag. This new PR
effectively addresses the 'undershooting' case.

With this, I've added a new pair of regression tests that are identical
to the [previous PR's
tests](https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/reorderable_list_test.dart#L734),
except for the fact that they simulate an undershoot on the return trip
(90% of the way back instead of 110% like the original tests). This
definitively captures the issue, failing in the master branch and
passing in this PR's branch.

Here is the specific case resolved by the [**old**
PR](flutter#151026):
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/b0ddc745-6e9e-4f12-97da-454e2e76b06d"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/03e181fa-f43b-4405-b0c0-16d3465ad990"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Here is the additional case resolved by **this** PR:
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/9b4bb591-aa2f-4cf0-88b8-a3ec32b0f0ac"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/31646e9c-78f4-4252-921f-53583193868f"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Two final observations worth noting:
- The fix proposed in this PR seems to **supersede** the previous PR's
solution; it addresses both cases (overshooting and undershooting) even
in my tests with the [original PR's changes
](https://github.com/flutter/flutter/pull/151026/files#diff-23a4bb073009d89f09084bdf5f85232de135b8f11be625e6312bb85900a90e67)
reverted. Probably best to keep the old PR's code anyway to be
conservative, but noteworthy.
- I also found it notable that neither this PR nor the older PR fix any
issue with "reversed lists", which, in my tests, are simply not subject
to this edge case as we've defined it. The regression tests added for
the reverse case are thus purely precautionary.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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.md
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…172380)

_**Note:** Alongside this PR, I've also prepared [another
PR](flutter#172882) with an alternative
solution involving a more substantial refactor that addresses the root
cause, rather than adding more conditional logic._

## Description

This PR fixes the proxy animation bug where dragging a `ReorderableList`
item downward and then back to its original position causes it to
animate to the wrong location (one position too low).

## The Problem

When dragging a `ReorderableList` item downward and then back to its
original position, the proxy widget briefly animates to the wrong
location (one position too low) before snapping to the correct spot.

**Reproduction**: Drag any item down past at least one other item, then
drag it back to where it started.

<p align="center">
<img
src="https://github.com/user-attachments/assets/d0931dff-5600-441c-8536-2c61789767d0"
alt="demo2" width="250">
</p>

## Root Cause

This bug is specific to dragging an item down and then bringing it back
up to nearly (but not 100% of the way ) to its original position:

1. When the item approaches its original position **from below**,
`_insertIndex` becomes `item.index + 1`
- This happens because Flutter's `ReorderableList` calculates
`_insertIndex` with the dragged item still present in the list (see
flutter#24786)
2. The proxy _should_ animate to the item's original position at
`item.index`
    - _But the proxy actually animates one position too low._
    - This happens because `_dragEnd` incorrectly calculates 
    `_finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
  _extentOffset(...)`
- The `_extentOffset(...)` addition, designed for items dropping
_between other items_, shifts the position down by one item's height
- The correct calculation for "returning home from below" should be just
`_itemOffsetAt(_insertIndex! - 1)`

Note that this only occurs when returning from below (`_insertIndex >
item.index`). Dragging upward (in a vertical list for example) or
doesn't trigger this bug.

## Existing Implementation

The existing `_dragEnd` method in `reorderable_list.dart`:

```dart
void _dragEnd(_DragInfo item) {
  setState(() {
    if (_insertIndex == item.index) {
      _finalDropPosition = _itemOffsetAt(_insertIndex!);
    } else if (_reverse) {
      if (_insertIndex! >= _items.length) {
        _finalDropPosition =
            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition =
            _itemOffsetAt(_insertIndex!) +
            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
      }
    } else {
      if (_insertIndex! == 0) {
        _finalDropPosition = _itemOffsetAt(0) - 
          _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);
      }
    }
  });
}
```

When returning from below, the code falls through to the final else
block, which incorrectly adds `_extentOffset`.

## Fix

Detect when `_insertIndex - item.index == 1` (indicating a return to
original position from below) and animate to the correct position.

```dart
if (_insertIndex! - item.index == 1) {
  // Drop at the original position when item returns from below
  _finalDropPosition = _itemOffsetAt(_insertIndex! - 1);
}
```

This fix was proposed by @frankpape in
flutter#90856 (comment);
I've merely validated and researched the background of why the fix
works, and supported it with tests.

**_Demo of the fixed implementation:_**
<p align="center">
<img
src="https://github.com/user-attachments/assets/a53e8920-ebca-4326-abe9-3b43b34419e5"
alt="fixed" width="250">
</p>

Fixes flutter#88331
Fixes flutter#90856
Fixes flutter#150843

## A note about a previous PR: 

While investigating this issue, I found a PR addressing what seemed to
be [the same exact
issue](flutter#150843): PR flutter#151026; it
turns out that that PR solved a _portion_ of the edge case: the case
where an item is dragged down and back and slightly **overshoots** its
original position when being dragged back & dropped—but that PR did not
account for the presence of this bug when the dragged item slightly
**undershoots** its original position on the return drag. This new PR
effectively addresses the 'undershooting' case.

With this, I've added a new pair of regression tests that are identical
to the [previous PR's
tests](https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/reorderable_list_test.dart#L734),
except for the fact that they simulate an undershoot on the return trip
(90% of the way back instead of 110% like the original tests). This
definitively captures the issue, failing in the master branch and
passing in this PR's branch.

Here is the specific case resolved by the [**old**
PR](flutter#151026):
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/b0ddc745-6e9e-4f12-97da-454e2e76b06d"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/03e181fa-f43b-4405-b0c0-16d3465ad990"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Here is the additional case resolved by **this** PR:
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/9b4bb591-aa2f-4cf0-88b8-a3ec32b0f0ac"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/31646e9c-78f4-4252-921f-53583193868f"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Two final observations worth noting:
- The fix proposed in this PR seems to **supersede** the previous PR's
solution; it addresses both cases (overshooting and undershooting) even
in my tests with the [original PR's changes
](https://github.com/flutter/flutter/pull/151026/files#diff-23a4bb073009d89f09084bdf5f85232de135b8f11be625e6312bb85900a90e67)
reverted. Probably best to keep the old PR's code anyway to be
conservative, but noteworthy.
- I also found it notable that neither this PR nor the older PR fix any
issue with "reversed lists", which, in my tests, are simply not subject
to this edge case as we've defined it. The regression tests added for
the reverse case are thus purely precautionary.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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.md
github-merge-queue bot pushed a commit that referenced this pull request Nov 5, 2025
…animation (#173241)

When rapidly dragging items in a ReorderableList before animations
complete, items would jump to their expected positions rather than
smoothly transitioning.

## Problem

<p align="center">
<img
src="https://github.com/user-attachments/assets/0efb250c-2960-4942-959f-59eccc20cefb"
alt="demo2" width="250">
</p>

The issue occurs when a reorder animation is interrupted by starting a
new drag operation. The interrupted animation would reset to the
starting position of the original animation rather than capturing the
current animated position, causing a visual jump.

## Solution

This PR fixes the issue by:
1. Storing the previous target offset before updating to a new target
2. When an animation is interrupted, calculating the actual current
position based on the animation's progress
3. Using this calculated position as the new starting point for the next
animation

<p align="center">
<img
src="https://github.com/user-attachments/assets/c24e4834-1c6b-41c1-8f44-17b4c79e0993"
alt="demo2" width="250">
</p>

This PR is part of a series of `ReorderableList` enhancements I've
developed for [TimeFinder](https://timefinder.app):
- #172740
- #172380
- #172739
- #172738

Fixes #173243

## Code Changes

```dart
// Before
_startOffset = offset;

// After  
final double currentAnimValue = Curves.easeInOut.transform(_offsetAnimation\!.value);
final Offset currentPosition = Offset.lerp(_startOffset, previousTarget, currentAnimValue)\!;
_startOffset = currentPosition;
```

This ensures smooth transitions without visual jumping, making rapid
reordering gestures feel more responsive and natural.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [migration
guide] as needed.
- [x] All existing and new tests are passing.
- [x] The analyzer (`flutter analyze --flutter-repo`) does not report
any problems on my PR.
- [x] I am willing to follow-up on review comments in a timely manner.

[Contributor Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[migration guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#create-a-migration-guide
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…172380)

_**Note:** Alongside this PR, I've also prepared [another
PR](flutter#172882) with an alternative
solution involving a more substantial refactor that addresses the root
cause, rather than adding more conditional logic._

## Description

This PR fixes the proxy animation bug where dragging a `ReorderableList`
item downward and then back to its original position causes it to
animate to the wrong location (one position too low).

## The Problem

When dragging a `ReorderableList` item downward and then back to its
original position, the proxy widget briefly animates to the wrong
location (one position too low) before snapping to the correct spot.

**Reproduction**: Drag any item down past at least one other item, then
drag it back to where it started.

<p align="center">
<img
src="https://github.com/user-attachments/assets/d0931dff-5600-441c-8536-2c61789767d0"
alt="demo2" width="250">
</p>

## Root Cause

This bug is specific to dragging an item down and then bringing it back
up to nearly (but not 100% of the way ) to its original position:

1. When the item approaches its original position **from below**,
`_insertIndex` becomes `item.index + 1`
- This happens because Flutter's `ReorderableList` calculates
`_insertIndex` with the dragged item still present in the list (see
flutter#24786)
2. The proxy _should_ animate to the item's original position at
`item.index`
    - _But the proxy actually animates one position too low._
    - This happens because `_dragEnd` incorrectly calculates 
    `_finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
  _extentOffset(...)`
- The `_extentOffset(...)` addition, designed for items dropping
_between other items_, shifts the position down by one item's height
- The correct calculation for "returning home from below" should be just
`_itemOffsetAt(_insertIndex! - 1)`

Note that this only occurs when returning from below (`_insertIndex >
item.index`). Dragging upward (in a vertical list for example) or
doesn't trigger this bug.

## Existing Implementation

The existing `_dragEnd` method in `reorderable_list.dart`:

```dart
void _dragEnd(_DragInfo item) {
  setState(() {
    if (_insertIndex == item.index) {
      _finalDropPosition = _itemOffsetAt(_insertIndex!);
    } else if (_reverse) {
      if (_insertIndex! >= _items.length) {
        _finalDropPosition =
            _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition =
            _itemOffsetAt(_insertIndex!) +
            _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
      }
    } else {
      if (_insertIndex! == 0) {
        _finalDropPosition = _itemOffsetAt(0) - 
          _extentOffset(item.itemExtent, _scrollDirection);
      } else {
        _finalDropPosition = _itemOffsetAt(_insertIndex! - 1) +
          _extentOffset(_itemExtentAt(_insertIndex! - 1), _scrollDirection);
      }
    }
  });
}
```

When returning from below, the code falls through to the final else
block, which incorrectly adds `_extentOffset`.

## Fix

Detect when `_insertIndex - item.index == 1` (indicating a return to
original position from below) and animate to the correct position.

```dart
if (_insertIndex! - item.index == 1) {
  // Drop at the original position when item returns from below
  _finalDropPosition = _itemOffsetAt(_insertIndex! - 1);
}
```

This fix was proposed by @frankpape in
flutter#90856 (comment);
I've merely validated and researched the background of why the fix
works, and supported it with tests.

**_Demo of the fixed implementation:_**
<p align="center">
<img
src="https://github.com/user-attachments/assets/a53e8920-ebca-4326-abe9-3b43b34419e5"
alt="fixed" width="250">
</p>

Fixes flutter#88331
Fixes flutter#90856
Fixes flutter#150843

## A note about a previous PR: 

While investigating this issue, I found a PR addressing what seemed to
be [the same exact
issue](flutter#150843): PR flutter#151026; it
turns out that that PR solved a _portion_ of the edge case: the case
where an item is dragged down and back and slightly **overshoots** its
original position when being dragged back & dropped—but that PR did not
account for the presence of this bug when the dragged item slightly
**undershoots** its original position on the return drag. This new PR
effectively addresses the 'undershooting' case.

With this, I've added a new pair of regression tests that are identical
to the [previous PR's
tests](https://github.com/flutter/flutter/blob/master/packages/flutter/test/widgets/reorderable_list_test.dart#L734),
except for the fact that they simulate an undershoot on the return trip
(90% of the way back instead of 110% like the original tests). This
definitively captures the issue, failing in the master branch and
passing in this PR's branch.

Here is the specific case resolved by the [**old**
PR](flutter#151026):
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/b0ddc745-6e9e-4f12-97da-454e2e76b06d"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/03e181fa-f43b-4405-b0c0-16d3465ad990"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Here is the additional case resolved by **this** PR:
<table>
  <tr>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/9b4bb591-aa2f-4cf0-88b8-a3ec32b0f0ac"
alt="Before" width="200"><br>
      <sub>Before</sub>
    </td>
    <td align="center">
<img
src="https://github.com/user-attachments/assets/31646e9c-78f4-4252-921f-53583193868f"
alt="After" width="200"><br>
      <sub>After</sub>
    </td>
  </tr>
</table>

Two final observations worth noting:
- The fix proposed in this PR seems to **supersede** the previous PR's
solution; it addresses both cases (overshooting and undershooting) even
in my tests with the [original PR's changes
](https://github.com/flutter/flutter/pull/151026/files#diff-23a4bb073009d89f09084bdf5f85232de135b8f11be625e6312bb85900a90e67)
reverted. Probably best to keep the old PR's code anyway to be
conservative, but noteworthy.
- I also found it notable that neither this PR nor the older PR fix any
issue with "reversed lists", which, in my tests, are simply not subject
to this edge case as we've defined it. The regression tests added for
the reverse case are thus purely precautionary.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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.md
IvoneDjaja pushed a commit to IvoneDjaja/flutter that referenced this pull request Nov 22, 2025
…animation (flutter#173241)

When rapidly dragging items in a ReorderableList before animations
complete, items would jump to their expected positions rather than
smoothly transitioning.

## Problem

<p align="center">
<img
src="https://github.com/user-attachments/assets/0efb250c-2960-4942-959f-59eccc20cefb"
alt="demo2" width="250">
</p>

The issue occurs when a reorder animation is interrupted by starting a
new drag operation. The interrupted animation would reset to the
starting position of the original animation rather than capturing the
current animated position, causing a visual jump.

## Solution

This PR fixes the issue by:
1. Storing the previous target offset before updating to a new target
2. When an animation is interrupted, calculating the actual current
position based on the animation's progress
3. Using this calculated position as the new starting point for the next
animation

<p align="center">
<img
src="https://github.com/user-attachments/assets/c24e4834-1c6b-41c1-8f44-17b4c79e0993"
alt="demo2" width="250">
</p>

This PR is part of a series of `ReorderableList` enhancements I've
developed for [TimeFinder](https://timefinder.app):
- flutter#172740
- flutter#172380
- flutter#172739
- flutter#172738

Fixes flutter#173243

## Code Changes

```dart
// Before
_startOffset = offset;

// After  
final double currentAnimValue = Curves.easeInOut.transform(_offsetAnimation\!.value);
final Offset currentPosition = Offset.lerp(_startOffset, previousTarget, currentAnimValue)\!;
_startOffset = currentPosition;
```

This ensures smooth transitions without visual jumping, making rapid
reordering gestures feel more responsive and natural.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [migration
guide] as needed.
- [x] All existing and new tests are passing.
- [x] The analyzer (`flutter analyze --flutter-repo`) does not report
any problems on my PR.
- [x] I am willing to follow-up on review comments in a timely manner.

[Contributor Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[migration guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#create-a-migration-guide
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…animation (flutter#173241)

When rapidly dragging items in a ReorderableList before animations
complete, items would jump to their expected positions rather than
smoothly transitioning.

## Problem

<p align="center">
<img
src="https://github.com/user-attachments/assets/0efb250c-2960-4942-959f-59eccc20cefb"
alt="demo2" width="250">
</p>

The issue occurs when a reorder animation is interrupted by starting a
new drag operation. The interrupted animation would reset to the
starting position of the original animation rather than capturing the
current animated position, causing a visual jump.

## Solution

This PR fixes the issue by:
1. Storing the previous target offset before updating to a new target
2. When an animation is interrupted, calculating the actual current
position based on the animation's progress
3. Using this calculated position as the new starting point for the next
animation

<p align="center">
<img
src="https://github.com/user-attachments/assets/c24e4834-1c6b-41c1-8f44-17b4c79e0993"
alt="demo2" width="250">
</p>

This PR is part of a series of `ReorderableList` enhancements I've
developed for [TimeFinder](https://timefinder.app):
- flutter#172740
- flutter#172380
- flutter#172739
- flutter#172738

Fixes flutter#173243

## Code Changes

```dart
// Before
_startOffset = offset;

// After  
final double currentAnimValue = Curves.easeInOut.transform(_offsetAnimation\!.value);
final Offset currentPosition = Offset.lerp(_startOffset, previousTarget, currentAnimValue)\!;
_startOffset = currentPosition;
```

This ensures smooth transitions without visual jumping, making rapid
reordering gestures feel more responsive and natural.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [migration
guide] as needed.
- [x] All existing and new tests are passing.
- [x] The analyzer (`flutter analyze --flutter-repo`) does not report
any problems on my PR.
- [x] I am willing to follow-up on review comments in a timely manner.

[Contributor Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[test-exempt]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[migration guide]:
https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#create-a-migration-guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

3 participants