Skip to content

Conversation

@ahmedsameha1
Copy link
Contributor

This is my attempt to handle #6537 for the DatePickerDialog UI control.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 13, 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 regression test to ensure that DatePickerDialog does not crash when rendered in a zero-sized area, addressing issue #6537. The new test case is a good addition. I've suggested making the test's expectations more explicit to improve its robustness and align with testing best practices.

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

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 PR.

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 14, 2025
Merged via the queue into flutter:master with commit 3c1e258 Aug 14, 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 14, 2025
testWidgets('DatePickerDialog renders at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: SizedBox.shrink(
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Aug 15, 2025

Choose a reason for hiding this comment

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

I suspect this test isn't actually testing an 0x0 dialog, if MaterialApp gives a tight BoxConstraints to the SizedBox. Could you double check the size of the DatePickerDialog after it's laid out and see if it's indeed 0x0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

print(tester.getSize(find.byType(DatePickerDialog))); prints Size(800.0, 600.0)

Please check this: #6537 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's supposed to be 0x0 right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a Scaffold or a Center to the DatePickerDialog, and verifies in the test that the final size of the DatePickerDialog is indeed Size.zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is no need to do this, according to: #6537 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure that a DatePickerDialog doesn't crash in 0x0 environment

The test you added is to verify that DatePickerDialog doesn't crash in 0x0 environment, I think you want to make sure its size is Size.zero right? Otherwise you're just verifying the wrong thing in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sir, did you ever read that #6537 (comment)? Currently, I am working on this issue according to that comment. If you see that my pull request doesn't follow that comment, your objections are welcome; otherwise, you could show your objections in the issue itself and not here.

Copy link
Contributor

@dkwingsmt dkwingsmt Aug 22, 2025

Choose a reason for hiding this comment

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

No offense but I agree that @LongCatIsLooong has a point. It's not about verifying the final size of DatePickerDialog to be zero. It's that the current way the tests are written does not even apply a zero size constraint to the target at all, therefore nothing is being tested. The outer widget MaterialApp uses a tight constraint Size(800, 600) to its child, meaning that the child will be of size 800x600 whatsoever even with SizedBox.shrink. In other words, the SizedBox.shrink here is a noop.

It's not technically necessary to add an expect that verifies the widget size to be zero, although adding one would make it extra safer. But we really need to add something between MaterialApp and the target widget, such as:

  testWidgets('DatePickerDialog renders at zero area', (WidgetTester tester) async {
    await tester.pumpWidget(
      MaterialApp(
        home: Center(
          child: SizedBox.shrink(
            child: DatePickerDialog(firstDate: firstDate, lastDate: lastDate),
          ),
        ),
      ),
    );
    // print(tester.getSize(find.byType(DatePickerDialog)));
  });

Only by adding the extra Center can we really create a 0x0 environment, which you can manually verify with that print - or automatically verify with an extra expect.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 15, 2025
flutter/flutter@f4334d2...52af7a5

2025-08-15 engine-flutter-autoroll@skia.org Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833)
2025-08-15 engine-flutter-autoroll@skia.org Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822)
2025-08-15 jason-simmons@users.noreply.github.com Update the RBE configuration for the recent Clang update (flutter/flutter#173803)
2025-08-15 matanlurey@users.noreply.github.com Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793)
2025-08-15 matanlurey@users.noreply.github.com Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799)
2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809)
2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798)
2025-08-14 mdebbar@google.com [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782)
2025-08-14 ahmedsameha1@gmail.com Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515)
2025-08-14 ahmedsameha1@gmail.com Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627)
2025-08-14 ahmedsameha1@gmail.com Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677)
2025-08-14 robert.ancell@canonical.com Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757)
2025-08-14 matanlurey@users.noreply.github.com Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797)
2025-08-14 jmccandless@google.com Predictive back route transitions by default (flutter/flutter#165832)
2025-08-14 houssemeddinefadhli81@gmail.com feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221)
2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792)
2025-08-14 31859944+LongCatIsLooong@users.noreply.github.com Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148)
2025-08-14 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784)
2025-08-14 jhy03261997@gmail.com [Range slider] Tap on active range,  the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725)
2025-08-14 engine-flutter-autoroll@skia.org Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789)
2025-08-14 ttankkeo112@gmail.com Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293)
2025-08-14 matanlurey@users.noreply.github.com Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790)
2025-08-14 victorsanniay@gmail.com [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480)
2025-08-14 bruno.leroux@gmail.com Fix InputDecorator label padding (flutter/flutter#173344)
2025-08-14 edwinzn9@gmail.com Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438)
2025-08-14 matanlurey@users.noreply.github.com Thread sub-builders for every engine-uploading builder (flutter/flutter#173742)

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 louisehsu@google.com,stuartmorgan@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
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
WillBLogical pushed a commit to WillBLogical/packages that referenced this pull request Aug 20, 2025
flutter/flutter@f4334d2...52af7a5

2025-08-15 engine-flutter-autoroll@skia.org Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833)
2025-08-15 engine-flutter-autoroll@skia.org Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826)
2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822)
2025-08-15 jason-simmons@users.noreply.github.com Update the RBE configuration for the recent Clang update (flutter/flutter#173803)
2025-08-15 matanlurey@users.noreply.github.com Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793)
2025-08-15 matanlurey@users.noreply.github.com Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799)
2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809)
2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798)
2025-08-14 mdebbar@google.com [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782)
2025-08-14 ahmedsameha1@gmail.com Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515)
2025-08-14 ahmedsameha1@gmail.com Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627)
2025-08-14 ahmedsameha1@gmail.com Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677)
2025-08-14 robert.ancell@canonical.com Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757)
2025-08-14 matanlurey@users.noreply.github.com Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797)
2025-08-14 jmccandless@google.com Predictive back route transitions by default (flutter/flutter#165832)
2025-08-14 houssemeddinefadhli81@gmail.com feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221)
2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792)
2025-08-14 31859944+LongCatIsLooong@users.noreply.github.com Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148)
2025-08-14 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784)
2025-08-14 jhy03261997@gmail.com [Range slider] Tap on active range,  the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725)
2025-08-14 engine-flutter-autoroll@skia.org Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789)
2025-08-14 ttankkeo112@gmail.com Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293)
2025-08-14 matanlurey@users.noreply.github.com Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790)
2025-08-14 victorsanniay@gmail.com [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480)
2025-08-14 bruno.leroux@gmail.com Fix InputDecorator label padding (flutter/flutter#173344)
2025-08-14 edwinzn9@gmail.com Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438)
2025-08-14 matanlurey@users.noreply.github.com Thread sub-builders for every engine-uploading builder (flutter/flutter#173742)

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 louisehsu@google.com,stuartmorgan@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
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
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
…lutter#173677)

This is my attempt to handle
flutter#6537 for the DatePickerDialog
UI control.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants