Repository navigation
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces DateInputCalendarDelegate, which allows for customizable date input formatting in showDatePicker and showDateRangePicker. The changes include a new example demonstrating its usage, updates to core date picker files to integrate the new delegate, and corresponding tests. The implementation appears solid, providing a flexible extension point for date input.
787eb5a to
25522f5
Compare
4cba849 to
4583052
Compare
4583052 to
c9c9dd2
Compare
dkwingsmt
left a comment
There was a problem hiding this comment.
Sorry for the late feedback.
I have questions about the API design. I think it would be a better idea so that the inputFormatters be a property of DatePicker instead of the delegate. A couple of reasons:
- Runtime type check like this signals bad class design, which I'd prefer avoiding if possible.
- Semantically, non-gregorian calendars will also want to format their text.
- It also feels unnecessary that people have to extend the entire delegate to define a new formatter. The two aspects feel independent.
Otherwise, great code quality, and fantastic example app. Thank you!
Those points are definitely valid. The use of a delegate was mainly because That said, I could refactor this into a class where I pass the |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a DateInputDelegate to allow custom date input formatting, parsing, and validation for showDatePicker and showDateRangePicker. A new example demonstrates how to use this delegate for configurable date input formats and separators. The review comments suggest updating deprecated FilteringTextInputFormatter.digitsOnly to FilteringTextInputFormatter.allow(RegExp(r'[0-9]')) in both the example and tests, adding missing documentation comments for the dateInputDelegate fields in DatePickerDialog and _InputDateRangePicker as per the style guide, and correcting a typo in the test delegate's help text from aaaammdd to yyyymmdd.
nate-thegrate
left a comment
There was a problem hiding this comment.
This looks really good!
Quick heads-up: I currently don't have commit access in this repo so I won't be able to give an approval.
If you're interested in making progress more quickly, I have a suggestion—if you have a chance, would you be able to look through #184283? It'd be super helpful if you could describe what's different between this PR and that one, so we can decide which PR to move forward with.
Originally, when I opened this PR back in mid-January, my idea was to introduce a new So instead, I introduced I’m still exploring a way to avoid requiring users to create a full custom formatter and extend the delegate just for formatting, but that should come after further decoupling. |
|
I agree. The tight coupling between formatter and parser means more work is needed than this, but this is a good start - and a much needed design for the best customizability. In the future we can discuss other ways that provide common presets based on this API. |
dkwingsmt
left a comment
There was a problem hiding this comment.
The implementation looks good to me, but I found two problems while local testing the example (I understand that the logic here is really delicate and complicated).
Overall really good quality. Thank you very much for your contribution!
| final String formatted = _applyMask(digits); | ||
| return TextEditingValue( | ||
| text: formatted, | ||
| selection: TextSelection.collapsed(offset: formatted.length), |
There was a problem hiding this comment.
This offset will cause the text cursor to always be moved to the end of the text, which isn't expected if the user is editing at the middle of the string, which is reproducible in my local testing.
The following video recording includes two segments:
- The first half shows pressing Delete in the middle of a string that's not immediately after a separator.
- The second half shows pressing Delete in the middle of a string that's immediately after a separator.
Screen.Recording.2026-04-01.at.2.06.00.PM.mov
|
I can confirm that editing in the middle of the string no longer resets the caret to the end of the string, although pressing backspace immediately after a separator (for example, pressing backspace at |
|
Thanks for your contribution! This PR has reached completion, however we're not able to merge this PR at the moment due to the Material/Cupertino code freeze (#184093). As soon as the decoupling is complete, we will post a way to port this PR to the new packages to land. In the meantime, I'll convert this PR to a draft and apply a label so that we can quickly find it when the freeze is lifted. |
|
I've marked this PR as not ready to port to flutter/packages yet. |
|
material_ui and cupertino_ui are now accepting contributions. :) This PR can now be ported over to flutter/packages! Guidance and instructions for easily porting over can be found in #188444 |
|
@dkwingsmt Ported to flutter/packages#12608. It will stay in draft due to the 2 PR limit. |
…te (#12608) This PR adds the `keyboardInputFormatters` method to `CalendarDelegate`. Previously, date input was strictly coupled to `MaterialLocalizations`. This change allows developers to provide a list of `TextInputFormatter`s via a custom `CalendarDelegate` to enforce specific input masks (e.g., dd/MM/yyyy) or restrict character entry in date pickers (`showDatePicker`, `showDateRangePicker`, and `InputDatePickerFormField`). Fixes flutter/flutter#83302 Fixes flutter/flutter#97134 Fixes flutter/flutter#62401 Fixes flutter/flutter#181043 Fixes flutter/flutter#115151 Fixes flutter/flutter#191639 Fixes partially flutter/flutter#150611 Port of flutter/flutter#181105 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
This PR adds the
keyboardInputFormattersmethod toCalendarDelegate.Previously, date input was strictly coupled to
MaterialLocalizations. This change allows developers to provide a list ofTextInputFormatters via a customCalendarDelegateto enforce specific input masks (e.g., dd/MM/yyyy) or restrict character entry in date pickers (showDatePicker,showDateRangePicker, andInputDatePickerFormField).Fixes #83302
Fixes #97134
Fixes #62401
Fixes #181043
Fixes #115151
Fixes partially #150611
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.