Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the Draggable widget.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Dec 30, 2025
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 adds a widget test to ensure that Draggable widgets do not crash when placed in a zero-sized area. The test correctly sets up the scenario, but it does not trigger a drag gesture, which is necessary to reproduce the crash described in the associated issue. I've suggested an addition to the test to initiate a drag gesture, which will properly verify the fix.

),
),
);
expect(tester.getSize(find.byType(Draggable<bool>)), Size.zero);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The test sets up the zero-sized Draggable correctly, but it doesn't attempt to drag it. The crash described in issue #6537 occurs when a drag is initiated. To properly test the fix, we should simulate a drag gesture on the Draggable.

    expect(tester.getSize(find.byType(Draggable<bool>)), Size.zero);

    // Start a drag gesture to ensure it doesn't crash.
    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(Draggable<bool>)));
    await gesture.up();

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this isn't true and the test is fine as-is, right?

@justinmc justinmc requested a review from victorsanni December 30, 2025 23:13
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

),
),
);
expect(tester.getSize(find.byType(Draggable<bool>)), Size.zero);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this isn't true and the test is fine as-is, right?

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 5, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Jan 6, 2026
Merged via the queue into flutter:master with commit abbb28f Jan 6, 2026
44 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants