Skip to content

[cupertino_ui] Prevent intercepting CupertinoSheet transitions mid-animation - #12515

Open
xxxOVALxxx wants to merge 1 commit into
flutter:mainfrom
xxxOVALxxx:prevent_intercepting_CupertinoSheet
Open

[cupertino_ui] Prevent intercepting CupertinoSheet transitions mid-animation#12515
xxxOVALxxx wants to merge 1 commit into
flutter:mainfrom
xxxOVALxxx:prevent_intercepting_CupertinoSheet

Conversation

@xxxOVALxxx

Copy link
Copy Markdown

Fixed an incompatibility between CupertinoSheetRoute and SwiftUI sheet, which allowed users to intercept animations for opening and closing a CupertinoSheetRoute, resulting in unexpected gesture behavior and animation failures.

This PR is a follow-up to flutter/flutter#181605

Pre-Review 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.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2

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

Copy link
Copy Markdown
Contributor

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 prevents drag gestures from interrupting the Cupertino sheet's dismissal animation by updating enabledCallback to check if the route controller is currently animating, and adding a check for enabledCallback in _CupertinoDraggableScrollableSheetState._dragStart. A widget test is also added to verify this behavior. The reviewer suggests also checking widget.enabledCallback() in _handleDragStart within _CupertinoDragGestureDetectorState to handle cases where an animation starts after a pointer is downed but before the drag gesture begins.

topGap: topGap,
child: _CupertinoDragGestureDetector<T>(
enabledCallback: () => enableDrag,
enabledCallback: () => enableDrag && !(route.controller?.isAnimating ?? false),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While enabledCallback is now correctly updated to check if the route is animating, _CupertinoDragGestureDetectorState only checks enabledCallback() during onPointerDown (line 1059).

If a programmatic animation starts after the pointer is downed but before the drag gesture actually starts (i.e., before the pointer moves enough to trigger onStart), _handleDragStart will still be called and will initiate the pop gesture, potentially intercepting the animation.

To make this fully robust and consistent with _CupertinoDraggableScrollableSheetState (which checks enabledCallback in _dragStart), consider also checking widget.enabledCallback() in _handleDragStart and handling a null _dragGestureController in _handleDragUpdate and _handleDragEnd.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added in the following PR #12516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: cupertino_ui triage-design Should be looked at in design triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant