Made insetPadding configurable for Date Picker Dialog #155651

sarbagyastha · 2024-09-25T05:33:57Z

This PR adds following properties to the DatePickerDialog:

insetPadding

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

…ter into date-picker-on-changed

ahyangnb · 2024-09-25T06:49:36Z

favor

sarbagyastha · 2024-09-27T00:53:32Z

CC @MitchellGoodwin

MitchellGoodwin

Code LGTM. But I'm wondering about the use cases of these. The date values are exposed by the future returned by the dialog being popped off the screen. If another callback exposes the date, then there are conflicting ways to get those values from the widget. I'm worried that exposing these callbacks will create anti-patterns where it's easier to introduce bugs.

justinmc

I would split out the insetPadding change from the onStart/EndDateChanged change. It's totally independent, right? I think the insetPadding change looks like it's probably easy to get landed on its own.

I'm also curious about how the new onStart/EndDateChanged parameters will be used. Is there a case where it's not possible to use the Future to get the selected date range?

MitchellGoodwin

I think before merging we need a clear use case for this. Because this is going to introduce a different way to get the date value along with what we had before, it feels off. It may be possible that the problem this is trying to fix could be solved by something else.

…ter into date-picker-on-changed

sarbagyastha · 2024-10-24T05:46:11Z

I think before merging we need a clear use case for this. Because this is going to introduce a different way to get the date value along with what we had before, it feels off. It may be possible that the problem this is trying to fix could be solved by something else.

Possible Use Case: Let's say we use the DatePickerDialog in the model route that show some kind of processed information based on the selected date(before confirming).

showDialog:
  Column:
    DatePickerDialog(onChanged: (date) => selectedDate = date)
    NotAvailableReasonMessageWidget(selectedDate)

But yeah, may be instead of adding the callbacks to the dialog, we can expose a new widget that composes of the Calendar and Input date picker, and have the callbacks in it. Then, use the widget to build DatePickerDialog without exposing the callbacks.

sarbagyastha · 2024-10-24T05:54:29Z

I would split out the insetPadding change from the onStart/EndDateChanged change. It's totally independent, right? I think the insetPadding change looks like it's probably easy to get landed on its own.

For now, I removed the callbacks from the PR.

MitchellGoodwin

LGTM! Thank you for the PR.

justinmc

LGTM 👍 Thanks for the PR!

auto-submit · 2024-10-30T17:51:14Z

auto label is removed for flutter/flutter/155651, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

justinmc · 2024-10-30T17:52:19Z

Ah I missed the Google testing failure. @sarbagyastha can you push a merge commit? I see no failures internally.

…r/flutter#155651)

flutter/flutter@0fe6153...f86b777 2024-11-01 [email protected] Roll Packages from 7cc1caa to 796afa3 (15 revisions) (flutter/flutter#158003) 2024-11-01 [email protected] Marks Linux_pixel_7pro service_extensions_test to be flaky (flutter/flutter#157853) 2024-11-01 [email protected] Roll Flutter Engine from 0a0d5c9be6ff to 3a090b46dd35 (1 revision) (flutter/flutter#157994) 2024-11-01 [email protected] Roll Flutter Engine from bacc5e1e73b7 to 0a0d5c9be6ff (3 revisions) (flutter/flutter#157991) 2024-11-01 [email protected] Add test for `interactive_viewer.transformation_controller.0.dart` (flutter/flutter#157986) 2024-11-01 [email protected] Roll Flutter Engine from d7e928911ac2 to bacc5e1e73b7 (1 revision) (flutter/flutter#157982) 2024-11-01 [email protected] Add test for `notification.0.dart` (flutter/flutter#157909) 2024-11-01 [email protected] performance: Override .elementAt in CachingIterable (flutter/flutter#152477) 2024-11-01 [email protected] Roll Flutter Engine from cd46383cd55e to d7e928911ac2 (4 revisions) (flutter/flutter#157978) 2024-11-01 [email protected] Roll Flutter Engine from bb77cf867aef to cd46383cd55e (11 revisions) (flutter/flutter#157972) 2024-11-01 [email protected] Add a warning/additional handlers for parsing`synthetic-package`. (flutter/flutter#157934) 2024-10-31 [email protected] Roll Flutter Engine from f2154ef3e31c to bb77cf867aef (6 revisions) (flutter/flutter#157960) 2024-10-31 [email protected] Renames `injectBuildTimePluginFilesForWebPlatform` and removes unused named parameter. (flutter/flutter#157944) 2024-10-31 [email protected] [flutter_driver] use mostly public screenshot API. (flutter/flutter#157888) 2024-10-31 [email protected] Made insetPadding configurable for Date Picker Dialog (flutter/flutter#155651) 2024-10-31 [email protected] Fix showSnackBar can't access useMaterial3 from the theme (flutter/flutter#157707) 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

…r/flutter#155651)

sarbagyastha added 7 commits September 25, 2024 07:43

added onDateChanged callback

7b7d01c

added insetPadding to DatePickerDialog

1f65a43

added onStartDateChanged and onEndDateChanged

0689a2b

made consistent with handle start end date change

1df811b

added test for onDateChanged

cf6d91e

added tests for date picker dialog

07d90b0

added test for date range picker dialog

b70be53

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Sep 25, 2024

sarbagyastha added 4 commits September 25, 2024 11:19

Merge branch 'master' into date-picker-on-changed

a7581de

minor fix

a061ec5

Merge branch 'date-picker-on-changed' of github.com:sarbagyastha/flut…

dd5374d

…ter into date-picker-on-changed

fixed indent

9f50180

Piinks requested a review from MitchellGoodwin September 25, 2024 18:22

Merge branch 'master' into date-picker-on-changed

a9d333a

MitchellGoodwin reviewed Oct 3, 2024

View reviewed changes

Piinks requested a review from MitchellGoodwin October 16, 2024 18:28

justinmc reviewed Oct 17, 2024

View reviewed changes

MitchellGoodwin requested changes Oct 22, 2024

View reviewed changes

sarbagyastha added 3 commits October 24, 2024 11:19

revert adding callbacks

f14b82d

Merge branch 'date-picker-on-changed' of github.com:sarbagyastha/flut…

5c4badb

…ter into date-picker-on-changed

Merge branch 'master' into date-picker-on-changed

b26d795

sarbagyastha changed the title ~~Added date selection callback to Date Picker Dialogs~~ Made insetPadding configurable for Date Picker Dialog Oct 24, 2024

MitchellGoodwin approved these changes Oct 24, 2024

View reviewed changes

MitchellGoodwin requested a review from justinmc October 24, 2024 16:51

MitchellGoodwin mentioned this pull request Oct 24, 2024

Add date selection callbacks to the DatePickerDialog and DateRangePickerDialog #155650

Open

justinmc approved these changes Oct 30, 2024

View reviewed changes

justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2024

Merge branch 'master' into date-picker-on-changed

2925ebb

MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2024

auto-submit bot merged commit 19d8fbc into flutter:master Oct 31, 2024
74 checks passed

engine-flutter-autoroll mentioned this pull request Nov 1, 2024

Roll Flutter from 0fe615343bd9 to 2da2332ad1be (14 revisions) flutter/packages#7996

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2024

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

85a3f5b

…r/flutter#155651)

engine-flutter-autoroll mentioned this pull request Nov 1, 2024

Roll Flutter from 0fe615343bd9 to f15913654977 (15 revisions) flutter/packages#7999

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2024

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

e201d97

…r/flutter#155651)

engine-flutter-autoroll mentioned this pull request Nov 1, 2024

Roll Flutter from 0fe615343bd9 to f86b77721524 (16 revisions) flutter/packages#8000

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 1, 2024

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

a396e10

…r/flutter#155651)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

924577f

…r/flutter#155651)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

6f1898f

…r/flutter#155651)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

82a548b

…r/flutter#155651)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

c2cc477

…r/flutter#155651)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

19d8fbc Made insetPadding configurable for Date Picker Dialog (flutte…

ae494f1

…r/flutter#155651)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Made insetPadding configurable for Date Picker Dialog #155651

Made insetPadding configurable for Date Picker Dialog #155651

Uh oh!

sarbagyastha commented Sep 25, 2024 •

edited

Loading

Uh oh!

ahyangnb commented Sep 25, 2024

Uh oh!

sarbagyastha commented Sep 27, 2024

Uh oh!

MitchellGoodwin left a comment

Uh oh!

justinmc left a comment

Uh oh!

MitchellGoodwin left a comment

Uh oh!

sarbagyastha commented Oct 24, 2024 •

edited

Loading

Uh oh!

sarbagyastha commented Oct 24, 2024

Uh oh!

MitchellGoodwin left a comment

Uh oh!

justinmc left a comment

Uh oh!

auto-submit bot commented Oct 30, 2024

Uh oh!

justinmc commented Oct 30, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Made insetPadding configurable for Date Picker Dialog #155651

Made insetPadding configurable for Date Picker Dialog #155651

Uh oh!

Conversation

sarbagyastha commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

ahyangnb commented Sep 25, 2024

Uh oh!

sarbagyastha commented Sep 27, 2024

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

sarbagyastha commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarbagyastha commented Oct 24, 2024

Uh oh!

MitchellGoodwin left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Oct 30, 2024

Uh oh!

justinmc commented Oct 30, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sarbagyastha commented Sep 25, 2024 •

edited

Loading

sarbagyastha commented Oct 24, 2024 •

edited

Loading