Opened 12 years ago

Closed 11 years ago

Last modified 9 years ago

#4692 closed enhancement (fixed)

[patch] [cla] Form: prevent submit if form has invalid values

Reported by: bill Owned by: Douglas Hays
Priority: high Milestone: 1.2
Component: Dijit - Form Version: 0.9
Keywords: Cc: gizmojo.org, nathan
Blocked By: Blocking:

Description (last modified by bill)

Make the submit button gray out (or alternately show an alert?) if you try to submit the form with invalid values in it.

This is presumably a change to the Form widget (in Form.js)

Attachments (3)

ValidationState.patch (3.8 KB) - added by nathan 12 years ago.
Patch which adds onValidStateChange to dijit.form.Form. You can connect to this function and get notifications whenever the validation state changes (your function will be passed the new state). See the test case (part of this patch) for an example of hooking it to a submit button.
ValidationState.2.patch (4.3 KB) - added by nathan 12 years ago.
Updated file with better documentation
ValidationState.3.patch (5.6 KB) - added by nathan 12 years ago.
Updated patch to include better test cases (include programmatically adding new widgets)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 12 years ago by bill

Description: modified (diff)

comment:2 Changed 12 years ago by gizmojo.org

Cc: gizmojo.org added

Well, imho it is not for Form to decide this -- an application may want a different behaviour.

However, I fully agree that Form should provide the means to check for this and to take appropriate action if Form fields have invalid values, i.e. first to check if field values validate e.g. Form.isValid(), then it would be nice to easily be able to take appropriate action:

  • disable/enable submit button(s)
  • liberally change labels of submit button(s)
  • set a form "status" message

This (non-dojo) demo form instance of such client-side form functionalities may be useful as a source of ideas (see gform.js, in particular): http://demo.gizmojo.org/register/

In addition, it should also be possible to:

  • check if field values have changed in any way, e.g. Form.isDirty() (see #4694)

comment:3 Changed 12 years ago by Douglas Hays

This enhancement is partially working as requested in the 1.1 trunk.
Run dijit/tests/form/Form.html and enter an invalid date in 1 of the DateTextBox? fields. Press the Submit button. The onsubmit handler checks for valid values and prevents the form from being submitted.

comment:4 Changed 12 years ago by Adam Peller

Owner: set to Douglas Hays

Yes, but disabling the button prior to onSubmit would be a far superior user experience (and require no intervention on the part of the developer)

It would be awesome if we could do this in _FormMixin. Doug, do you want to take a stab at this? Is it as simple as iterating through the widgets on creation, then just "OR"ing in the results onChange of each widget?

comment:5 Changed 12 years ago by bill

Milestone: 1.11.2
Type: defectenhancement

I'm not sure that disabling the submit button whenever the form is incomplete/invalid is the best design. Having a grayed out submit button might just confuse (or infuriate) the user because they don't realize why they can't submit the form. (And also it doesn't fit common expectations for how the web works.) We do mark which fields are invalid, and sometimes we mark incomplete fields (but only when you've tabbed through them), but especially on a long form, the user might not notice. I'm marking for 1.2 but let's get some feedback before implementing, perhaps from a usability expert if possible.

comment:6 Changed 12 years ago by bill

Cc: nathan@… added
Owner: changed from Douglas Hays to nathan

Nathan wants to work on this one; please attach a patch and modify but summary to say [patch] [cla] when you do (assuming you have signed a CLA).

See also #6005. For large forms it's better to not disable the submit button, but rather to have it tell the user where the mistake is, so this should be an option to Form, not the default behavior.

comment:7 Changed 12 years ago by nathan

Cc: nathan added; nathan@… removed
Status: newassigned

comment:8 Changed 12 years ago by nathan

Cc: nathan removed

comment:9 Changed 12 years ago by nathan

We discussed this, and will have the form widget connect to each of its child widgets setValue() function. When the valid status of the form changes (either from invalid to valid or vice-versa), a function will be called which can be attached to by another widget (such as a submit button).

Changed 12 years ago by nathan

Attachment: ValidationState.patch added

Patch which adds onValidStateChange to dijit.form.Form. You can connect to this function and get notifications whenever the validation state changes (your function will be passed the new state). See the test case (part of this patch) for an example of hooking it to a submit button.

comment:10 Changed 12 years ago by nathan

Milestone: 1.21.1
Summary: Form: prevent submit if form has invalid values[patch] [cla] Form: prevent submit if form has invalid values

Changed 12 years ago by nathan

Attachment: ValidationState.2.patch added

Updated file with better documentation

comment:11 Changed 12 years ago by nathan

Cc: nathan added
Owner: changed from nathan to Douglas Hays
Status: assignednew

Changed 12 years ago by nathan

Attachment: ValidationState.3.patch added

Updated patch to include better test cases (include programmatically adding new widgets)

comment:12 Changed 12 years ago by Douglas Hays

Milestone: 1.11.2

Some issues with _ChangeConnections(): 1) should not be private since it has to be called by users, 2) removes all existing children and re-adds them (should we have a addChild and removeChild), 3) related to (2), there is already an addChild and removeChild and they aren't being used - maybe they should be.
Major issue: with a disabled submit button, the user has no way to know which of the fields are invalid if they haven't already been focused and blurred so the form becomes hard to use.
I propose that the button be enabled whenever there are invalid fields that are currently showing as valid (required and empty and nonblurred).

comment:13 Changed 12 years ago by bill

Description: modified (diff)

The "disabled submit button" thing is just the way the test file works, not a feature of dijit.Form, so I wouldn't worry about that.

As for addChild()/removeChild(), those might be nice to have in general (see #6192) although it's not even clear what those functions would do; sounds like you are talking more about registering new children than actually automatically inserting them into the DOM like BorderContainer?.addChild() does. But _changeConnections() seems equally good to me, as a way to rescan what child widgets there are. Not sure how addChild()/removeChild() are superior to just rescanning?

comment:14 Changed 11 years ago by Douglas Hays

Status: newassigned

comment:15 Changed 11 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [14161]) Fixes #4692. Add onValidStateChange connect point to dijit.form.Form widget so that custom behavior like disabled submit buttons can be created.

comment:16 Changed 9 years ago by bill

(In [23218]) Rename ValidationState test to !Form_state so it's clear that it's a test for the Form widget. Refs #4692.

comment:17 Changed 9 years ago by bill

(In [23219]) Rename ValidationState test to !Form_state so it's clear that it's a test for the Form widget. Refs #4692.

comment:18 Changed 9 years ago by bill

(In [23220]) Simplify test by using focus() instead of robot mouse move / mouse click. Some of the tests end up not using robot at all. Refs #4692.

comment:19 Changed 9 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.