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 HTML5 attributes to text widgets. #116

Closed
wants to merge 4 commits into from

Conversation

Projects
None yet
@nandoflorestan
Copy link

commented Sep 4, 2012

With this, textarea, textinput and password templates support the
maxlength, placeholder and required attributes.

  • required already exists as a field attribute so we just read that.
  • maxlength and placeholder are read from the widget instance.

Also, HTML5 supports several new type values ('date', 'number' etc.)
so this becomes flexible in textinput.pt so this template can be reused.
It reads widget.type and defaults to "text".

What about people who aren't using HTML 5?

Why, I don't know, are there any?

Maybe it is better if you tell me how to proceed. Should Deform be globally
informed about the HTML version, then templates can be adjusted? How would we
do this?

Add HTML5 attributes to text widgets.
With this, textarea, textinput and password templates support the
*maxlength*, *placeholder* and *required* attributes.

- *required* already exists as a field attribute so we just read that.
- *maxlength* and *placeholder* are read from the widget instance.

Also, HTML5 supports several new *type* values ('date', 'number' etc.)
so this becomes flexible in textinput.pt so this template can be reused.
It reads widget.type and defaults to "text".

What about people who aren't using HTML 5?
==========================================

Why, I don't know, are there any?

Maybe it is better if *you* tell me how to proceed. Should Deform be globally
informed about the HTML version, then templates can be adjusted? How would we
do this?
required 'required' if field.required else None;
size field.widget.size;
type getattr(field.widget, 'type', 'text');
value cstruct or None;

This comment has been minimized.

Copy link
@kiorky

kiorky Sep 21, 2012

Member

Why defaulting to None, and not for exemple ''

This comment has been minimized.

Copy link
@tilgovi

tilgovi Oct 11, 2012

Contributor

Defaulting to None means the attribute will be excluded from the tag, which seems reasonable to me.

id field.oid;
maxlength getattr(field.widget, 'maxlength', None);
placeholder getattr(field.widget, 'placeholder', None);
required 'required' if field.required else None;

This comment has been minimized.

Copy link
@kiorky

kiorky Sep 21, 2012

Member

required stuff must be part of another pullrequest

This comment has been minimized.

Copy link
@tilgovi

tilgovi Oct 11, 2012

Contributor

Why is that? It's an HTML5 attribute as well, it seems.

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2012

@nandoflorestan Do you have the desire to clean this PR up?

Else, I could be convinced to take up the task.

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2012

I would add autocapitalize and autocomplete to this.
The white space changes are unnecessary and inconsistent with the rest of the templates. They should be removed.

Looks good otherwise.

@nandoflorestan

This comment has been minimized.

Copy link
Author

commented Oct 11, 2012

@tilgovi, agree. Can you take over then?

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2012

Happy to. Will post the update patch in a bit.

@kiorky

This comment has been minimized.

Copy link
Member

commented Oct 20, 2012

what's the status ?

@davidjb

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2013

Are there plans for this pull request to all the other various HTML5 attributes as well? (eg http://www.w3.org/TR/html-markup/input.text.html#input.text)

Perhaps it would be easier if we could specify a dict/2-tuple of arbitrary properties to have associated with each input or other control. This would be more flexible and be a solution for doing things like adding data-* attributes and anything else as well that might come up in the future.

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2013

Hey, sorry for remaining silent on this. I haven't had time to look at it. Last I did I came to the same conclusion as @davidjb: it probably makes sense to just have a list of 2-tuples to add as attributes.

@davidjb

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2013

@tilgovi For what it's worth, I should clarify that my thoughts are that all attributes (including those that exist as class attributes already: eg size, class, style etc) for Deform would benefit from being defined in a 2-tuple manner.

At present, if you want to add any new attribute (not just data-*), you end up needing to customise and re-define all these attributes on both your Widget class and in your template. Maintaining sets of copies of the templates just because I need to add an attribute (eg data-*, tabindex, autocomplete or anything else) is overly complicated.

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2013

I'm agreed, but I've stopped using deform's templates for my current project, opting for hand-written HTML templates and direct colander validation, so I haven't much motivation to fix this myself at this point.

@chrisrossi

This comment has been minimized.

Copy link
Member

commented Mar 19, 2013

There seems to be low enthusiasm for this particular patch. Shall I close? FWIW, the Widget constructor does accept arbitrary keywords and attaches them to the widget instance.

https://github.com/Pylons/deform/blob/master/deform/widget.py#L131

@tilgovi

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2013

Seems fine.

@cguardia cguardia closed this Mar 20, 2013

@neilferreira

This comment has been minimized.

Copy link

commented Apr 15, 2013

Any chance that this patch will be re-visited at some point? It is really unfortunate that deform still doesn't have the ability to support basic HTML5 validation features like 'required'

This patch really should have been merged in at the time it was submitted.

@stephenmac7

This comment has been minimized.

Copy link

commented Jul 25, 2013

@chrisrossi Yes, but the default templates do not have it implemented.

@nandoflorestan

This comment has been minimized.

Copy link
Author

commented Jul 25, 2013

While somebody should take up the task of holistically supporting HTML5 in deform, my patch indeed is useful and should have been accepted a long time ago. It wouldn't have done any harm.

@mcdonc mcdonc reopened this Jul 25, 2013

@mcdonc

This comment has been minimized.

Copy link
Member

commented Jul 25, 2013

I'm reopening this so maybe one of us can take a look at it at an upcoming sprint in Halle in August.

@Themanwithoutaplan

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2013

This will be revisited once @iElectric has merged deform_bootstrap because this already supports a lot of HTML5 form attributes. It will need to be extended to cover the various types (number, e-mail, tel, range, etc.) and other functions including patterns and have tests added.

@nandoflorestan

This comment has been minimized.

Copy link
Author

commented Aug 24, 2013

What do you mean, "merge deform_bootstrap".

@mcdonc

This comment has been minimized.

Copy link
Member

commented Sep 13, 2013

There's a big set of changes in the https://github.com/Pylons/deform/tree/deform2 branch (it uses twitter bootstrap 3 classnames, and is probably not going to be particularly fun to style if you don't want to use bootstrap)

@nandoflorestan

This comment has been minimized.

Copy link
Author

commented Sep 13, 2013

Well, is that branch the future of deform?

If not, is vanilla deform going to support HTML5 attributes?

@mcdonc

This comment has been minimized.

Copy link
Member

commented Sep 13, 2013

Yes, it is the branch that is the future of deform.

@arturambroziak

This comment has been minimized.

Copy link

commented Feb 16, 2016

any plans for adding maxlength support?

@stevepiercy

This comment has been minimized.

Copy link
Member

commented Feb 16, 2016

@arturambroziak yes, see discussion preceding your comment, and the TODO under https://github.com/Pylons/deform/blob/master/CHANGES.txt#L71.

Beyond that, no additional PR exists to add maxlength, but would be welcome and very likely merged, provided tests and docs are included with the code change.

@magoarcano

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2017

Hi, I created a pull request:
#345

It adds any attribute support: maxlength, required, data-*, placeholder, etc

Now there is no need to edit the templates, and you can pass the parameters in a dict. For example:
my_field = colander.SchemaNode(colander.Integer(), widget=TextInputWidget(attributes={'maxlength': 8, 'placeholder': 'nickname', 'disabled': True}))

@ericof ericof closed this in #345 Nov 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.