Store API support in CheckedMultiSelect

[adros]

CheckedMultiSelect? still does not support new dojo/store API even if _FormSelectWidget already does.

I have attached patch file which will solve this issue.

[bill]

Although, that will break support for the old dojo.data stores. Maybe it should branch, or the patch should wait until 2.0?

[adros]

Why would it break support for old dojo.data? 'setStore' method of CheckedMultiSelect? calls inherited method from _FormSelectWidget, which mixes in 'get' and 'query' methods if dojo.data store was passed in.

[bill]

Oh OK. See also #16828 though. I imagine that value[0] won't work for dojo/store. The problem is there's no test case for your setStore() so I can't tell if your change works or not. If you want to add an automated test to dojox/form/tests/test_CheckedMultiSelect.html that would be great.

[adros]

Patch for automated test

[adros]

Attachment (test_CheckedMultiSelect.html.diff) added by ticket reporter.

[bill]

Thanks for the test file patch. Unfortunately the test file is not really testing setStore(), either for the old dojo.data stores or the new dojo.store support. To prove that, I can erase the whole method and all the tests still pass.

Maybe we just shouldn't be supporting the store having initially selected items?

My other concern is that the query in setStore() is hardcoded to {selected: true}, so it will only work for a certain structure of data, and possibly only for memory stores, not for stores with a different query syntax.

[adros]

Maybe the hardcoded query should be replaced for some param in fetchArgs, e.g. 'selectdQuery'. And only if this param is provided in fetchArgs, the store would be queried for selected items.

[bill]

Yes, maybe. But then, that option should really be in dijit/form/MultiSelect and dijit/form/Select too.

Note that _FormSelectWidget (the base class) has lots of fancy code for watching changes to the data store and reflecting them into the widget, so if we supported the data store controlling which values were selected, we would probably want changes to the data store about which items were selected to be reflected to the widget, and in reverse too.... anyway it gets complicated and is perhaps a better job for dojox/mvc than for the widgets to do themselves.

So for now I'm just going to remove that method completely. It fixes support for dojo/store and removes code that according to #16828 wasn't working anyway.

[bill]

In [30865]:

Remove setStore() override method. It was there to let the store indicate which items were originally selected, but didn't seem to work correctly (refs #16828), and broke support for the new dojo/store API. It's probably something that should be handled by dojox/mvc anyway. Fixes #16815 !strict. Thanks to adros (CLA on file) for the patch to the test.