Skip to content

Fixes animation glitch into bottom sheet#183303

Merged
auto-submit[bot] merged 7 commits intoflutter:masterfrom
Mairramer:fix/animation-glitch-bottom-sheet
Mar 18, 2026
Merged

Fixes animation glitch into bottom sheet#183303
auto-submit[bot] merged 7 commits intoflutter:masterfrom
Mairramer:fix/animation-glitch-bottom-sheet

Conversation

@Mairramer
Copy link
Contributor

Fixes #183299

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot 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.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 6, 2026
@Mairramer Mairramer marked this pull request as ready for review March 6, 2026 17:55
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 glitch in the modal bottom sheet. The fix involves recalculating the animation curve in handleDragEnd based on the current animation progress when a drag gesture concludes, ensuring a smooth transition. A new widget test is also added to verify that the bottom sheet's position does not jump when a drag is released. However, the new test has a flaw in its implementation that needs to be addressed.

@dkwingsmt dkwingsmt self-requested a review March 9, 2026 07:06
dkwingsmt
dkwingsmt previously approved these changes Mar 9, 2026
Copy link
Contributor

@dkwingsmt dkwingsmt 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!

@dkwingsmt dkwingsmt requested a review from victorsanni March 9, 2026 19:07
victorsanni
victorsanni previously approved these changes Mar 10, 2026
@dkwingsmt dkwingsmt self-requested a review March 11, 2026 18:24
@dkwingsmt
Copy link
Contributor

The Google testing failure is a real assertion error of

  @override
  void didUpdateWidget(_ModalBottomSheet<T> oldWidget) {
    super.didUpdateWidget(oldWidget);
    assert(oldWidget.route == widget.route);

    assert(
      _curvedSheetAnimation.curve == (widget.animationStyle?.curve ?? _kModalBottomSheetCurve),
    );
    assert(
      _curvedSheetAnimation.reverseCurve ==
          (widget.animationStyle?.reverseCurve ?? _kModalBottomSheetCurve),
    );
    assert(_sheetAnimation.parent == _curvedSheetAnimation);  // This assertion failed.
  }

It seems reasonable since this PR updated _sheetAnimation.parent. However I'm concerned that this isn't caught by a unit test. Do you think you can come up with a unit test for this?

@Mairramer Mairramer dismissed stale reviews from dkwingsmt and victorsanni via 20cc414 March 13, 2026 11:34
@Mairramer
Copy link
Contributor Author

Mairramer commented Mar 13, 2026

The Google testing failure is a real assertion error of

  @override
  void didUpdateWidget(_ModalBottomSheet<T> oldWidget) {
    super.didUpdateWidget(oldWidget);
    assert(oldWidget.route == widget.route);

    assert(
      _curvedSheetAnimation.curve == (widget.animationStyle?.curve ?? _kModalBottomSheetCurve),
    );
    assert(
      _curvedSheetAnimation.reverseCurve ==
          (widget.animationStyle?.reverseCurve ?? _kModalBottomSheetCurve),
    );
    assert(_sheetAnimation.parent == _curvedSheetAnimation);  // This assertion failed.
  }

It seems reasonable since this PR updated _sheetAnimation.parent. However I'm concerned that this isn't caught by a unit test. Do you think you can come up with a unit test for this?

I removed the assert, since it doesn’t make sense in this case. In the previous PR where I introduced these asserts, I wasn’t able to test, so I added a few that seemed reasonable to validate.

@Mairramer Mairramer requested a review from victorsanni March 13, 2026 12:49
Copy link
Contributor

@dkwingsmt dkwingsmt 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 for the quick fix!

@dkwingsmt dkwingsmt added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Mar 17, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Mar 17, 2026
Merged via the queue into flutter:master with commit b2fc6c0 Mar 18, 2026
78 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 19, 2026
flutter/flutter@d117642...dd64978

2026-03-18 [email protected] Roll Skia from 2fb5fa71eb12 to f0a13e5efbad (2 revisions) (flutter/flutter#183830)
2026-03-18 [email protected] Roll Skia from ae3d36cb9e29 to 2fb5fa71eb12 (3 revisions) (flutter/flutter#183823)
2026-03-18 [email protected] Linux reuse sibling (flutter/flutter#183653)
2026-03-18 [email protected] Roll Skia from 84a180a1fa80 to ae3d36cb9e29 (4 revisions) (flutter/flutter#183812)
2026-03-18 [email protected] fix(web): handle asynchronously disposed platform views (flutter/flutter#183666)
2026-03-17 [email protected] (Test cross-imports) Remove legacy Material import from sliver_constraints_test (flutter/flutter#183351)
2026-03-17 [email protected] Fix Android Studio pluginsPath when version is unknown (do not use 0.0) (flutter/flutter#182681)
2026-03-17 [email protected] Fixes animation glitch into bottom sheet (flutter/flutter#183303)
2026-03-17 [email protected] Handle#6537 second grouped test (flutter/flutter#182529)
2026-03-17 [email protected] Add a Clarification for the docs of suggestionsBuilder of SearchAnchor (flutter/flutter#183106)
2026-03-17 [email protected] Remove obsolete null checks from style guide (flutter/flutter#181703)
2026-03-17 [email protected] [Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback (flutter/flutter#183561)
2026-03-17 [email protected] Encode source file patches as UTF-8 in the code formatter script (flutter/flutter#183761)
2026-03-17 [email protected] Fix widget inspector control layout and add safe area regression test (flutter/flutter#180789)
2026-03-17 [email protected] Reland "[Android] Add mechanism for setting Android engine flags via Android manifest (take 2)" (flutter/flutter#182522)
2026-03-17 [email protected] fix(web): fix crash in Skwasm when transferring non-transferable texture sources (flutter/flutter#183799)
2026-03-17 [email protected] Roll Skia from dba893a44d7a to 84a180a1fa80 (7 revisions) (flutter/flutter#183803)
2026-03-17 [email protected] Framework: Improve DropdownButton selectedItemBuilder assertion (flutter/flutter#183732)
2026-03-17 [email protected] Add mainAxisAlignment to NavigationRail (flutter/flutter#183514)
2026-03-17 [email protected] Update android triage process to not look at unassigned p1s every week (flutter/flutter#183805)
2026-03-17 [email protected] Adds macos impeller new gallery transition perf test. (flutter/flutter#183802)
2026-03-17 [email protected] fix(web_ui): move prepareToDraw after raster to improve concurrency and stability (flutter/flutter#183791)
2026-03-17 [email protected] [build] Generate debug info for assembly. (flutter/flutter#183425)
2026-03-17 [email protected] [web] Fix occasional failure to find Chrome tab (flutter/flutter#183737)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Animation glitch in bottom sheet closing

3 participants