Opened 12 years ago

Closed 12 years ago

Last modified 8 years ago

#4010 closed defect (fixed)

FilteringSelect: entering invalid value doesn't change hidden value

Reported by: bill Owned by: haysmark
Priority: high Milestone: 1.0
Component: Dijit - Form Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description

In FilteringSelect? if I select a valid value (like California) but then enter an invalid value (like xxx) the hidden value should change to "", shouldn't it?

Otherwise when the form is submitted (if we allow submission with an invalid value) then the result is unexpected.

Attachments (3)

4010.patch (763 bytes) - added by haysmark 12 years ago.
Set hidden submit value to "" when user enters invalid input. onChange does not fire. Fixes #4010.
4010.2.patch (1.7 KB) - added by haysmark 12 years ago.
Set hidden submit value to "" when user enters invalid input. onChange fires with null. Changed ComboBox? escape handling to best work with this change. Fixes #4010.
4010.3.patch (1.8 KB) - added by haysmark 12 years ago.
Set hidden submit value to "" when user enters invalid input. onChange fires with null. Changed ComboBox? escape handling to best work with this change. Initialized _lastDisplayedValue in ComboBox?. Fixes #4010.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 12 years ago by haysmark

FilteringSelect? used to have a similar behavior and we agreed to remove it.

  • It doesn't make sense to allow "" to go to the server if the server doesn't supply a "" in the data store. Native select does not submit "" if there is not a "" option. The result would be unexpected to the server, so to speak.
  • What if the item store already has a "" item? Then setting the value to "" would clobber the user's input onBlur, which Alex has already vetoed.

comment:2 Changed 12 years ago by bill

Regarding:

It doesn't make sense to allow "" to go to the server if the server doesn't supply a "" in the data store. Native select does not submit "" if there is not a "" option. The result would be unexpected to the server, so to speak.

Yes, that's right. But also, allowing "CA" to go the server when the user has typed "Mexico" doesn't make sense either. User shouldn't be allowed to submit the form while there are invalid values, in FilteringSelect? or in other fields, and that's something we'll be working on for 1.1. Note that this isn't an issue with native select since native select doesn't allow you to enter an invalid value.

What if the item store already has a "" item? Then setting the value to "" would clobber the user's input onBlur, which Alex has already vetoed.

Clearly you should never erase the user's input, even if it's invalid. If the user types an invalid value Maybe Widget.value needs to be set to null or undefined, which would set the hidden input field to "" but not affect the displayed input field. I'm not sure of the implementation.

comment:3 Changed 12 years ago by haysmark

So if the user enters an invalid value, should onChange fire with null or "" or not at all?

Changed 12 years ago by haysmark

Attachment: 4010.patch added

Set hidden submit value to "" when user enters invalid input. onChange does not fire. Fixes #4010.

comment:4 Changed 12 years ago by haysmark

With this patch the submit value gets set to "", but the user's onChange handler does not fire. That means if you load test_FilteringSelect, the value fields will not change but if you press submit you will see that the value is empty in the address bar.

comment:5 Changed 12 years ago by bill

If the user enters an invalid value, onChange fire with null. Can you update the patch that way?

Changed 12 years ago by haysmark

Attachment: 4010.2.patch added

Set hidden submit value to "" when user enters invalid input. onChange fires with null. Changed ComboBox? escape handling to best work with this change. Fixes #4010.

comment:6 Changed 12 years ago by haysmark

Bill here is a patch to make onChange fire on invalid value (with null) and on Escape (with reverted value).

comment:7 Changed 12 years ago by Douglas Hays

Needs further investigation before committing the patch. from bill,
it looks to me like ESC won't revert the value now, if the original value was blank (like a state drop down with no initial state selected)

comment:8 Changed 12 years ago by Douglas Hays

if(this._lastDisplayedValue ...) may need to be tightened up to account for "" as a valid value

comment:9 Changed 12 years ago by haysmark

So the real problem is that the original value isn't even being set in ComboBox?. That's why I added that clause. It might be better just to find a way to initialize the value at the start without tripping onChange.

Changed 12 years ago by haysmark

Attachment: 4010.3.patch added

Set hidden submit value to "" when user enters invalid input. onChange fires with null. Changed ComboBox? escape handling to best work with this change. Initialized _lastDisplayedValue in ComboBox?. Fixes #4010.

comment:10 Changed 12 years ago by haysmark

Here is a safer patch.

comment:11 Changed 12 years ago by bill

The patch says:

//#4010: blank out the value but don't call setValue so _lastValueReported stays the same

but that seems the opposite of what you want. _lastValueReported should always be synchronized with the last value reported by onChange().

Talked to Doug on IRC just now. We think that typing an illegal value (and tabbing away) should call setValue(undefined) which will call onChange(null).

comment:12 Changed 12 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [11273]) Fixes #4010. Set hidden value to "" on blur and call onChange with undefined.

comment:13 Changed 8 years ago by bill

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