Skip to content

Conversation

@MitchellGoodwin
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin commented Feb 15, 2023

Addresses #119071. At large widths, the picker will expand outwards in order to fill the space and will lose the look of being a wheel. In order to avoid this, this PR constrains the max width to either our default based off of native, or a width estimated based off of the widths of the columns, if bigger than the default. Similarly, the native date picker wheel does not use up the whole width.

Before:
Screenshot 2023-02-15 at 11 04 09 AM

After:
Screenshot 2023-02-15 at 11 03 06 AM

A native iOS date picker wheel (left) and the new Flutter CupertinoDatePicker unbounded in an app. Both have a width that does not take up the whole available space.
Screenshot 2023-02-15 at 11 01 37 AM

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.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels Feb 15, 2023
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #120827 at sha 7cbc11a0ffb69d31e128dd5853b27715f5890d6a

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Feb 15, 2023
@thecalamiity
Copy link
Contributor

I believe this addresses #54241 as well? Would be great to check if it fixes #110319 too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Container has an alignment args in its constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the other picker need the same treatmeant?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move the logic to _DatePickerLayoutDelegate so you don't have to copy paste it.

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 moved the majority of the logic to _DatePickerLayoutDelegate. 👍

@MitchellGoodwin MitchellGoodwin force-pushed the cupertino-date-picker-width branch 2 times, most recently from ccaae2d to e6ff872 Compare February 28, 2023 19:01
@MitchellGoodwin MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2023

// The wheel does not bend outwards at large widths.
expect(
distance >= (octoberX - decemberX),
Copy link
Member

Choose a reason for hiding this comment

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

This test didn't actually work because of a bug - fixed it here https://github.com/flutter/flutter/pull/121342/files#r1148615910

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants