Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#10431 closed defect (fixed)

FilteringSelect : required fields are never empty and thus will never be in an error state

Reported by: Cormac Flynn Owned by: Douglas Hays
Priority: high Milestone: 1.5
Component: Dijit - Form Version: 1.4.0b
Keywords: filteringselect select Cc:
Blocked By: Blocking:

Description (last modified by bill)

Suppose I create a required FilteringSelect field as below

<select dojoType="dijit.form.FilteringSelect" name="testselect" id="testselect" required="true">
 <option value="A">A</option>
 <option value="B">B</option>
</select>

Upon page load, option A will be preselected. It is impossible for this field to be in an error state with respect to the required attribute as it will always have a valid value.

Suppose then that I want to create a required FilteringSelect where the user is forced to choose an option and where if the field remains untouched upon form submission it will be highlighted as being in an error state. This seems like a reasonable expectation of how the required attribute should work. I might try the following

<select dojoType="dijit.form.FilteringSelect" name="testselect" id="testselect" required="true">
 <option/>
 <option value="A">A</option>
 <option value="B">B</option>
</select>

But this field will never be in an error state with respect to the required field. The first option is still preselected and, even though it has no value attribute and the user has not selected any option for the field, the empty string is assigned as a valid value.

In version 1.3 there was a workaround for this issue - I could do the following

<select dojoType="dijit.form.FilteringSelect" name="testselect" id="testselect" required="true" value="null">
 <option value="A">A</option>
 <option value="B">B</option>
</select>

With the select box value attribute set to null, no value is initially chosen. If the user does not touch the field and then submits the form, the field will be highlighted as being in the error state.

This workaround no longer works in 1.4 - setting value="null" breaks the instantiation of the FilteringSelect object.

Without this workaround, I can't see any case where a required FilteringSelect field will, upon form submission, be highlighted as being in error with respect to the required attribute value. It will always have a valid value.

Attachments (1)

10431.patch (5.5 KB) - added by Douglas Hays 10 years ago.
patch file to support value="" and automated test

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by bill

Component: GeneralDijit
Description: modified (diff)
Owner: changed from anonymous to Douglas Hays
Summary: Required FilteringSelect fields are never empty and thus will never be in an error stateFilteringSelect : required fields are never empty and thus will never be in an error state

Hmm, I've never seen the value="null" workaround (apparently any illegal value would work the same), but I see your point that it worked in 1.3 and in 1.4 causes an exception.

I thought value="" would work but it doesn't seem to.

The workaround is to declare your FilteringSelect using a store rather than with the data inlined.

Anyway, Doug, this is technically a regression, what do you think?

comment:2 Changed 10 years ago by Douglas Hays

Resolution: invalid
Status: newclosed

Just don't specify the value attribute and it'll work.

comment:3 Changed 10 years ago by bill

Resolution: invalid
Status: closedreopened

As stated in the original description, when no value is specified (for the FilteringSelect <select> tag, not for the <option> tags), FilteringSelect pre-selects the first value.

comment:4 Changed 10 years ago by Cormac Flynn

If the workaround is to use a store, then the required attribute is broken for FilteringSelect? widgets created declaratively. Shouldn't this be documented somewhere?

comment:5 Changed 10 years ago by bill

Milestone: tbd1.5

Yup, we should update docs.dojocampus.org with the info, and then fix this properly in dojo 1.5 so that FilteringSelect, even with an <option>'s list, works like other form widgets, ie:

<select dojoType=dijit.form.FilteringSelect value="" required="true">
    ...

comment:6 Changed 10 years ago by Cormac Flynn

Incidentally, the new dijit.form.Select widget has (almost) the same issue. The required attribute has no effect on how the widget is validated upon form submission and the workaround no longer functions, however setting value="null" doesn't, in this case, prevent the widget from being instantiated.

For my purposes, it isn't practicable to have a store for every single FilteringSelect? or Select I use so I guess I'm going to have to create my own subclasses, unless anyone knows another workaround?

comment:7 Changed 10 years ago by Douglas Hays

Possible workaround until a proper fix can be implemented. Put this in the markup before the OPTION tags:

<script type="dojo/method" event="_callbackSetLabel" args="result, dataObject, priorityChange">
if(result.length == 1 && !result[0]){ result = []; }
dijit.form.FilteringSelect.prototype._callbackSetLabel.call(this, result, dataObject, priorityChange);
</script>

Seems to work with the value="null" trick. Incidentally, the value attribute of a SELECT is munged (set to selectedIndex) by some browsers and so the 1.5 fix may be trickier than anticipated.

comment:8 Changed 10 years ago by Douglas Hays

The above workaround is due to the following in FilteringSelect?:

onItem: function(item){
        self._callbackSetLabel([item], undefined, priorityChange);
}

[item] should be item? [item} : []
and then the workaround would not be necessary.

comment:9 Changed 10 years ago by bill

Incidentally, the new dijit.form.Select widget has (almost) the same issue.

Thanks, I split that issue off to a separate ticket, #10446.

comment:10 Changed 10 years ago by Douglas Hays

(In [20975]) Refs #10431. [undefined] should not be sent to FilteringSelect::_callbackSetLabel but rather [].

comment:11 Changed 10 years ago by Douglas Hays

Status: reopenednew

comment:12 Changed 10 years ago by Douglas Hays

I still need to create automated testcases, but I'd like to get verification that the above patch fixes the problems and doesn't cause new ones. Specify value="" in markup and set required="true".

Changed 10 years ago by Douglas Hays

Attachment: 10431.patch added

patch file to support value="" and automated test

comment:13 Changed 10 years ago by Cormac Flynn

The patch resolves the issue for me, and passes my unit tests, but obviously these wouldn't be anywhere near exhaustive. But, in my case at least, it hasn't introduced any issues so far

comment:14 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [21276]) Fixes #10431 in trunk. Added support for value="" for FilteringSelect? so that required=true will trigger an error if no value is chosen by the user. Added automated testcase.

comment:15 Changed 10 years ago by Douglas Hays

(In [21279]) Refs #10431 in trunk. Check for corner case where a ComboBox? is created with an INPUT markup tag AND no store defined (so no options to select from).

comment:16 Changed 9 years ago by bill

(In [23242]) Fix test to account for new Incomplete state, refs #11970, #10431.

comment:17 Changed 8 years ago by bill

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