Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the LongPressDraggable 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 31, 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 test to ensure LongPressDraggable doesn't crash in a zero-size environment. The test correctly sets up the scenario with a zero-sized widget. However, it misses simulating the long press gesture, which is the actual trigger for the crash described in the associated issue. I've added suggestions to complete the test by adding the long press action and also to improve code style by using const constructors.

),
),
);
expect(tester.getSize(find.byType(LongPressDraggable<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

This test is intended to verify that LongPressDraggable doesn't crash in a zero-sized environment, as per the issue description. However, it only checks that the widget can be created with a zero size. It doesn't simulate a long press, which is the action that would trigger the crash.

To properly test the fix, you should simulate a long press gesture on the widget.

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

    // This should not crash.
    await tester.longPress(find.byType(LongPressDraggable<bool>));
    await tester.pump();

Comment on lines 130 to 131
feedback: Text('Y'),
child: Text('X'),
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better performance and to follow Dart best practices, it's recommended to use const for constructors of widgets that do not change, like Text with a constant string.

Suggested change
feedback: Text('Y'),
child: Text('X'),
feedback: const Text('Y'),
child: const Text('X'),

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Comment on lines 130 to 131
feedback: Text('Y'),
child: Text('X'),
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

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.

Since an overlay is involved, I think we should set the viewport size instead.

@victorsanni victorsanni requested a review from dkwingsmt January 9, 2026 23:57
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 12, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Jan 12, 2026
Merged via the queue into flutter:master with commit 76aa2f8 Jan 12, 2026
44 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 12, 2026
flutter/flutter@3134be8...d81cd3e

2026-01-12 47866232+chunhtai@users.noreply.github.com Migrate doc and code to use sendAnnounce (flutter/flutter#180776)
2026-01-12 engine-flutter-autoroll@skia.org Roll Packages from 51fe1d9 to e57e7f4 (3 revisions) (flutter/flutter#180846)
2026-01-12 dixita0607@users.noreply.github.com Fix: Allow dartpad embed clipboard-write permission for "Copy to clipboard" functionality (flutter/flutter#178057)
2026-01-12 engine-flutter-autoroll@skia.org Roll Dart SDK from 87fbfd5381b6 to 42fd9ef68c1a (1 revision) (flutter/flutter#180836)
2026-01-12 engine-flutter-autoroll@skia.org Roll Skia from aefdde600f1e to 487a9943210b (3 revisions) (flutter/flutter#180835)
2026-01-12 goderbauer@google.com Bump ffigen (flutter/flutter#180507)
2026-01-12 engine-flutter-autoroll@skia.org Roll Skia from d42a43daa6cf to aefdde600f1e (1 revision) (flutter/flutter#180829)
2026-01-12 ahmedsameha1@gmail.com Make sure that a LongPressDraggable doesn't crash in 0x0 environment (flutter/flutter#180408)
2026-01-12 ahmedsameha1@gmail.com Make sure that a FlutterLogo doesn't crash in 0x0 environment (flutter/flutter#180617)
2026-01-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from T_HOCpDjLgzi1SUMq... to VYeyMPe1lyCtlcl-V... (flutter/flutter#180825)
2026-01-11 engine-flutter-autoroll@skia.org Roll Skia from f39cc645b1dd to d42a43daa6cf (2 revisions) (flutter/flutter#180819)

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 stuartmorgan@google.com,tarrinneal@google.com 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

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.

3 participants