Code

Opened 3 years ago

Closed 15 months ago

Last modified 9 months ago

#16630 closed New feature (fixed)

Support for HTML5 input types

Reported by: jonash Owned by: claudep
Component: Forms Version: master
Severity: Normal Keywords: html5
Cc: riccardo.magliocchetti@…, tgecho, poswald, dharris+django@…, dbrgn, buggystick, philipe.rp@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I attached a patch that adds support for HTML5 input types (URL, e-mail, number).

New widgets:

  • IntegerInput -- HTML5 type="number"
  • URLInput -- HTML5 type="url"
  • EmailInput -- HTML5 type="email"

Changes to fields:

  • EmailField: Uses EmailInput
  • URLField: Uses URLInput
  • IntegerField: Uses IntegerInput, adds the max="..." and min="..." HTML5 attrs if max_value/min_value are specified
  • FloatField: inherits behaviour from IntegerField and adds step="any"
  • DecimalFields: Now based on IntegerField because they share code; also maxlength="..." and step="..." are set accordingly

The second patch that is attached adapts the tests accordingly. (Some HTML attribute reordering was neccessary because of Python's dict ordering behaviour.)

Attachments (9)

html5-input-types-take1.patch (7.2 KB) - added by jonash 3 years ago.
html5-input-types-tests-take1.patch (40.9 KB) - added by jonash 3 years ago.
html5-input-types-take2.patch (7.8 KB) - added by jonash 2 years ago.
against [17281]
html5-input-types-tests-take2.patch (40.0 KB) - added by jonash 2 years ago.
against [17821]
html5-input-types-tests-take3.patch (40.8 KB) - added by jonash 2 years ago.
Against [17904]
html5-input-types-take3.patch (8.0 KB) - added by jonash 2 years ago.
Against [17904]
16630-input_type.diff (6.7 KB) - added by claudep 20 months ago.
Made input_type customizable
html5-input-types-take4.patch (52.8 KB) - added by claudep 20 months ago.
Patch, tests, docs
html5-input-types-take5.patch (52.8 KB) - added by melinath 19 months ago.
Updated patch to current master.

Download all attachments as: .zip

Change History (56)

Changed 3 years ago by jonash

Changed 3 years ago by jonash

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This is a consistent subset of #15924 which was closed because it was too broad.

The backwards compatibility concerns expressed over there still apply. Is it guaranteed that this won't cause regressions in supported browsers (especially IE >=7) ?

comment:2 Changed 3 years ago by jonash

Yep should be backwards compatible because of HTML5's strong backwards compatibility. Of course I can't guarantee anything; I'll do some IE testing though.

comment:3 Changed 3 years ago by jonash

Input types unknown to browsers (including IE7 and IE8 which I just tested) are interpreted as type="text" so this is backwards compatible as far as browsers are concerned.

comment:4 Changed 3 years ago by jonash

bump

comment:5 Changed 3 years ago by carljm

This patch would be superseded/fixed if #15667 is merged. That shouldn't block this patch, as it's not clear when (or even if, depending on performance issues) we can do #15667 - just making a note for reference.

comment:6 Changed 2 years ago by rm_

  • Cc riccardo.magliocchetti@… added

comment:7 Changed 2 years ago by tgecho

  • Cc tgecho added

Is there anything I can do to help this move forward? The patches doesn't seem to apply cleanly any more... would it be worthwhile for me to take a stab at fixing them up? Document the changes?

comment:8 Changed 2 years ago by jonash

bump

comment:9 Changed 2 years ago by julien

jonash, bumping a ticket without adding extra information isn't going to make it go faster :-)
If you think you've already answered all the questions/remarks mentioned here so far, you think the patch is ready, and you'd like to receive more feedback, then please take it to the mailing list or to #django-dev.

Changed 2 years ago by jonash

against [17281]

Changed 2 years ago by jonash

against [17821]

comment:10 Changed 2 years ago by aaugustin

  • Needs documentation set

comment:11 Changed 2 years ago by aaugustin

Paul Oswald raised some concerns about backwards-compatibility when localization is active (copy/paste from django-developers):


I have a concern regarding this.. A few times I have tried to
integrate django-floppyforms which takes a similar approach to what
the html5 widgets offer (input types are specified by default) and it
often causes pain. The reason is that while browsers say they support
html5 input types sometimes that support is lacking or very badly
implemented. As an example, if you say <input type="date"
value="2011-12-28"> the only officially supported date format is an
RFC-3339 'full-date' format (YYYY-MM-DD) according to the spec:

http://dev.w3.org/html5/markup/input.date.html#input.date.attrs.value
http://tools.ietf.org/html/rfc3339

This means that you cannot have any other format of date string in
that form field.

Now, this ticket 16630 doesn't change the date field specifically but
it does change the number field. (Is there a similar ticket for
changing the date field?) I've run into a similar problem with the
type='number' that this ticket does change. The problem I ran into was
that forms cannot easily use the THOUSANDS_SEPARATOR because it is not
a valid number. It has to be a 'float' to be valid. This means
technically you need to use the text widget for that.

http://dev.w3.org/html5/markup/datatypes.html#common.data.float

So by my thinking this patch (and by extension the thinking of
browsers and the w3) is non-backwards compatible with the way that
django formats numbers when USE_THOUSANDS_SEPARATOR is True or when
localization is turned on.

https://docs.djangoproject.com/en/dev/ref/settings/#use-thousand-separator

Maybe there is something I'm missing here? I just want to flag this as
a concern and make sure that developers know what they are getting
into by enabling that. I would be for this being the default if it
could be disabled. That way, I can use modernizr.js and turn only
certain marked fields into type="number" or type="date". At the
minimum, we would need to document that the default behavior is
changing.

comment:12 Changed 2 years ago by poswald

  • Cc poswald added

I have found that wether or not a developer will want the html5 input type="date" or type="number" feature on a given form input depends on several things:

  • the type of the data
  • the usage of the data (for example an id number might never be humanized with commas whereas an amount will be)
  • if the data needs to be localized in a way not allowed by the html5 spec
  • the capabilities of the user's browser's built-in widgets (iPad and iPod give input type="number" nice numeric keyboards)
  • if the user's browser's built-in widgets are 'better' than a javascript polyfill widget (type='date' in Safari is currently a simple up/down arrow, hardly enough for most uses)

Because the last two of these need to be determined on a per-user or per-request basis the only real solution that solves all of these is a client-side Javascript library. Unfortunately, the last one even rules out a tool like Modernizr since it reports support even when the support is terrible. That being said, some widgets (like input type='search' ) are clear wins today. For some developers, the above won't be issues and they will want html5 input type support right away.

If you're going to try to tackle it server-side, the absolutely ideal situation would be a way to allow the developer to switch date and number types on and off on a per-request basis. Second best would be switching them on and off on a per-widget basis which is what you have in this patch. For those troublesome types, the default just needs to be off with a way to switch them on for select data. Perhaps the best approach is to allow the form to pass in the type to override the default of 'text'? Something like this:

    class MyForm(forms.ModelForm):
        date = forms.DateField(label=_('Date'), type='date')
        num = forms.IntegerField(label=_('Number'), type='number')

This way, when browsers get better support for these data types, it is a simple matter of changing the default but developers can still manually override defaults.

comment:13 Changed 2 years ago by jonash

How about adding a input_type parameter to class Input(Widget) so that you can override the input type if it doesn't fit your needs?

class MyForm(forms.ModelForm):
  class Meta:
    ...
    # Use "type=date" despite the pathetic browser support:
    widgets = {'date': forms.Input(input_type='date')}

Changed 2 years ago by jonash

Against [17904]

Changed 2 years ago by jonash

Against [17904]

comment:14 Changed 2 years ago by jonash

Added an implementation of my suggestion in comment 13.

comment:15 Changed 2 years ago by dharris

  • Cc dharris+django@… added

comment:16 Changed 22 months ago by dbrgn

  • Cc dbrgn added

comment:17 Changed 22 months ago by claudep

I think that a way to go forward on this ticket would be first to factor out the new input_type parameter (code/test/docs). On question that comes to mind is if the input_type should be set on Input (then it should also be supported on Input subclasses) or on TextInput.

Version 0, edited 22 months ago by claudep (next)

comment:18 Changed 20 months ago by buggystick

  • Cc buggystick added

Changed 20 months ago by claudep

Made input_type customizable

comment:19 follow-up: Changed 20 months ago by claudep

Following my comment:17, I've factored out the input_type customizability of TextInput-based widgets from the existing patches, with tests and docs.

comment:20 Changed 20 months ago by anonymous

I think we should really default to type="number" and type="email" for number and email fields, respectively. It's much easier to type on mobile devices and doesn't really hurt in other browsers.

The main issue with date and floating point input is that it's not properly localized in most browsers. That doesn't apply to integers and emails, though.

comment:21 in reply to: ↑ 19 Changed 20 months ago by claudep

Replying to claudep:

Following my comment:17, I've factored out the input_type customizability of TextInput-based widgets from the existing patches, with tests and docs.

... and then I realized it's already possible to customize the type attribute using the attr dict, without requiring to a new input_type parameter. I may commit some test adjustments soon, then we can polish the latest patches from jonash.

comment:22 Changed 20 months ago by Claude Paroz <claude@…>

In [f1bdfbd24bcc76d21c4bf7442959bdf630ac4dec]:

Document and test 'type' usage in Widget attrs

Refs #16630.

Changed 20 months ago by claudep

Patch, tests, docs

comment:23 Changed 20 months ago by claudep

  • Needs documentation unset

After my previous commit, I updated the patch from jonash and added minimal documentation. I renamed IntegerInput to NumberInput. Final reviews welcome.

comment:24 Changed 19 months ago by jezdez

That would be hugely backwards incompatible, right? I'm -1 on this and support #15667 instead.

Also just for the record, the DecimalField in the latest patch uses manual format localization instead of the system in django.utils.formats.

comment:25 Changed 19 months ago by jonash

Backwards incompatible in what way?

I don't think #15667 and this ticket are mutually exclusive. Plus I think it doesn't make sense to postpone features that *might* be superseded by another feature in remote future...

comment:26 Changed 19 months ago by melinath

This is not backwards-incompatible in any way that I can see. As was previously mentioned, old browsers will interpret these HTML5 types as text fields. As for localization - the HTML inputs will no longer accept values that use THOUSANDS_SEPARATOR on the client side. As far as I know, that's not a backwards-compatibility issue; users will get a clear error message and be able to fix the issue without submitting the form. And on the python side, if THOUSANDS-separated text *does* get through (for example from an old browser) it will still be handled correctly.

Also this doesn't seem mutually exclusive with #15667, but it does seem more likely to be finished soon.

Changed 19 months ago by melinath

Updated patch to current master.

comment:27 Changed 19 months ago by melinath

  • Triage Stage changed from Accepted to Ready for checkin

Tests pass, docs look good to me, marking RFC.

comment:28 Changed 19 months ago by claudep

  • Triage Stage changed from Ready for checkin to Accepted

AFAIK, the compatibility issues are for CSS and JS:

  • input[type="text"] CSS selector will suddenly not apply any more for the new types (url, email, number).
  • If you access an input in Javascript based on the type property, e.g. if (form.elements[0].type=='text') { ... } , your script might be broken.

My personal view is that these incompatibilities are not so serious (mainly visual for CSS and in JS, it is not so frequent to test on input types), but you need to convince other core developers, too.

comment:29 Changed 19 months ago by melinath

If the issue is convincing core devs, shouldn't this be marked DDN?

comment:30 Changed 19 months ago by melinath

  • Triage Stage changed from Accepted to Design decision needed

comment:31 Changed 19 months ago by chronos

  • Cc philipe.rp@… added

comment:32 Changed 15 months ago by carljm

  • Triage Stage changed from Design decision needed to Accepted

IMO this is a change we simply need to make at some point in order to stay up to date, and waiting won't make the (minor) CSS/JS backwards-incompatibilities any less incompatible. We could do something more complex like a settings-flag to enable the change, but I really don't think the compatibility issues are worth that; we've never made any backwards-compatibility promises regarding form HTML output.

#15667 is not really relevant, as it's not clear if that can even happen, due to performance issues with the template engine. And in any case, this doesn't prevent that. So I think we should just go ahead and do this. Though we definitely do need to note the output changes in the "backwards incompatibilities" section of the release notes, so the patch is still lacking that.

comment:33 Changed 15 months ago by claudep

  • Owner changed from nobody to claudep
  • Status changed from new to assigned

Thanks Carl for your input. I'll take care of the commit.

comment:34 Changed 15 months ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

Latest version of the patch: https://github.com/django/django/pull/679

comment:35 Changed 15 months ago by charettes

  • Triage Stage changed from Ready for checkin to Accepted

The patch should take into account localized fields.

i.e. To keep the fix simple, localized IntegerField, FloatField and DecimalField should be rendered as text inputs with no additional attributes.

We could add pattern and maxlength generation based on the locale's settings latter on.

comment:36 Changed 15 months ago by claudep

Simon, you pointed out a valid issue on the pull request with maxlength on DecimalField. But maxlength is not used on IntegerField and FloatField, so I don't see how localization affects those fields in this patch. Could you elaborate?

comment:37 Changed 15 months ago by charettes

Say you have a localized IntegerField(initial=1000) with NUMBER_GROUPING = ' ' then it would be rendered as <input type="number" value="1 000" /> which is an invalid value for such HTML input type. Take a look at this fiddle in chrome, the value is totally stripped.

I'll add additional notes on the PR.

comment:38 Changed 15 months ago by claudep

I've split the pull request in three parts: EmailInput, URLInput and NumberInput. If first two are undisputed, we could commit this part and concentrate on the localization issue of NumberInput.

comment:39 Changed 15 months ago by Claude Paroz <claude@…>

In 4f16376274a4e52074722c615fccef5fac5f009a:

Added HTML5 email input type

Refs #16630.

comment:40 Changed 15 months ago by Claude Paroz <claude@…>

In f7394d2c32b4b4717066f254adaa513d08ab32a4:

Added HTML5 url input type

Refs #16630.

comment:41 Changed 15 months ago by charettes

The number input implementation seems to be a bit unstable at the moment.

Chrome implemented it quickly and attempted to localize the input value to the useragent's locale while submitting it the document (or closest parent with a lang attribute) locale. It raised some issues and it's quite hard to work with.

Firefox halted development because many questions arose concerning the decimal mark, group size and separator.

comment:42 Changed 15 months ago by claudep

  • Resolution set to fixed
  • Status changed from assigned to closed

I'm closing this ticket in favour of #19686 which will discuss the specific issue of number input type. Thanks for all involved contributors until now!

comment:43 Changed 14 months ago by Claude Paroz <claude@…>

In 7ec2a21be15af5b2c7513482c3bcfdd1e12782ed:

Fixed #19686 -- Added HTML5 number input type

Thanks Simon Charette for his help on the patch. Refs #16630.

comment:44 Changed 9 months ago by Tim Graham <timograham@…>

In 2a979d2a7bec485e4b90b7ae99ace0dd16faa948:

Updated contrib.admin to use Email/URLInputs; refs #16630

comment:45 Changed 9 months ago by Tim Graham <timograham@…>

In 5cc1ea4773f628f93dd2db9d353dc6b980e4a3ab:

[1.6.x] Updated contrib.admin to use Email/URLInputs; refs #16630

Backport of 2a979d2a7b from master

comment:46 Changed 9 months ago by anonymous

Is ColorInput available as a widget yet? Do you want a patch for it?

comment:47 Changed 9 months ago by claudep

I don't think so. Specialized input types should be left to third-party apps.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.