Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#9914 closed defect (fixed)

FilteringSelect/ComboBox onBlur changes an already selected valid item

Reported by: Mindblast Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Dijit - Form Version: 1.3.2
Keywords: FilteringSelect Cc:
Blocked By: Blocking:

Description

I'm using FilteringSelect? with a QueryReadStore? and a custom labelFunc to present some additional information. Now when i have selected a valid item in the FilteringSelect? (from the drop down list) and the FS then looses focus, another fetch is done with the query set to the label set by my custom labelFunc (which is not quite compatible with the searchAttr my backend expects). My backend just returns a number of possible items and the first one of these is then selected into the FilteringSelect? despite the fact i had a valid item in there already. I did some digging and found that in ComboBox:_setBlurValue if the popupWidget is not active the displayedValue is essentially just read out and then put back in which then leads to all this refetching because FilteringSelect:_setDisplayedValueAttr is called. I have solved this for me locally by defining _setBlurValue in FilteringSelect? like this:

_setBlurValue: function() {
    if(!this._isvalid)
	this.inherited(arguments);
},

I doubt that this would be the best way to solve the problem though.

Attachments (1)

9914.patch (32.2 KB) - added by Douglas Hays 10 years ago.
proposed fix to remember both item and value during user-interactions - needs lots of testing

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by bill

Hmm, I'm having trouble understanding your description. Can you attach a test case?

Of course QueryReadStore is asynchronous but why is FilteringSelect doing a server access when the user selects something from the drop down list? Is that something that happens all the time or just because of your custom label func?

comment:2 in reply to:  1 Changed 10 years ago by mindblast

Replying to bill:

Hmm, I'm having trouble understanding your description. Can you attach a test case?

Of course QueryReadStore is asynchronous but why is FilteringSelect doing a server access when the user selects something from the drop down list? Is that something that happens all the time or just because of your custom label func?

No no the problem is, it does a query when the control looses focus (tabbing out or clicking somewhere else with the mouse), this is also unrelated to my labelFunc. The labelFunc just causes my server to not be able to do a query.

For instance see here: http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/data/demos/demo_QueryReadStore_FilteringSelect.html

Say you select California.. the first query is done when you click on the down arrow button as i would expect. Then you select California in the dropdown list so the FilteringSelect? has a valid item. But then when you click somewhere else with the mouse the FilteringSelect? does another query with "name" (the searchAttr) set to "California". This is completely unnecessary when there is already a valid item and also if there was a labelFunc that would change the entry to say "California, where the Terminator rules" the query would then also have this text in the "name" field, giving your backend a little headache what the search string should be.

Also my _setBlurValue function above was still not quite working as expected, now it looks like this:

_setBlurValue: function() {
    if(!this.item)
	this.inherited(arguments);
},

seems to work ok so far..

comment:3 Changed 10 years ago by bill

Owner: set to Douglas Hays

comment:4 Changed 10 years ago by Douglas Hays

Mindblast,bill: Currently the drop down list displays either labelAttr or searchAttr strings. Should the drop down list display labelFunc() returned strings instead? The labelfunc() functionality can really mess up autcomplete/highlighted text.

comment:5 Changed 10 years ago by bill

Doughays - Yes, the drop down list should display labelFunc() returned strings. I agree that using labelFunc() on the drop down list will mess up highlighting, but so will using separate searchAttr and labelAttr. Anytime that the search-string and the label don't match each other, highlighting will have problems, and there are also more serious problems which I've listed below.

Mindblast - thanks for the description, now I understand the problem. The main point of this ticket seems valid, that there's an unnecessary query occurring. However, I don't think that your design will be safe even after that's fixed. Here's why:

Neither labelFunc() nor labelAttr conceptually work very well... they are really useful for adding icons to each drop down item, and even then, the icons are only visible in the drop down, not in the <input> area after a drop down item has been selected.

If you try to use labelFunc() for changing the text, for example "California" --> "California, where the terminator rules", then besides the problem listed in this ticket, if a user directly types in "California, where the terminator rules" into the <input> box, FilteringSelect has no way to figure out which item that typed text refers to (other than by running labelFunc() on every single item). I guess we could do that although obviously it would be slow for large data sets.

comment:6 Changed 10 years ago by Douglas Hays

Priority: normalhigh
Status: newassigned

comment:7 Changed 10 years ago by Douglas Hays

Milestone: tbd1.4

Changed 10 years ago by Douglas Hays

Attachment: 9914.patch added

proposed fix to remember both item and value during user-interactions - needs lots of testing

comment:8 Changed 10 years ago by Douglas Hays

Mindblast, this is a big change, can you donate any test cycles to the patch?

comment:9 Changed 10 years ago by bill

The basic idea (remembering both item and value during user-interactions) sounds good, and I didn't notice anything strange about the patch code except that item is initially null (as defined in the prototype) but then afterwards you change it to undefined. I'll try to look over the patch more though; like you said it's a big change. But I guess it's worth putting into 1.4 now, assuming all the unit tests are passing.

This patch looks like it also fixes #6022, although you just need to rename _setValueFromItem() to _setItemAttr()?

comment:10 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [20406]) Fixes #6022, #9914 !strict. Maintain this.item whenever the ComboBox? value changes to prevent needless store fetches. Add _setItemAttr support. Add labelFunc capability in ComboBox? previously only in FilteringSelect?. Add additional labelFunc test for ComboBox? and add automated tests for this.item and labelFunc.

comment:11 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

After this change ComboBox_mouse.html is failing with customLabelFunc getting called one less time than expected (3 times instead of 4). Maybe the test just needs to be updated?

comment:12 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: reopenedclosed

(In [20452]) Fixes #9914. Changed testcase to account for the custom onChange adding 1 extra labelFunc() call.

comment:13 Changed 8 years ago by bill

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