Skip to content

[Material] Redesign Time Picker - #59191

Merged
fluttergithubbot merged 14 commits into
flutter:masterfrom
rami-a:feature-time-picker
Jun 23, 2020
Merged

fluttergithubbot merged 14 commits into
flutter:masterfrom
rami-a:feature-time-picker

Conversation

@rami-a

@rami-a rami-a commented Jun 10, 2020 •

Copy link
Copy Markdown
Contributor

Description

This is a large PR that redesigns the time picker to match the upcoming Material spec. The main changes include:

  • Redesigned header with more obvious selection for hours/minutes/am/pm
  • Redesigned 24 hour layout with a single ring of numbers instead of the double ring
  • Text input mode
  • Theming capabilities
  • Updated screenreader experience to make the picker easier to use (require less swipes)
Before After
Simulator Screen Shot - iPhone 11 Pro - 2020-06-10 at 15 21 19 Simulator Screen Shot - iPhone 11 Pro - 2020-06-10 at 15 18 34
No text input Simulator Screen Shot - iPhone 11 Pro - 2020-06-12 at 11 52 56

Related Issues

Fixes #53905
Fixes #56465

Tests

I added the following tests:

  • Updating existing tests to match new behavior
  • Adding tests for text input mode
  • Adding tests for theming

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

@fluttergithubbot fluttergithubbot added a: internationalization Supporting other languages or locales. (aka i18n) p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Jun 10, 2020

@JoseAlba JoseAlba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think if you add Dartpad examples into the TimePicker and TImePickerThemeData class it would be great. That way when someone visits the website they will be able to see the time picker from the API docs.

@rami-a

rami-a commented Jun 11, 2020

Copy link
Copy Markdown
Contributor Author

The documentation for showTimePicker already includes several examples, and it's just one function you call, so I don't think a dart pad example would add much more

https://api.flutter.dev/flutter/material/showTimePicker.html

@rami-a rami-a closed this Jun 11, 2020
@rami-a
rami-a force-pushed the feature-time-picker branch from 8e20b35 to 49dc9b1 Compare June 11, 2020 15:59
@rami-a rami-a reopened this Jun 11, 2020

@HansMuller HansMuller left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just some comments about time_picker_theme.dart

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments for time_picker.dart

Everything looks pretty good; most of the comments are just prove that I looked at everything :-).

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes LGTM. I think the color properties could be documented a little more clearly; I added a few comments suggestions.

@jgluck93

jgluck93 commented Jul 1, 2020 •

Copy link
Copy Markdown

When will this change be live? @rami-a

@Zazo032

Zazo032 commented Jul 1, 2020

Copy link
Copy Markdown
Contributor

When will this change be live? @rami-a

This is already live on Flutter's master channel

@jgluck93

jgluck93 commented Jul 1, 2020

Copy link
Copy Markdown

So I'm guessing its not in the stable channel? @Zazo032 Thank you for the fast reply.

vasilich6107 added a commit to artflutter/whatsup_flutter_june2020 that referenced this pull request Jul 6, 2020
vasilich6107 added a commit to artflutter/whatsup_flutter_june2020 that referenced this pull request Jul 6, 2020
@lalomartins

lalomartins commented Jul 31, 2020 •

Copy link
Copy Markdown

Uh, my users actually reported the 24h dial as a bug and I had to revert this locally.

(If this is not the appropriate place to discuss it, which I think is very likely, I'd be grateful if someone could point me to the right venue.)

The point of the ring mode for the time picker is to emulate an analog clock; people are used to these, and can tell at a glance what hour it's pointing to. While there is such a thing as 24h analog clocks, those are novelties, and I've never seen someone actually use one. So a 24h ring makes very unintuitive UX, and is therefore basically useless.

The PR mentions it reflects the upcoming Material spec. Is that visible anywhere public, or is it internal to Google? Last I checked, the published guidelines said a time picker ‘is not under development’.

@andrelabonte

Copy link
Copy Markdown

hi @lalomartins
Thanks for reaching out! We always appreciate feedback from the community!
One of the main goals was to improve the accessibility of selecting time, since this was a commonly reported issue with the previous design.
We are continuing to study this component design to make sure it is usable and behaves as expected for most users.
If you could share some details on the app use-cases you are targeting with the Time Picker and some info on the locale of the users, that would be very helpful.

@lalomartins

Copy link
Copy Markdown

@andrelabonte just here or do you want me to contact you by email? My address is… easy to find.

It's a private app with a very small audience, but quite international, and I'm very happy to give feedback.

The app in which I got feedback basically tracks events. It makes an usually correct guess about when it happened, but gives users the option to edit it. People who gave me feedback love the Material picker because they can type the number if they want, or pick from the dial when they prefer that, and the dial mimics an analog clock, so they know by reflex where the hours and minutes go. When I showed them a version built against Flutter dev, the only complaint I got was that the dial was no longer useful.

@lalomartins

Copy link
Copy Markdown

Now that 1.20 has been released and the time picker spec has actually been published, I can't help but make two observations:

  1. The spec doesn't actually state you should draw 24h on a single dial. Unfortunately, it also doesn't say you shouldn't, and all screenshots are in 12h mode.

  2. In ‘Principles’, it states the reason is that the dial is ‘Familiar’, which is exactly why I wanted to use it in the first place. I quote: “The analog clock face option provides an added level of assurance and confirmation when picking times.” To me, this heavily implies the 24h mode should stick to a clock layout, and keep 12h. Whether you do this by having two concentric dials as it was before, or a separate AM/PM switch, I'll have to leave to the designers; but clearly the ‘24h analog clock’ mode fails to fulfil this principle.

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: internationalization Supporting other languages or locales. (aka i18n) framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unclear how to change colors of time picker(showTimePicker) Add option to have a title in DatePicker and TimePicker