Skip to content
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

Add support for html5 native time-related widgets #142

Merged
merged 8 commits into from Mar 20, 2013

Conversation

lmctv
Copy link
Contributor

@lmctv lmctv commented Jan 7, 2013

  • for the DateInputWidget, the .datepicker interfered with
    the native widget shown on field enter
    • the DateTimeInputWidget's template defined the input field
      as being of type "text" instead of "datetime"; therefore,
      the native datitime widget was never shown

To enable the correct behaviour, the jqueryui datepicker will be
activated only after detecting the native support for the respective
input type is missing via a custom generated modernizr.js.

The installed version of modernizr.js was generated enabling only detection
of input types and attributes; this way, the filesize is just 2394 bytes,
instead of 15483 of the full minified distribution.

I'm attaching the screenshots I've taken both from desktop chrome:

chrome/desktop, date input: both native and jquery "enhanced" visible
chrome-both-native-and-jquery-date

chrome/desktop, date input: correct behaviour: only native visible
chrome-native-date-ok

chrome/android, date input: both native and jquery "enhanced" visible
date-android-both-native-and-jqueryui

chrome/android, date input: correct behaviour: only native visible
date-android-native

chrome/android, datetime input: only jquery dialog visible
datetime-android-no-native

chrome/android, datetime input: correct behaviour: only native visible
datetime-android-native

  * for the DateInputWidget, the .datepicker interfered with
    the native widget shown on field enter

  * the DateTimeInputWidget's template defined the input field
    as being of type "text" instead of "datetime"; therefore,
    the native datitime widget was never shown

To enable the correct behaviour, the jqueryui datepicker will be
activated only after detecting the native support for the respective
input type is missing via a custom generated modernizr.js.

The installed version of modernizr.js was generated enabling only detection
of input types and attributes; this way, the filesize is just 2394 bytes,
instead of 15483 of the full minified distribution.
for both DateInputWidget and DateTimeInputWidget:

 * default rendering as html5 input with jquery fall-back
 * if prefer_jquery is set, render as jquery-enhanced text input
@lmctv
Copy link
Contributor Author

lmctv commented Jan 11, 2013

@kiorky, @rbu, @tonthon would you mind taking a look at these changes?

and use that preference to enable native rendering of date and datetime
@@ -14,8 +16,10 @@
<script type="text/javascript">
deform.addCallback(
'${oid}',
function(oid) {
$('#' + oid).datepicker(${options});
function(oid) {<span tal:condition="prefer_html5" tal:omit-tag="">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using an actual tag, and then suppressing it with 'tal:omit-tag', a clearer spelling is to use a synthetic tag in the 'tal:' namespace. E.g.:

function(oid){<tal:if tal:condition="prefer_html5">

@lmctv
Copy link
Contributor Author

lmctv commented Jan 29, 2013

At least in the chrome for android case, correct handling of html datetime input depends on
Pylons/colander#79 "Iso8601 truncation".

@lmctv
Copy link
Contributor Author

lmctv commented Jan 30, 2013

I've added two new views to my fork of deformdemo to show the native widgets in action. While I'm not doing a pull request for the changes before this one gets integrated, the changes are at https://github.com/lmctv/deformdemo/branches/test_jquery_replace_for_html5 .

@mcdonc
Copy link
Member

mcdonc commented Jan 30, 2013

For the record, I took a look at this branch recently (while onsite at a customer gig, because we need the functionality). I'm not sure I can give very actionable feedback because I was (and am) in a hurry, but I know that I'm maybe not a fan of the conditionals in "if prefer_html5" sections in the code.

It feels like deform should just render html5 widgets by default except for cases where browser support is all over the map (like datetime and date, are there others that have spotty/weird support amongst browsers?).

Any thoughts?

@mcdonc
Copy link
Member

mcdonc commented Jan 30, 2013

Uh crap I probably attached that comment to the wrong issue, given that this one is just about datetime support. And maybe I'm misremembering, but it seemed like when I looked the code anticipated "prefer_html5" for all widgets.

@lmctv
Copy link
Contributor Author

lmctv commented Jan 30, 2013

No, no, the issue is the right one... I just thought adding an opt-in functionality would have been the path of least resistence for adding html5 widgets one by one; if you prefer so, I could very well drop the conditional, but I fear possible browser compatibility problems, and the risk to have deform create FrankenForms, that could fail validation for any and all html versions...

@chrisrossi
Copy link
Member

I'll weigh in in support of @mcdonc. It would discourage use of 'prefer_html5' and rely on modernizr to do the right thing. Is there ever a time when being able to turn off html5 would be necessary?

@lmctv
Copy link
Contributor Author

lmctv commented Mar 19, 2013

With many thanks to @chrisrossi... no more "prefer_html5" pollution; let those who really want to avoid native widgets just tell it to the renderer by explicitly asking for a type_name != 'date' or 'datetime' on widget instantiation.

@mcdonc mcdonc merged commit e21f9ae into Pylons:master Mar 20, 2013
@mcdonc
Copy link
Member

mcdonc commented Mar 20, 2013

Woo hoo! Merged. Thanks @lmctv for the patch and thanks to @ppaez for doing some testing before the merge.

@lei12
Copy link

lei12 commented Feb 6, 2014

I'm facing the non-displaying Datepicker widget issue in Firefox, and the js script related to this Deform date field fails with

ReferenceError: Modernizr is not defined

I installed Deform 2 months ago, so what should I do about it ?

@lmctv
Copy link
Contributor Author

lmctv commented Feb 23, 2014

@lei12 are you sure you are correctly exposing the deform/static/ path to your web client? the required modernizr implementation is in static/scripts/modernizr.custom.input-types-and-atts.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants