Skip to content

Conversation

@ADmad
Copy link
Member

@ADmad ADmad commented Dec 14, 2018

Refs #12817

@ADmad ADmad added this to the 3.8.0 milestone Dec 14, 2018
@ADmad ADmad changed the title 3.next - Allow easily generating browsers's native date/time inputs WIP - 3.next - Allow easily generating browsers's native date/time inputs Dec 14, 2018
@ADmad
Copy link
Member Author

ADmad commented Dec 14, 2018

Needs tests with values.

datetime-local needs values in form yyyy-MM-ddThh:mm so we probably need to add a widget for it to format the value as such. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime-local

@ADmad ADmad changed the title WIP - 3.next - Allow easily generating browsers's native date/time inputs WIP - 3.next - Allow easily generating browser's native date/time inputs Dec 14, 2018
@codecov
Copy link

codecov bot commented Dec 15, 2018

Codecov Report

Merging #12820 into 3.next will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             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
Impacted Files Coverage Δ Complexity Δ
src/View/Helper/FormHelper.php 93.96% <66.66%> (-0.26%) 389 <0> (+5)
src/I18n/RelativeTimeFormatter.php 95.52% <0%> (+0.49%) 74% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 071bc7f...de7a69e. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Dec 15, 2018

Codecov Report

Merging #12820 into 3.next will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             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
Impacted Files Coverage Δ Complexity Δ
src/View/Helper/FormHelper.php 93.96% <66.66%> (-0.26%) 389 <0> (+5)
src/I18n/RelativeTimeFormatter.php 95.52% <0%> (+0.49%) 74% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 071bc7f...de7a69e. Read the comment docs.

@ADmad
Copy link
Member Author

ADmad commented Dec 15, 2018

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
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member

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.

Copy link
Contributor

@markusramsak markusramsak Dec 22, 2018

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Member Author

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.

@ADmad ADmad closed this Dec 17, 2018
@ADmad ADmad deleted the 3.next-native-datetime branch December 17, 2018 19:53
@ADmad ADmad mentioned this pull request Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants