It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

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.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@Jaineel-Mamtora If possible, can you add before/after screenshots to make the review easier?

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 #173706 at sha b517118

Golden file changes are available for triage from new commit, Click here to view.

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 #173706 at sha a301cf5

@Jaineel-Mamtora Thanks for your effort. However, the baseline of the text does not fit. Now, the separator is below the text baseline. I think that fixing this issue is not just setting the stretch value.

Also, keep in mind that other languages have other separators, like . or h.

@Jaineel-Mamtora Thanks for your effort. However, the baseline of the text does not fit. Now, the separator is below the text baseline. I think that fixing this issue is not just setting the stretch value.

Also, keep in mind that other languages have other separators, like . or h.

@ManuelRauber, thanks for pointing them out. I'll try to fix the baseline issue.

Further, upon investigating, it seems like the baselines in the Material 3 design docs for the input and the separator are inconsistent (pardon me, if that's not the case). Do we want to make the baseline consistent?

Please check below screenshots for reference:

Fig. 1: Material 3 Time Picker as per docs

Fig. 2: My perception of consistent baseline for Material 3 Time Picker

Please confirm which one should be considered as consistent.

Golden file changes are available for triage from new commit, Click here to view.

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 #173706 at sha a11088c

Golden file changes are available for triage from new commit, Click here to view.

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 #173706 at sha c78da92

@Jaineel-Mamtora Thanks for your effort. However, the baseline of the text does not fit. Now, the separator is below the text baseline. I think that fixing this issue is not just setting the stretch value.
Also, keep in mind that other languages have other separators, like . or h.

@ManuelRauber, thanks for pointing them out. I'll try to fix the baseline issue.

Further, upon investigating, it seems like the baselines in the Material 3 design docs for the input and the separator are inconsistent (pardon me, if that's not the case). Do we want to make the baseline consistent?

Please check below screenshots for reference:

Fig. 1: Material 3 Time Picker as per docs

Fig. 2: My perception of consistent baseline for Material 3 Time Picker

Please confirm which one should be considered as consistent.

Indeed, according to the specs, at least in that screenshot, its also off. But since it's off in all of the screenshots, it might just be the intended way.

My personal taste would be figure 2, the real baseline. But since Flutter material should respect the material design docs, it may be better to go for figure 1 instead.

Indeed, according to the specs, at least in that screenshot, its also off. But since it's off in all of the screenshots, it might just be the intended way.

My personal taste would be figure 2, the real baseline. But since Flutter material should respect the material design docs, it may be better to go for figure 1 instead.

Got it. Then, I think my current change should suffice. I checked for remaining separators, and the UI is looking consistent for them all. Please find below the screenshot for the separators:

Fig 1: hour-min '.' separator

Fig 2: hour-min 'h' separator

Fig 3: hour-min ':' separator in 24 hours format

I checked in Android (API-35), macOS, iOS (16 and 16 Pro Max). The UI looks consistent as shown above. Also, all the test cases are passing.

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.

Gentle nudge @MitchellGoodwin (from triage)

@Jaineel-Mamtora thank you for putting together this PR! Apologies for the late review. The approach looks reasonable, can you resolve the merge conflicts and rebase? The linux analyze check looks unrelated. Once it's all green again, I'm going to run Google testing on this PR just to see the golden file tests for Google apps using Flutter. I suspect that there might be some failures there that might show some odd behavior with real life use cases that we would want to address here. We may want to add some padding to make sure that the stretch doesn't stretch too far.

My sincere apologies 🥲. I was trying to merge the upstream changes in my forked branch. But, accidentally rebased it. I am a bit new to contributing to large repositories. Please let me know, in case I have wrongfully made any changes to any part of this codebase.

No worries, it happens. And it seems like you fixed it. A lot of the time it's best to make a new PR a new branch with the changes, but that doesn't look necessary here. I removed the extra reviewers that got added.

Golden file changes are available for triage from new commit, Click here to view.

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 #173706 at sha ce8eb81

@Jaineel-Mamtora change LGTM, but I'm unable to rebase through github, as github says there is a conflict, though I do not see one. Could you try rebasing again? If it continues to act up, it might be easier to create a new branch with the changes, open a new PR, and request my review on that one.

From the screenshot attached, it seems the Hour and Minute box get wider. Could you help check if their widths are still 96? This is the specs: https://m3.material.io/components/time-pickers/specs#f2ce126b-9675-43a7-9314-b62eec57e2bd

Sure, let me check it today and I'll update.

Hi @QuncCccccc,

I checked, for the default locale, it has the same dimensions as mentioned in the material design docs. Please check below screenshot:

In case of other locale, for example, Spanish locale (es), the Period selector container (AM/PM) goes away, so it takes the available horizontal space as shown in below screenshot:

Please let me know your thoughts on this.

In case of other locale, for example, Spanish locale (es), the Period selector container (AM/PM) goes away, so it takes the available horizontal space

I see. Thanks for checking! I think this looks okay! LGTM!

autosubmit label was removed for flutter/flutter/173706, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.