Opened 16 years ago
Closed 15 years ago
#1956 closed enhancement (duplicate)
dojo.widget.incrementalFinderDataProvider with "backoff" (prevents many requests)
Reported by: | Owned by: | bill | |
---|---|---|---|
Priority: | high | Milestone: | 1.0 |
Component: | Dijit | Version: | 0.4 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
dojo.widget.incrementalComboBoxDataProvider = function(/*String*/ url, /*Number*/ limit, /*Number*/ timeout){
this.searchUrl = url; this.inFlight = false; this.activeRequest = null; this.allowCache = false; this.timeout = null; this.cache = {};
this.init = function(/*Widget*/ cbox){
this.searchUrl = cbox.dataUrl;
};
this.addToCache = function(/*String*/ keyword, /*Array*/ data){
if(this.allowCache){
this.cache[keyword] = data;
}
};
this.startSearch = function(/*String*/ searchStr, /*String*/ type, /*Boolean*/ ignoreLimit){
if(this.inFlight){
this.timeout = dojo.lang.setTimeout(this,this.startSearch,20,searchStr,type,ignoreLimit); return;
} if(this.timeout) {
dojo.lang.clearTimeout(this.timeout); this.timeout=null;
}
this.performSearch(searchStr, type, ignoreLimit);
};
this.performSearch = function(/*String*/ searchStr, /*String*/ type, /*Boolean*/ ignoreLimit){
var tss = encodeURIComponent(searchStr); var realUrl = dojo.string.substituteParams(this.searchUrl, {"searchString": tss}); var _this = this; var request = dojo.io.bind({
url: realUrl, method: "get", mimetype: "text/json", load: function(type, data, evt){
_this.inFlight = false; if(!dojo.lang.isArray(data)){
var arrData = []; for(var key in data){
arrData.push([data[key], key]);
} data = arrData;
} _this.addToCache(searchStr, data); _this.provideSearchResults(data);
}
}); this.inFlight = true;
};
};
Attachments (8)
Change History (25)
comment:1 Changed 16 years ago by
Description: | modified (diff) |
---|
- Have you filed a CLA (www.dojotoolkit.org/icla.txt)?
- Please attach a diff file against the latest dojo 0.4.1RC code
Changed 16 years ago by
comment:2 Changed 16 years ago by
- Yes i have -- faxed this morning (is there anything else i need to do WRT this?)
- couldn't find tagged version (probably my svn ignorance) so i diffed against trunk (hope this is ok)
comment:3 Changed 16 years ago by
Owner: | changed from bill to alex |
---|---|
Status: | new → assigned |
a good chunk of this patch is whitespace noise. Is that intended? I'll merge it, but I'd prefer a cleaner patch.
comment:4 Changed 16 years ago by
This isn't a patch against the latest. (Well technically it is, but...) He took an old version of the incrementalDataProvider and modified that, instead of modifying the newest version. So it basically lost all the recent changes. We need a patch against the newest version.
Changed 16 years ago by
Attachment: | ComboBox.js-Diff added |
---|
comment:6 Changed 16 years ago by
Allright I've rewritten the code against latest (i apologize -- no excuse for that last diff).
Couple of problems
- there's still lots of whitespace in the diff file after the incrementalComboBoxDataProvider class which is the only thing i changed. Any suggestions as to how to prevent this (bit of an SVN newbie)?
- I've tried writing a test for this however since there's no latency in file requests it's difficult to exhibit the request limiting code, any suggestions as to how to fake out latency?
comment:7 Changed 16 years ago by
Milestone: | 0.4.1 → 0.5 |
---|
Hi Izaak,
Looks like the whitespace changes were actually things you fixed (IE, the original file had spurious whitespace). I checked in the whitespace changes as a separate checkin.
For testing, I'd suggest writing a php file. Although dojo is purely client side we can stick a few PHP files in the test directory as necessary.
As far as the algorithm goes, I'm not sure this makes sense. If I type the letter "a" and then type the letter "l", but it takes a long time to search for all words starting w/the letter "a", then eventually a drop down will be displayed with all the words that start with "a", even though I've already typed in "al". The current code will fire off request for "a" and "al" (as I type "a", and then "l"), and then just ignore the return value from the "a" request.
comment:8 Changed 16 years ago by
There may be cases where if the servers request is fast enough that 20msec value should probably be changed (I originally was going to tie this into the server response time). This number seemed to work reasonably well where the server response time was around 0.5 seconds.
What I've been trying to achieve was if somebody types "abc" quickly the b request basically doesn't get sent. It sounds like the 20msec setting is too long? I'd like to show the user results if they hesitate, but prevent unnecessary requests...
Perhaps i should build a buffer and always only send after ever X msec (where X is sufficiently small that they don't feel like it's slow)?
WRT Testing Is there any documentation/examples as to set that up I can look at (I presume there's some kind of server that gets instantiated to serve up these php files).
comment:9 Changed 16 years ago by
FYI i have had slightlyoff (from #dojo) have a look at it, seemed to think it was allright.
Also behavior where typing "a" then "l" and the "a" request getting ignored exists prior to patch.
Changed 16 years ago by
Attachment: | ComboBox.2.js-Diff added |
---|
Changed 16 years ago by
Attachment: | remoteComboBoxData.php added |
---|
PHP file for testing request limiting functionality
Changed 16 years ago by
Attachment: | test_ComboBox.html-diff added |
---|
diff of test_ComboBox.html from dojo/tests/widgets
comment:11 Changed 16 years ago by
ok please ignore last diff -- i've tested it out with different latiencies, i'm not convinced it works...
Changed 16 years ago by
Attachment: | ComboBox.3.js-Diff added |
---|
combo box with request prevention (no timeouts)
Changed 16 years ago by
Attachment: | remoteComboBoxData.2.php added |
---|
PHP file for testing request limiting functionality
Changed 16 years ago by
Attachment: | test_ComboBox.2.html-diff added |
---|
test for combBox with request limiting behaviour
comment:13 Changed 16 years ago by
Ok this one seems to avoid race conditions and the tests is reasonably configurable
comment:14 Changed 16 years ago by
I have done some loose tests on this with the ComboBox? searchDelay set to 0 to see if it had an effect; it did not (beyond speeding up initial requests)
comment:15 Changed 15 years ago by
Owner: | changed from alex to haysmark |
---|---|
Status: | assigned → new |
Combobox/Select? have been completely rewritten; not sure if this is even an issue anymore. But we should have some kind of backoff to prevent too many requests going out. Assigning to Mark to consider.
comment:16 Changed 15 years ago by
Component: | Widgets → Dijit |
---|---|
Milestone: | 0.9 → 1.0 |
Owner: | changed from haysmark to bill |
comment:17 Changed 15 years ago by
Resolution: | → duplicate |
---|---|
Status: | new → closed |
This ticket is really long and the patch is no longer valid because Combobox has been rewritten. Replaced with #4102.