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 [email protected] Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854)
2025-08-15 [email protected] Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848)
2025-08-15 [email protected] Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833)
2025-08-15 [email protected] Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826)
2025-08-15 [email protected] Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822)
2025-08-15 [email protected] Update the RBE configuration for the recent Clang update (flutter/flutter#173803)
2025-08-15 [email protected] Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793)
2025-08-15 [email protected] 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 [email protected] Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798)
2025-08-14 [email protected] [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782)
2025-08-14 [email protected] Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515)
2025-08-14 [email protected] Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627)
2025-08-14 [email protected] Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677)
2025-08-14 [email protected] Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757)
2025-08-14 [email protected] Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797)
2025-08-14 [email protected] Predictive back route transitions by default (flutter/flutter#165832)
2025-08-14 [email protected] feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221)
2025-08-14 [email protected] Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792)
2025-08-14 [email protected] Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148)
2025-08-14 [email protected] Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784)
2025-08-14 [email protected] [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 [email protected] Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789)
2025-08-14 [email protected] Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293)
2025-08-14 [email protected] Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790)
2025-08-14 [email protected] [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480)
2025-08-14 [email protected] Fix InputDecorator label padding (flutter/flutter#173344)
2025-08-14 [email protected] Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438)
2025-08-14 [email protected] 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 [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
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
…r#9832)

flutter/flutter@f4334d2...52af7a5

2025-08-15 [email protected] Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854)
2025-08-15 [email protected] Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848)
2025-08-15 [email protected] Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833)
2025-08-15 [email protected] Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826)
2025-08-15 [email protected] Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822)
2025-08-15 [email protected] Update the RBE configuration for the recent Clang update (flutter/flutter#173803)
2025-08-15 [email protected] Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793)
2025-08-15 [email protected] 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 [email protected] Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798)
2025-08-14 [email protected] [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782)
2025-08-14 [email protected] Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515)
2025-08-14 [email protected] Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627)
2025-08-14 [email protected] Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677)
2025-08-14 [email protected] Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757)
2025-08-14 [email protected] Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797)
2025-08-14 [email protected] Predictive back route transitions by default (flutter/flutter#165832)
2025-08-14 [email protected] feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221)
2025-08-14 [email protected] Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792)
2025-08-14 [email protected] Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148)
2025-08-14 [email protected] Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784)
2025-08-14 [email protected] [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 [email protected] Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789)
2025-08-14 [email protected] Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293)
2025-08-14 [email protected] Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790)
2025-08-14 [email protected] [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480)
2025-08-14 [email protected] Fix InputDecorator label padding (flutter/flutter#173344)
2025-08-14 [email protected] Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438)
2025-08-14 [email protected] 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 [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
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