Opened 12 years ago

Closed 10 years ago

Last modified 8 years ago

#6022 closed enhancement (fixed)

ComboBox: set value programatically according to item from store

Reported by: wolfram Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Dijit - Form Version: 1.0
Keywords: Cc: bill, alex
Blocked By: Blocking:

Description (last modified by alex)

Add ability for ComboBox.setValue() to take an item, in which case it will set ComboBox.item = item (just like when you select something from the drop down list), and display the item's value in the <input>.

Sort of implicit in this patch is the idea that ComboBox.item should be set whenver there is an item corresponding to what the user typed in.

Attachments (3)

patch6022.diff (698 bytes) - added by wolfram 12 years ago.
patch6022-1.diff (1.4 KB) - added by wolfram 10 years ago.
extended patch
6022.patch (37.7 KB) - added by Douglas Hays 10 years ago.
possible fix to support attr('item',blah) and to better sync this.item with onChange

Download all attachments as: .zip

Change History (22)

comment:1 Changed 12 years ago by dylan

Milestone: 1.2

comment:2 Changed 12 years ago by bill

Resolution: wontfix
Status: newclosed

No, there isn't any builtin way to make ComboBox automatically select a value, and no plans to add it; I'd suggest posting to the dijit support forum. Certainly you should be attaching to some method in ComboBox? (called when the data from the store is returned) rather than using a timer.

Changed 12 years ago by wolfram

Attachment: patch6022.diff added

comment:3 Changed 12 years ago by wolfram

Resolution: wontfix
Status: closedreopened

The attached patch allows for a simple and clean way to set a value that actually comes from a store. May be a more integrated way is possible, but that one looks like a very flexible one. The patch actually just allows to call setValue() using an item from a store. And it makes only sense using an item from the widget's store. Like so:

>>> w = dijit.byId("myComboBox")
>>> w.store.fetch({query:{name:"whatever*"}, onComplete:function(items){ w.setValue(items[0]) }})

This example just uses the store of the combobox, retreives the item we need (actually it might be a number of items, but items[0] is only used). And we just stick this item into setValue() which (after applying the patch) sets the value properly using the item.

The advantages are:

  • setting a value form an item is possible
  • the combobox internals are not required to know
  • you can fire any kind of search against the store and still preprocess the items before calling setValue()
  • its relatively simple :-)

hope this can convince bill :-)

comment:4 Changed 12 years ago by bill

Description: modified (diff)
Summary: ComboBox: Select an option coming from a store programmaticallyComboBox: set value programatically according to item from store
Type: defectenhancement

Perhaps we could add this feature; couple things for me to think about though. Basically, ComboBox.item isn't meant to be accessed by the developer since sometimes it's null (like when the user has typed a random value into the input). Never meant to support this and not sure if I want to, but this patch sort-of implies that we are doing that. Not sure if I want to expand the API to support that although I can see some value. Need to check:

  • if the user types in a value equal to something in the drop down list, but doesn't actually select the value from the drop down list nor uses auto complete, check if ComboBox.item gets set.
  • need to test if patch works for FilteringSelect

comment:5 Changed 12 years ago by John Locke

I opened #6197 before finding this issue, and I think it's related. In an application I'm developing, being able to access ComboBox?.item is very useful, and it's trivial to check for it being null before passing it to the store.

Obviously, I think this is a useful part to the api, and assumed it would be there since it wasn't marked private...

comment:6 Changed 11 years ago by alex

Cc: bill added
Description: modified (diff)

where are we on this? bill?

comment:7 Changed 11 years ago by alex

Cc: alex added

comment:8 in reply to:  4 Changed 11 years ago by wolfram

Replying to bill:

Perhaps we could add this feature; couple things for me to think about though. Basically, ComboBox.item isn't meant to be accessed by the developer since sometimes it's null (like when the user has typed a random value into the input). Never meant to support this and not sure if I want to, but this patch sort-of implies that we are doing that. Not sure if I want to expand the API to support that although I can see some value. Need to check:

  • if the user types in a value equal to something in the drop down list, but doesn't actually select the value from the drop down list nor uses auto complete, check if ComboBox.item gets set.
  • need to test if patch works for FilteringSelect

I added the .item property and it is not private to be possible to read from outside! That is intentionally, because when the data come from a store it makes a loooot of sense to be able to access the item who's value is the one you have selected in the combobox. And since an item can contain more data than just the displayed value you need access to the item. And that it can be null is part of the contract! I.e.

var id = widget.store.getValue(widget.item, "id");
var firstName = widget.store.getValue(widget.item, "firstName");
var lastName = widget.store.getValue(widget.item, "lastName");

You can access a looot of data through the item, if the backend sends them along. But the combobox normally only shows one piece of those data, i.e. the "username" (would make sense with the above exacmple code).

I am very much for making the .item a first class citizien, since the dojo.data API is an important part for dojo and combining it with the combobox makes the combobox even stronger.

comment:9 Changed 11 years ago by bill

Ok, that sounds good, Wolfram can you prepare a patch that does that? I looked at test_ComboBox.html and by calling dijit.byId("setvaluetest").item from firebug at various times I noticed that

  • when the widget is originally displayed item is null (it should be set to something)
  • selecting California from the drop down list correctly sets item
  • typing in California and then hitting the tab key... item is null (it should be set to something)

comment:10 Changed 11 years ago by wolfram

Owner: set to wolfram
Status: reopenednew

I will fix it. Meanwhile there is a nice example that shows the behaviour in real time http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/data/demos/demo_QueryReadStore_ComboBox.html (Watch out latest WebKit? seems to have an issue with the arrow-down key ... just noticed)

That the item is initially set to null is correct! There is no item selected, so what should it be set to? null is intended, since it means something like nothing, nix, nada ... or what would be your expectation here Bill?

Actually the last thing you mentioned might be something to handle additionally "typing in California and then hitting the tab key... item is null (it should be set to something)". So I would add this to the patch, otherwise its there.

comment:11 Changed 11 years ago by bill

Milestone: 1.21.3

bump enhancements to next milestone, as we prepare to close out 1.2

comment:12 Changed 11 years ago by bill

Milestone: 1.31.4

bumping 1.4 tickets to 1.5, and most 1.3 tickets to 1.4

Changed 10 years ago by wolfram

Attachment: patch6022-1.diff added

extended patch

comment:13 Changed 10 years ago by wolfram

Hi Bill, I finally ported the new feature to trunk and added the selecting of an item for the case "typing in California and then hitting the tab key... item is null (it should be set to something)" you mentioned. Does this look commitable?

comment:14 Changed 10 years ago by bill

Hi Wolfram, thanks for the patch.

Regarding setting the ComboBox/FilteringSelect value from an item:

First, test_ComboBox.html doesn't even load. Keeping the code in setValue() as is, isItem() needs to be defined for the builtin data store, and then need to make sure that automated robot tests for both ComboBox and FilteringSelect work.

The other thing though is that I'm thinking that attr('item', ...) is a better API than attr('value', item)... why overload attr('value', ...)?

Regarding getting the item value, first a couple issues:

  1. item isn't defined initially; for example, see test_ComboBox.html and do dijit.byId("setvaluetest").item in the console
  2. the arguments to store.fetch() in onBlur are hardcoded; at the least it should be using this.searchAttr instead of "name", and also should be using the queryOptions (this.ignoreCase, etc).
  3. FilteringSelect already has code somewhere that looks up the value based on the display value. Try typing "California" in the first test box in test_FilteringSelect.html and hitting tab, and notice the onChange handler call with "CA". You seem to be duplicating that code and probably now for FilteringSelect there are (unnecessarily) two fetch() calls to the datastore.

Given the asynchronous nature of data stores (although admittedly the commonly used ItemFileReadStore is synchronous), ComboBox.item won't be defined until sometime after blur... since users should really be getting the item value via attr('item') rather than accessing it directly (as per dijit standards), I'm wondering if the attr('item') should return a Deferred. That would also allow the store.fetch() call to be deferred (no pun intended) until a caller asks for the item. Thus solving issue (2) above.

I'm also thinking about initialization of FilteringSelect widgets, which is currently very inefficient because it causes a data store query on page load. For example this needs to look up the mapping from AZ to Arizona:

<input dojoType=dijit.form.FilteringSelect store=... value=AZ>

It would be more efficient for the user to specify markup like this:

<input dojoType=dijit.form.FilteringSelect store=... value=AZ displayValue=Arizona>

and then defer any data store accesses until we need to show a drop down or the user asks for the item.

comment:15 Changed 10 years ago by bill

Milestone: 1.4future

Bumping this milestone since no response.

comment:16 Changed 10 years ago by Douglas Hays

Milestone: future1.4
Owner: changed from wolfram to Douglas Hays
Status: newassigned

Changed 10 years ago by Douglas Hays

Attachment: 6022.patch added

possible fix to support attr('item',blah) and to better sync this.item with onChange

comment:17 Changed 10 years ago by Douglas Hays

wolfram, can you donate any test cycles to this patch? It's a pretty big change.

comment:18 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:19 Changed 8 years ago by bill

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