Opened 11 years ago

Closed 9 years ago

#22383 closed New feature (fixed)

Add the required tag to the input fields which are required for database entry

Reported by: abhishek.garg@… Owned by: Tim Graham <timograham@…>
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: anubhav9042@…, jon.dufresne@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,
When we use django forms, it would be nice if it automatically add the required tag to the input fields which are required for database entry, this way the user won't even be able to submit the form, instead of getting back the form with errors that "missing field" .

Change History (27)

comment:1 by ANUBHAV JOSHI, 11 years ago

Sounds sensible to me.

comment:2 by ANUBHAV JOSHI, 11 years ago

Cc: anubhav9042@… added
Summary: Django form auto add requried tagAdd the required tag to the input fields which are required for database entry
Triage Stage: UnreviewedAccepted
Version: 1.6master

comment:3 by Tim Graham, 11 years ago

Are there any backwards compatibility concerns? For example, what if someone is not using an HTML5 doctype?

in reply to:  3 comment:4 by ANUBHAV JOSHI, 11 years ago

Replying to timo:

Are there any backwards compatibility concerns? For example, what if someone is not using an HTML5 doctype?

I don't think there is any problem, if <!DOCTYPE html> is not used, then form does not gets submitted just as the case when doctype is used. The only difference is that the help message does not pops out.

comment:5 by ANUBHAV JOSHI, 11 years ago

Loic suggested that this would be good as an optional feature.

comment:7 by Tim Graham, 11 years ago

A form attribute doesn't seem like the most elegant solution, but it's consistent with other form attributes like Form.required_css_class and I cannot think of a better way. A setting is obviously a no-go...

I suggest calling it use_required_attribute instead of required_html_tag. IMO, I think most people would want this behavior so the attribute could default to True and people could opt-out by setting it to False. It would just need to be noted in the release notes as a backwards incompatible change.

comment:8 by ANUBHAV JOSHI, 11 years ago

Ok.
Wouldn't it be better to provide people to choose whether they want to go the conventional way or other just like in case of required_css_class. Also if we make it default all of form_tests totaling 390 fail.

But also as Tim Graham says "I don't think the feature will be used very much if it isn't turned on by default." also seems valid situation.

Thoughts?

Last edited 11 years ago by ANUBHAV JOSHI (previous) (diff)

comment:9 by ANUBHAV JOSHI, 11 years ago

Owner: changed from nobody to ANUBHAV JOSHI
Status: newassigned

comment:10 by loic84, 11 years ago

Having the required HTML5 attribute changes the browser behavior significantly. If we want it on by default, then I think we need a deprecation period, maybe the strategy used for #20684 can be applied here?

in reply to:  10 comment:11 by ANUBHAV JOSHI, 11 years ago

Replying to loic84:

Having the required HTML5 attribute changes the browser behavior significantly. If we want it on by default, then I think we need a deprecation period, maybe the strategy used for #20684 can be applied here?

I like this approach.
We can add the optional behaviour now with a warning that it would be made default in 1.9.

Last edited 11 years ago by ANUBHAV JOSHI (previous) (diff)

comment:12 by loic84, 11 years ago

So sum up what's been discussed on IRC:

I didn't suggest a quick deprecation cycle, but a normal deprecation cycle based on the value of a boolean. (btw, even if we did a faster deprecation, that would be using a subclass of DeprecationWarning, not just Warning)

To deprecate you could do:

class form.Form(object):
    #...
    use_required_attribute = None

class BoundField(object):
    #...
    def as_widget():
	if self.form.use_required_attribute is None:
	    warnings.warn()
	elif form.use_required_attribute:
	    attrs['required'] = True

Now what happens next depends on the actual goal, do we want to use use_required_attribute as a permanent way to control this feature, or as a mere deprecation tool.

Personally I'm not thrilled by this feature, although I know lot of people will want it, so I'd like to keep the switch.

Another point of interest, right now there is no way to tell a form that it should render as HTML4 or HTML5. When we discussed #20684 we acknowledged that if someone really cared about HTML4 validation, they could use required="required" instead of required=True, but if this becomes the default and we kill use_required_attribute, we'd be outputting HTML5.

comment:13 by ANUBHAV JOSHI, 11 years ago

I am in favor for accelerated deprecation, although I don't know if it'll be allowed here.

Last edited 11 years ago by ANUBHAV JOSHI (previous) (diff)

comment:15 by Stephen Burrows, 10 years ago

FTR, there are (potentially) backwards-compatibility issues regarding the "required" attribute and formsets. See https://github.com/gregmuellegger/django-floppyforms/issues/75

comment:16 by Tim Graham, 9 years ago

Owner: ANUBHAV JOSHI removed
Status: assignednew

comment:17 by Jon Dufresne, 9 years ago

Cc: jon.dufresne@… added
Has patch: set

Implemented use_required_attribute as described above.

https://github.com/django/django/pull/6341

comment:18 by Tim Graham, 9 years ago

I fear the deprecation will be quite annoying if every form in a project needs to be modified to silence all warnings. I wonder if template-based widget rendering (#15667) might ease this change. A project could provide custom widget templates if they don't want the required attribute (or if they want required='required'.

comment:19 by Jon Dufresne, 9 years ago

I fear the deprecation will be quite annoying if every form in a project needs to be modified to silence all warnings.

Alternatively, a project could monkey patch the base form class to set a True/False default for use_required_attribute. I could document this technique if you agree with the approach.

A project could provide custom widget templates if they don't want the required attribute (or if they want required='required'.

Wouldn't this still require a deprecation path such that the required attribute isn't set by default until a future version? In this scenario where would warning be produced? Would the project need to override all templates to squelch all warnings?

comment:20 by Tim Graham, 9 years ago

I guess I'm not sure if a deprecation path provides more value than making a backwards-incompatible change. For example, if we expect a majority of projects to adopt this change, then a deprecation will require every Django project to silence the warning instead of a subset of users to opt-out. Maybe you could try to get some other opinions on the mailing list thread.

comment:21 by Jon Dufresne, 9 years ago

I guess I'm not sure if a deprecation path provides more value than making a backwards-incompatible change.

I've added alternative PR that skips the deprecation cycle: <https://github.com/django/django/pull/6352>. It uses the same Form.use_required_attribute approach as the previous PR.

I'll follow through with mailing list to get other opinions.

comment:23 by Jon Dufresne, 9 years ago

From Alex Riina in the thread:

What's the plan for formsets with extra?

I could see the required only getting applied to the first min forms but I'm not sure there is an actual workable case there. It seems like it will get too messy with adding and deleting at the same time.

If can_delete is false and extra is 0, it seems like the required attribute could at least be used. Because of this, I think it should probably be an initialization argument, default to false, or be overridden when constructing forms in formsets.

Updated PR based on this feedback. The required attribute is no longer applied to formsets by default. Can now override the Form.use_required_attribute value by passing the kwarg use_required_attribute to the form's constructor.

comment:24 by Tim Graham, 9 years ago

Patch needs improvement: set

For the record, Loic dropped his suggestion of a deprecation cycle: "I guess people can easily add novalidate to their <form> to opt out of that".

I've left some comments for improvement on the pull request.

comment:25 by Jon Dufresne, 9 years ago

Patch needs improvement: unset

I've left some comments for improvement on the pull request.

I have addressed all comments in the PR. All additional feedback is welcome. Thanks.

comment:26 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:27 by Tim Graham <timograham@…>, 9 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In ec61216:

Fixed #22383 -- Added support for HTML5 required attribute on required form fields.

Note: See TracTickets for help on using tickets.
Back to Top