Opened 12 years ago

Closed 11 years ago

Last modified 9 years ago

#5974 closed defect (fixed)

[patch] FilteringSelect: don't require a blank entry in the drop down list

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

Description (last modified by bill)

Non-required <select> inputs in a native HTML form typically have a blank option in the drop down, to indicate that no value is specified. (See for example the Milestone field when filing a new ticket in trac.) Currently the only way for a developer to implement a non-required FilteringSelect field is to do the same thing, to have a blank option in the drop down.

However, since FilteringSelect (which has the same UI as ComboBox) appears to the user more like an <input> than a <select>, the user expects to be able to clear the value just by backspacing over the data in the <input>, and doesn't expect to see a blank value in the drop down list. It's also troublesome to have a blank item in your data store.

Eliminate requirement that optional FilteringSelect fields have a blank entry in the drop down. Modify FilteringSelect so that clearing the <input> (by backspacing) makes getValue() return null or undefined (like a blank DateTextBox). Support required parameter for FilteringSelect just like ValidationTextBox... default would be required=true but developer can override. If required=false then a blank input is not flagged as invalid.

setValue(null) (or maybe undefined) would clear the value, same as DateTextBox.

I remember some argument before about supporting a blank key as a valid hidden value for a filtering select, ie:

<select dojoType="dijit.form.FilteringSelect>
    <option value="1"></option>
    <option value="2">blue</option>
</select>

However, I consider this a corner case not worth supporting.

Ping me on #dojo if this doesn't make sense.

Attachments (4)

5974.patch (2.7 KB) - added by haysmark 12 years ago.
Fixes #5974. FilteringSelect? now respects required=false. Also addressed a related bug in ComboBoxDataStore? that was causing required=false to fail to fire onChange(undefined) with blank input.
5974.2.patch (3.1 KB) - added by haysmark 12 years ago.
Fixes #5974. FilteringSelect? now respects required=false. setValue(undefined) clears the input. Also addressed a related bug in ComboBoxDataStore? that was causing required=false to fail to fire onChange(undefined) with blank input.
5974.3.patch (3.8 KB) - added by haysmark 12 years ago.
Fixes #5974. FilteringSelect? now respects required=false. setValue(null) clears the input. Also addressed a related bug in ComboBoxDataStore? that sent all items to the ComboBox? on a totally blank query.
5974.4.patch (1.1 KB) - added by haysmark 11 years ago.
Fixes #5974. Remove query for "" when the user types a blank value.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 12 years ago by bill

Description: modified (diff)

Changed 12 years ago by haysmark

Attachment: 5974.patch added

Fixes #5974. FilteringSelect? now respects required=false. Also addressed a related bug in ComboBoxDataStore? that was causing required=false to fail to fire onChange(undefined) with blank input.

comment:2 Changed 12 years ago by haysmark

Description: modified (diff)
Status: newassigned

comment:3 Changed 12 years ago by haysmark

Summary: FilteringSelect: don't require a blank entry in the drop down list[patch] FilteringSelect: don't require a blank entry in the drop down list

comment:4 Changed 12 years ago by haysmark

Bill, here is how I was going to handle required=false. I still need to implement setValue(null) before we commit.

Changed 12 years ago by haysmark

Attachment: 5974.2.patch added

Fixes #5974. FilteringSelect? now respects required=false. setValue(undefined) clears the input. Also addressed a related bug in ComboBoxDataStore? that was causing required=false to fail to fire onChange(undefined) with blank input.

comment:5 Changed 12 years ago by haysmark

Bill, I implemented setValue(undefined) by converting it to setDisplayedValue("").

comment:6 Changed 12 years ago by bill

This is basically looking good but I want to resolve some issues with null/undefined/""/no value; I sent Mark mail about it so as to not make this ticket incredibly long. But basically the idea is to standardize on null as the indicator for empty/invalid value.

Changed 12 years ago by haysmark

Attachment: 5974.3.patch added

Fixes #5974. FilteringSelect? now respects required=false. setValue(null) clears the input. Also addressed a related bug in ComboBoxDataStore? that sent all items to the ComboBox? on a totally blank query.

comment:7 Changed 12 years ago by haysmark

Bill, I went through and normalized the use of undefined and "" in FilteringSelect? to null.

comment:8 Changed 12 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [13989]) Support required=true/false parameter for FilteringSelect? so that a blank entry in the dropdown list is not necessary. Patch from Mark Hays (IBM, CCLA on file). Fixes #5974 !strict. Also some cleanup so that onChange() and getValue() return "" for when the input field is blank/invalid. (Before I had thought to return null in those cases but "" seems better.) Refs #5919.

comment:9 Changed 12 years ago by liucougar

(In [13993]) refs #5974: no need to use our own hack in editor plugin for blank entry in dropdown list

comment:10 Changed 11 years ago by paulprince

Resolution: fixed
Status: closedreopened

required=false has no effect on FilteringSelect? when it is using a QueryReadStore?

You can see this demonstrated by going to http://archive.dojotoolkit.org/dojo-2008-08-04/dojotoolkit/dojox/data/tests/QueryReadStore.html and executing:

dijit.byId("fs").required=false;

The behavior of the FilteringSelect? is not changed.. you cannot set it to be blank.

comment:11 Changed 11 years ago by haysmark

Actually that is a different problem: someone's test PHP is returning the full result list when it should return nothing. So if you click Arizona, but then clear the input, you will get Alabama since it is the first thing the PHP returns. The FilteringSelect? is correctly doing what the server told it to do. This doesn't affect the ComboBoxes? on the same page because there is no reverse lookup in ComboBox?.

Opening a new ticket for this.

comment:12 Changed 11 years ago by haysmark

Resolution: duplicate
Status: reopenedclosed

paulprince, thank you for finding this. Please refer to #7369.

comment:13 Changed 11 years ago by bill

Resolution: duplicate
Status: closedreopened

Hmm, why is FilteringSelect doing a query on "" when it knows the results should be an empty list? Couldn't it just bypass that?

I suppose there's a bug in QueryReadStore.php, although it seems dangerous to depend on query.php?field= working as you expect, since for a user-directed search form like this:

<form action="search.php" method="get">
   <input type="text" name="first">
   <input type="text" name="second">
   <input type="submit">
</form>

... presumably leaving a field blank would mean to find everything.

(In any case we shouldn't close this ticket as a duplicate; perhaps the previous comment from paulprince is a separate ticket but the main bulk of this ticket isn't a duplicate of anything.)

comment:14 Changed 11 years ago by paulprince

Huge agreement here: if the server-side should always return [] for query="" (which sounds reasonable to me), then there's no reason to even query the server when the field is blank. Implementing this would eliminate A LOT of ajax requests.

comment:15 Changed 11 years ago by paulprince

Ok, I've thought about this, and I don't think the server should always return [] for query=""

Imagine a case of an autocompleting combobox/filteringselect, where you want a set of default values returned on query="".

I think FilteringSelect? should be able to tolerate a return for query="" which neither is empty nor begins with a "" entry, and still allow itself to be set to blank when required = false

For now, I am simply beginning my results lists for query="" with a "" entry, but this is not very intuitive for new users, and I don't see a good reason why it should be needed: isn't that exactly what .required is for?

comment:16 Changed 11 years ago by dante

Cc: paulprince added

Changed 11 years ago by haysmark

Attachment: 5974.4.patch added

Fixes #5974. Remove query for "" when the user types a blank value.

comment:17 Changed 11 years ago by haysmark

Appended workaround for deviant dojo.data stores.

So now setValue("")->setDisplayedValue("")->callBackSetLabel([])->required logic

comment:18 Changed 11 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [15118]) Fixes #5974: Don't query for "" when the user types a blank value. Patch from Mark Hays (IBM, CCLA on file). Thanks Mark! !strict

comment:19 Changed 11 years ago by bill

Description: modified (diff)

comment:20 Changed 11 years ago by bill

See also #7630 as a response to this change.

comment:21 Changed 9 years ago by bill

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