Implement the strategy we have in #1252178: Add Modernizr to core for date inputs.

Files: 
CommentFileSizeAuthor
#38 interdiff.txt1.37 KBgoogletorp
#38 polyfill_date_input_type-1835016-38.patch5.28 KBgoogletorp
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,587 pass(es).
[ View ]
#36 polyfill_date_input_type-1835016-36.patch729.69 KBgoogletorp
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch polyfill_date_input_type-1835016-36.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#36 interdiff.txt851 bytesgoogletorp
#34 polyfill_date_input_type-1835016-34.patch5.13 KBgoogletorp
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,345 pass(es).
[ View ]
#31 interdiff.txt2.56 KBgoogletorp
#31 polyfill_date_input_type-1835016-31.patch2.56 KBgoogletorp
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch polyfill_date_input_type-1835016-31.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#24 interdiff.txt3.43 KBgoogletorp
#24 polyfill_date_input_type-1835016-24.patch5.16 KBgoogletorp
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,227 pass(es).
[ View ]
#17 polyfill_date_input_type-1835016-17.patch4.74 KBgoogletorp
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,164 pass(es).
[ View ]
#16 1835016-jquery-date-popup-16.patch3.14 KBSharique
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,912 pass(es).
[ View ]
#14 1835016-jquery-date-popup-14.patch6.64 KBSharique
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,730 pass(es).
[ View ]
#11 Screen Shot 2013-12-08 at 2.14.35 PM.png7.88 KBwebchick
#11 Screen Shot 2013-12-08 at 2.10.35 PM.png21.43 KBwebchick
#3 jquery-datepicker.patch7.4 KBKarenS
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch jquery-datepicker_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

nod_’s picture

Parts of the patch from #501428-50: Date and time field type in core. Parts of it are missing apparently, wasn't too careful during the extraction of this changes.

KarenS’s picture

StatusFileSize
new6.83 KB

That patch is missing things, it's more like this.

KarenS’s picture

StatusFileSize
new7.4 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch jquery-datepicker_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Actually that's still missing things, it's this.

rupl’s picture

Status:Postponed» Active

Marking this active since #1252178: Add Modernizr to core is pending!

KarenS’s picture

And #1252178: Add Modernizr to core is committed! I'm not quite sure how to implement this, I'm hoping someone can provide a patch or example.

rupl’s picture

I can certainly help with the Modernizr part! Since this looks to be mainly JavaScript changes, you can access the relevant test by writing a conditional like so:

if (Modernizr.inputtypes.date) {
  // Rely on native support
}
else {
  // Summon the polyfill!
}

Or if you only want to cover the NOT:

if (!Modernizr.inputtypes.date) {
  // Polyfill code
}

You can see the full list of support within your browser by going to your browser's JS console and evaluating Modernizr.inputtypes. It will return an Object full of booleans corresponding to each test result. Here's the output I got from Chrome 23.0.1271.64 on OSX:

> Modernizr.inputtypes
  Object
    color: true
    date: true
    datetime: false
    datetime-local: false
    email: true
    month: false
    number: true
    range: true
    search: true
    tel: true
    time: true
    url: true
    week: false
klonos’s picture

Mark Trapp’s picture

Also coming from there. I've been maintaining Date Popup Authored, which was borne out of that issue, for d6 and d7, but all it really does is replace the Authored On textfield element with (contrib) Date's date_popup element (plus a few extra things to ensure Drupal gets the correct input).

Looks like this pretty much obsoletes the module for D8, am I right?

KarenS’s picture

Splitting the authored on field into HTML5 date and time elements is covered in #501428: Date and time field type in core. If the polyfill is added we'll also have a fallback to the jQuery datepicker. So yes, I think this will obsolete the Date Popup Authored module.

sun’s picture

Component:javascript» datetime.module
Issue tags:+JavaScript, +html5, +polyfill
webchick’s picture

Issue summary:View changes
Priority:Normal» Major
Status:Active» Needs review
StatusFileSize
new21.43 KB
new7.88 KB

Hm. I think this (or another issue that covers this) might be major, or possibly critical...

Here's what a datetime field looks like on Chrome:

Lovely date picker

So far, so good...

But here's what it looks like in Firefox:

Two blank textfields

There's absolutely no guidance about what to do until you submit the form and get an error message. :(

Also, since there's a patch here, marking needs review.

nod_’s picture

Status:Needs review» Needs work
Related issues:+#2088383: Datetime FAPI DX

This should user Modernizr for feature detection.

And see #2088383: Datetime FAPI DX for the whole date input thing.

(and yay for collapsed text format!)

klonos’s picture

Parent issue:» #501428: Date and time field type in core
Sharique’s picture

Status:Needs work» Needs review
StatusFileSize
new6.64 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,730 pass(es).
[ View ]

Trying to add jquery popup widget for date.

Sharique’s picture

Status:Needs review» Needs work

This is not working perfectly, I wasn't expecting tests to be passed, hoping failed test might be help find right path.

Sharique’s picture

Status:Needs work» Needs review
StatusFileSize
new3.14 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,912 pass(es).
[ View ]

Here is updated patch, atleast jquery popup is working now.

googletorp’s picture

StatusFileSize
new4.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,164 pass(es).
[ View ]

I've reposted the patch from #2467351: Add jQuery UI datepicker for the Date element of the datetime field here.

I've taken a very different approach than the current patch. The idea is to add a JS fallback to all date fields instead of creating a separate widget for it.

GaëlG’s picture

Status:Needs review» Reviewed & tested by the community

#17 works well on Firefox, and Chrome behavior is unchanged (Chrome natively handles the input type date). We needed #2467449: jQuery UI datepicker styles broken in Seven for datepicker styling.
There is no timepicker for hours and minutes, but I guess this is another issue?

The last submitted patch, 3: jquery-datepicker.patch, failed testing.

googletorp’s picture

googletorp’s picture

nod_’s picture

Status:Reviewed & tested by the community» Needs work

Sorry not there yet. New direction looks good though.

We don't use $.each, use a filtered for loop, better yet, see below for using data attribute.
The behavior needs a detach function
I would do the format translation in the JS side, in case someone wants to override the jquery ui lib and use their own stuff that has a different time format format.
We're trying to get rid of ids in the JS, can you put the date format in a data-drupal-date-format attribute (or something with a better name), like we're doing with #states? (see states.js and the data-drupal-states attribute).

Thanks!

googletorp’s picture

Assigned:Unassigned» googletorp

@nod_ Thanks for the review, I'll look into this

googletorp’s picture

Status:Needs work» Needs review
StatusFileSize
new5.16 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,227 pass(es).
[ View ]
new3.43 KB

I've followed the guidelines in #22, putting the settings on the element and adding a detach method to the behavior.

Status:Needs review» Needs work

The last submitted patch, 24: polyfill_date_input_type-1835016-24.patch, failed testing.

The last submitted patch, 24: polyfill_date_input_type-1835016-24.patch, failed testing.

googletorp’s picture

Status:Needs work» Needs review

So test bot is finally working again - ready for review :)

nod_’s picture

Status:Needs review» Needs work

Still an eslint error:

core/misc/date.js
  22:10  error  Split 'var' declaration into multiple statements  one-var
  1. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
    @@ -267,6 +267,7 @@ public static function processDatetime(&$element, FormStateInterface $form_state
    +        '#date_date_format' => $element['#date_date_format'],

    +++ b/core/lib/Drupal/Core/Render/Element/Date.php
    @@ -37,11 +41,37 @@ public function getInfo() {
    +      $element['#attributes']['data-drupal-date-format'] = array($element['#date_date_format']);

    +++ b/core/misc/date.js
    @@ -0,0 +1,54 @@
    +          dateFormat = $(this).data('drupalDateFormat');
    ...
    +          dateFormat = dateFormat
    +            .replace('Y', 'yy')
    +            .replace('m', 'mm')
    +            .replace('d', 'dd');
    +          datepickerSettings.dateFormat = dateFormat;

    I've looked up the HTML5 spec, the format is hardcoded for the date field, no need to make it customizable or anything. We can just hardcode it

  2. +++ b/core/misc/date.js
    @@ -0,0 +1,54 @@
    +      if (Modernizr.inputtypes.date === false) {

    Let's inverse this and reduce nesting:

    if (Modernizr.inputtypes.date) {
      return;
    }
  3. +++ b/core/misc/date.js
    @@ -0,0 +1,54 @@
    +        $context.find('input[data-drupal-date-format]').each(function() {
    ...
    +        $(context).find('input[data-drupal-date-format].hasDatepicker')

    Since we don't need to have a configurable date format (the HTML5 spec say that the format is always yyyy-mm-dd) we can select input[type="date"]. It's also more in the spirit of a polyfill than relying on a data attribute.

  4. +++ b/core/misc/date.js
    @@ -0,0 +1,54 @@
    +          if ($(this).attr('min')) {

    you're reusing $(this) 5 times in the worst case. Let's put that in a variable like $input = $(this) and use that afterwards.

  5. +++ b/core/misc/date.js
    @@ -0,0 +1,54 @@
    +        $(context).find('input[data-drupal-date-format].hasDatepicker')
    +          .datepicker("destroy")
    +          .removeClass("hasDatepicker");

    Where does this class comes from? I would expect jquery ui to remove it on destroy.

    We have an api for that stuff:

    $(context).find('input[type="date"]').removeOnce('date-polyfill').datepicker('destroy');

    Makes me remember you need to use .once in the attach function.

googletorp’s picture

StatusFileSize
new2.56 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch polyfill_date_input_type-1835016-31.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new2.56 KB

Regarding 1 + 3

Date fields, in core or contrib might allow that you enter the date in more than one way. Eventhough the format is hardcoded in html5 date fields, the format the the end users will see is localized. This means that the JavaScript fallback will stick out to the end users.
If we want this to be usable, we need to make it flexible. As I already have made one Drupal 8 site, where I implemeted a fallback JS datepicker, I know that this is a real requirement, which is important for some site owners.

I know we can save a few lines of code by not supporting this, but seeing how few countries actually support the Y-m-d date format (http://en.wikipedia.org/wiki/Date_format_by_country) I vote for making it configurable.

I've addressed the other issues in the review.

googletorp’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 31: polyfill_date_input_type-1835016-31.patch, failed testing.

googletorp’s picture

StatusFileSize
new5.13 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,345 pass(es).
[ View ]

Seems patch from #31 was an interdiff as well - uploading the correct patch instead.

googletorp’s picture

Status:Needs work» Needs review
googletorp’s picture

StatusFileSize
new851 bytes
new729.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch polyfill_date_input_type-1835016-36.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I fixed a few lint issues, now I'm finally using the proper Drupal ESLint conf.

Status:Needs review» Needs work

The last submitted patch, 36: polyfill_date_input_type-1835016-36.patch, failed testing.

googletorp’s picture

Status:Needs work» Needs review
StatusFileSize
new5.28 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,587 pass(es).
[ View ]
new1.37 KB

Something went haywire in the last patch, so fixed that. Also fixed array using the short syntax instead.