Ability to specify which TimePickerEntryModes to use and hide switch button #85189

viralkachhadiya · 2021-06-24T05:05:28Z

Related to #85013

added new two enum dialOnly and inputOnly. when App writer choose this type of TimePickerEnteryMode, showing only that type of dialog and hide switch button. when App writer choose other mode, showing switch button .

flutter-dashboard · 2021-06-24T05:05:30Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

viralkachhadiya · 2021-06-24T05:08:40Z

@darrenaustin
@HansMuller

Can you please check my latest PR?

darrenaustin

@viralkachhadiya thanks for the contribution. Looks good, but I have a few suggestions below.

In addition, we need a test to verify that this new feature works (and to prevent someone coming along later and breaking it 😉). You can add it to the rest of the time picker tests in: packages/flutter/test/material/time_picker_test.dart. If you need some help with this, you can look at the date picker PR that was mentioned in the bug report to see how that was tested.

packages/flutter/lib/src/material/time_picker.dart

google-cla · 2021-06-30T10:39:51Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-06-30T10:58:31Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

viralkachhadiya · 2021-06-30T10:59:54Z

@darrenaustin I resolved all issue and fixed with new structure. please checkout it.

google-cla · 2021-06-30T14:24:52Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

darrenaustin · 2021-06-30T18:30:59Z

Hi @viralkachhadiya, looks like there is an issue when you merged master. It picked up a bunch of changes that weren't from you, which throws off the CLA bot. The easiest way to fix this is to rebase your changes on top of master and then force push to your branch on github:

% git pull --rebase upstream master
% git push --force origin specifying_timepickermode_switchbutton

darrenaustin

Thanks for the updates, they look good (with a suggestion below).

However we still need an added test for this feature as I mentioned in my last review.

darrenaustin · 2021-06-30T18:33:06Z

packages/flutter/lib/src/material/time_picker.dart

Instead of an assert it would be better to use a FlutterError here. Take a look for other examples that use this. Also, _entryMode in the string needs a $ in front of it, or the error message will literarily be: `Can not change entry mode from _entryMode'.

google-cla · 2021-07-01T08:25:44Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-01T08:29:28Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-01T08:40:31Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-01T08:44:24Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

viralkachhadiya · 2021-07-01T09:16:59Z

Hi @viralkachhadiya, looks like there is an issue when you merged master. It picked up a bunch of changes that weren't from you, which throws off the CLA bot. The easiest way to fix this is to rebase your changes on top of master and then force push to your branch on github:
% git pull --rebase upstream master
% git push --force origin specifying_timepickermode_switchbutton

i try it but still it has same merge issue.

google-cla · 2021-07-01T11:02:54Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-01T11:25:20Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

viralkachhadiya · 2021-07-01T13:06:40Z

@darrenaustin I did not know. Why is it happing?
Can you help me please?

google-cla · 2021-07-01T16:54:42Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-02T17:24:21Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-07-02T17:25:00Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

viralkachhadiya · 2021-07-03T05:58:43Z

@darrenaustin can you check it?

viralkachhadiya · 2021-07-22T05:52:41Z

@PixelToast @Piinks can you please review this pr?

darrenaustin

@viralkachhadiya, sorry this took me a while to take another look at this. The code looks fine as I mentioned in my last review (although it looks like you have a new change that came in from a merge that will need to be reverted).

However, you keep requesting reviews for this change when you have not yet provided a test for that ensures this change works. I have asked now twice before, once with pointers on how to go about writing such a test. Without a test we will not be able to accept this PR as per https://github.com/flutter/flutter/wiki/Tree-hygiene#tests.

darrenaustin · 2021-07-22T06:06:20Z

bin/internal/flutter_plugins.version

What does this change have to do with the TimePicker? It looks like something that came in with a merge that shouldn't have. Please revert this change.

i can't do it bcz whenever I merge my local branch with master then it is generate automatically

viralkachhadiya · 2021-08-08T15:15:43Z

@darrenaustin I added test. please review my PR.

Piinks · 2021-08-17T19:54:04Z

Hi @viralkachhadiya it looks like many tests are failing currently for this PR, can you take a look?

flutter-dashboard · 2021-09-06T19:57:16Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

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.

Piinks · 2021-09-09T20:26:18Z

This PR has not been updated in a while, so I am going to close it. If you would like to continue working on this change, please re-open it. Thank you!

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 24, 2021

google-cla bot added the cla: yes label Jun 24, 2021

3zrv approved these changes Jun 24, 2021

View reviewed changes

darrenaustin self-requested a review June 24, 2021 19:36

viralkachhadiya changed the title ~~Added new functionality in Time Picker~~ Ability to specify which TimePickerEntryModes to use and hide switch button Jun 26, 2021

darrenaustin suggested changes Jun 28, 2021

View reviewed changes

packages/flutter/lib/src/material/time_picker.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/time_picker.dart Outdated Show resolved Hide resolved

google-cla bot added cla: no and removed cla: yes labels Jun 30, 2021

darrenaustin suggested changes Jun 30, 2021

View reviewed changes

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from 728697a to c6c5da6 Compare July 1, 2021 08:29

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from c6c5da6 to 28f09c9 Compare July 1, 2021 08:40

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from 5a8c3c3 to 2260767 Compare July 1, 2021 16:54

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from 2260767 to 0b9005e Compare July 2, 2021 17:24

viralkachhadiya closed this Jul 2, 2021

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from cb569b7 to 3cfe83b Compare July 2, 2021 17:34

viralkachhadiya reopened this Jul 2, 2021

google-cla bot added cla: yes and removed cla: no labels Jul 2, 2021

darrenaustin suggested changes Jul 22, 2021

View reviewed changes

added new functionality on timepicker

af22c2d

viralkachhadiya force-pushed the specifying_timepickermode_switchbutton branch from 76a1beb to af22c2d Compare August 8, 2021 14:07

added test

86c4707

Piinks closed this Sep 9, 2021

LempereurBenjamin mentioned this pull request May 24, 2022

TimePicker : Ability to define dialOnly / inputOnly modes #104491

Merged

8 tasks

Ability to specify which TimePickerEntryModes to use and hide switch button #85189

Ability to specify which TimePickerEntryModes to use and hide switch button #85189

Uh oh!

Conversation

viralkachhadiya commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Jun 24, 2021

Uh oh!

viralkachhadiya commented Jun 24, 2021

Uh oh!

darrenaustin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

google-cla bot commented Jun 30, 2021

Uh oh!

google-cla bot commented Jun 30, 2021

Uh oh!

viralkachhadiya commented Jun 30, 2021

Uh oh!

google-cla bot commented Jun 30, 2021

Uh oh!

darrenaustin commented Jun 30, 2021

Uh oh!

darrenaustin left a comment

Choose a reason for hiding this comment

Uh oh!

darrenaustin Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

viralkachhadiya commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

viralkachhadiya commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 1, 2021

Uh oh!

google-cla bot commented Jul 2, 2021

Uh oh!

google-cla bot commented Jul 2, 2021

Uh oh!

viralkachhadiya commented Jul 3, 2021

Uh oh!

viralkachhadiya commented Jul 22, 2021

Uh oh!

darrenaustin left a comment

Choose a reason for hiding this comment

Uh oh!

darrenaustin Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

viralkachhadiya Aug 8, 2021

Choose a reason for hiding this comment

Uh oh!

viralkachhadiya commented Aug 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks commented Aug 17, 2021

Uh oh!

flutter-dashboard bot commented Sep 6, 2021

Uh oh!

Piinks commented Sep 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

viralkachhadiya commented Jun 24, 2021 •

edited

Loading

viralkachhadiya commented Aug 8, 2021 •

edited

Loading