Opened 7 years ago

Closed 7 years ago

#15726 closed defect (fixed)

[regression] FilteringSelect: "more choices" doesn't appear for async dojo.data store

Reported by: charly Owned by: bill
Priority: blocker Milestone: 1.8
Component: Dijit - Form Version: 1.8.0rc1
Keywords: Cc:
Blocked By: Blocking:

Description

Hello,

Code:

<div data-dojo-type="dojox.data.QueryReadStore" 
                data-dojo-id="store_customer"
                data-dojo-props="url:'MYURL',
                                requestMethod:'get',
                                doClientPaging:'true',
                                doClientSorting:'false'">
           </div> 
         
           <input name="fs_offercustomer"
             id="fs_offercustomer"
             value=""
             type="text"           
             data-dojo-type="dijit.form.FilteringSelect"
             data-dojo-props="store:store_customer,
                              placeHolder:'Search Customer',
                              searchDelay:400,
                              searchAttr:'name',
                              labelAttr:'name',
                              highlightMatch:'all',
                              required:false,
                              autoComplete:false, 
                              labelType:'html',
                              hasDownArrow:false,
                              trim:true,
                              pageSize:5,                             
                              fetchProperties:{start:0, count:5}">

Problem:

  1. I use FilteringSelect? with QueryReadStore?, when I return any suggestions form the store I miss an option "more choices". But when I try the same input a 2nd time I get the option.
  1. When I go to the last page, it appears only more/previous choices without list items.
  1. In Dojo 1.7.2 I don't use pageSize:5 only fetchProperties and it works but not in 1.8.0rc1.

Similar Ticket: 13339 ;

thx charly

Attachments (3)

fix_total.patch (848 bytes) - added by bill 7 years ago.
set total correctly, it needs to be either available immediately, either as a promise or a scalar value
fix_total_2.patch (1.9 KB) - added by bill 7 years ago.
fix both spots
15726.patch (2.7 KB) - added by Douglas Hays 7 years ago.
added automated testcase

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by charly

Problem:

  1. when you set labelType:'html', the input search text isn't highlighting of the entries.
  1. onmouseover-highlighting of the entries doesn't work if you use trundra, see 15200

comment:2 Changed 7 years ago by Douglas Hays

Component: Dijit - FormDojoX Data
Milestone: tbd1.8.1
Owner: changed from Douglas Hays to bill
Status: newassigned

bill, can you look at this? It's another QueryReadStore issue. The total value is coming back with the pagesize set in dojo/store/util/QueryResults.js the first time. After that, total gets reset to the real value in the compatibility API ComboBoxMixin::query::onBegin.

comment:3 Changed 7 years ago by bill

Cc: Douglas Hays added
Component: DojoX DataDijit - Form
Summary: FilteringSelect: "more choices" is awayFilteringSelect: "more choices" doesn't appear first time

It's apparently not a QueryReadStore issue since I can reproduce the problem in dijit by setting pageSize:10 on the "Artificially slowed-down data store" in dijit/tests/form/autoComplete.html. But I'll take a look anyway because I think it's in my back-compat code code in ComboBoxMixin.js to support old dojo.data stores.

comment:4 Changed 7 years ago by bill

Cc: Douglas Hays removed
Milestone: 1.8.11.8
Owner: changed from bill to Douglas Hays
Summary: FilteringSelect: "more choices" doesn't appear first time[regression] FilteringSelect: "more choices" doesn't appear for async dojo.data store

Actually, it broke from your [28065]. I have a patch that seems to fix it but I'll let you check it over, and check in. I didn't run the full regression test.

This seems like a candidate to slip into 1.8 final, because it's a serious regression, isn't it? Too bad our unit tests didn't pick up on it, we need to add a test for paging from an async store.

Changed 7 years ago by bill

Attachment: fix_total.patch added

set total correctly, it needs to be either available immediately, either as a promise or a scalar value

comment:5 Changed 7 years ago by bill

PS: Results.total should be set at the start of the query, either set to a value or to a promise.

Since results.total is not set before query() returns, and since it's not even set when the "results" promise resolves, this code in QueryResultsStore kicks in, which incorrectly sets the total to be the pageSize, assuming that pageSize <= the actual number of rows that matched the query:

if(!results.total){
        results.total = Deferred.when(results, function(results){
                return results.length;
        });
}

comment:7 Changed 7 years ago by bill

Priority: undecidedblocker

Changed 7 years ago by bill

Attachment: fix_total_2.patch added

fix both spots

comment:8 Changed 7 years ago by bill

For the record, [28065] just exposed a bug in the dojo.data (old API) support code that was there all along.

Anyway, I ran the regression and the change (fix_total_2.patch) looks OK.

Changed 7 years ago by Douglas Hays

Attachment: 15726.patch added

added automated testcase

comment:9 Changed 7 years ago by Douglas Hays

Owner: changed from Douglas Hays to bill

bill, the patch looks good. I added an automated testcase for good measure. I agree that this should go into 1.8. Refs #12373. dojox/data/demos/demo_QueryReadStore_FilteringSelect.html shows "More choices" now as well.

comment:10 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [29363]:

Fix paging through results for asynchronous dojo.data (old API) store. The return Object from query() needs to include a total attribute, either a scalar or a Promise. Fixes #15726 !strict.

Note: See TracTickets for help on using tickets.