-
Notifications
You must be signed in to change notification settings - Fork 3.4k
WIP - 3.next - Allow easily generating browser's native date/time inputs #12820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Needs tests with values.
|
Codecov Report
@@ Coverage Diff @@
## 3.next #12820 +/- ##
============================================
- Coverage 90.42% 90.41% -0.01%
- Complexity 14033 14038 +5
============================================
Files 522 522
Lines 36105 36113 +8
============================================
+ Hits 32647 32653 +6
- Misses 3458 3460 +2
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## 3.next #12820 +/- ##
============================================
- Coverage 90.42% 90.41% -0.01%
- Complexity 14033 14038 +5
============================================
Files 522 522
Lines 36105 36113 +8
============================================
+ Hits 32647 32653 +6
- Misses 3458 3460 +2
Continue to review full report at Codecov.
|
|
This is ready for some feedback (Ignore missing/incorrect docblocks). |
| * This class is intended as an internal implementation detail | ||
| * of Cake\View\Helper\FormHelper and is not intended for direct use. | ||
| */ | ||
| class DateTimeNativeWidget implements WidgetInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should just make native date inputs the default for 4.0. HTML5 date input support has gotten much better with evergreen browsers and not having to support multiple selectboxes would let us save some complexity internally as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with making native datetime inputs default in 4.0 and dropping the selects altogether. In practice most sites/apps use JS based pickers anyway for better consistency and usability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should retain the array marshalling for datetimes for backwards compatibility. I bet somewhere someone is using array dates in an API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping datetime array marshaling sounds reasonable.
So should I reopen this against 4.x instead, with the helper and widget code for selects removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is a good idea. We could move the old widgets into a plugin that folks can use if they really need that behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer using dropdown to select the year for birthday, which can allow to select "9999" for the year if someone prefers not to say his year of birth. Therefore for birthday I like the select dropdown version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this as opt-in sounds reasonable for 3.x.
Also keeping the old one (if we switch over), even for 4.x, as opt-in would be nice.
Many still use that actually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like using Tiny Datepicker (https://github.com/chrisdavies/tiny-date-picker) where possible but my clients prefer picking the year of birth in a dropdown where I can add the year option "prefer not to say" oder "unknown" in my web based contact relationship management system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think year, and month inputs could stay as select boxes. The native HTML inputs are better for date and time values though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to switch to native inputs then by default they should be used for all available types. There's no HTML input for year so keeping the select box for year is fine but there is one for month so for that native input should be used.
Refs #12817