Skip to content

Commit

Permalink
Reduces dropdown flicker during a remote search. Fixes twitter#176.
Browse files Browse the repository at this point in the history
The behaviour of Bloodhound has been changed such that the
 #get callback is only run if matches in the search index
 (i.e. local or prefetched) have been found, or if we're
 not making a network request.

The behaviour of Typeahead has been changed such that the
dropdown is only cleared by the query changing if the query
becomes an empty string.
  • Loading branch information
mjstallard committed Feb 24, 2014
1 parent 7a58770 commit 8702694
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 23 deletions.
15 changes: 9 additions & 6 deletions src/bloodhound/bloodhound.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,23 @@ var Bloodhound = window.Bloodhound = (function() {
},

get: function get(query, cb) {
var that = this, matches, cacheHit = false;
var that = this, matches = [], cacheHit = false;

matches = this.index.get(query);
matches = this.sorter(matches).slice(0, this.limit);
if(!this.index.isEmpty()) {
matches = this.index.get(query);
matches = this.sorter(matches).slice(0, this.limit);
}

if (matches.length < this.limit && this.transport) {
cacheHit = this._getFromRemote(query, returnRemoteMatches);
}

// if a cache hit occurred, skip rendering local matches
// because the rendering of local/remote matches is already
// in the event loop
!cacheHit && cb && cb(matches);
// in the event loop.
// if we don't have any local matches and we're going to the
// network, don't render unnecessarily.
!cacheHit && (matches.length > 0 || !this.transport) && cb && cb(matches);

function returnRemoteMatches(remoteMatches) {
var matchesWithBackfill = matches.slice(0);
Expand All @@ -184,7 +188,6 @@ var Bloodhound = window.Bloodhound = (function() {
// the remote results and can break out of the each loop
return matchesWithBackfill.length < that.limit;
});

cb && cb(that.sorter(matchesWithBackfill));
}
},
Expand Down
4 changes: 4 additions & 0 deletions src/bloodhound/search_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ var SearchIndex = (function() {

serialize: function serialize() {
return { datums: this.datums, trie: this.trie };
},

isEmpty: function isEmpty() {
return this.datums.length === 0;
}
});

Expand Down
4 changes: 3 additions & 1 deletion src/typeahead/typeahead.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,9 @@ var Typeahead = (function() {

_onQueryChanged: function onQueryChanged(e, query) {
this.input.clearHint();
this.dropdown.empty();
if(query.length === 0) {
this.dropdown.empty();
}
query.length >= this.minLength && this.dropdown.update(query);
this.dropdown.open();
this._setLanguageDirection();
Expand Down
90 changes: 76 additions & 14 deletions test/bloodhound_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,25 +283,87 @@ describe('Bloodhound', function() {
}
});

it('should call #get callback once if cache hit', function() {
var spy = jasmine.createSpy();
describe('when there is not matching data in the search index', function() {
beforeEach(function() {
this.bloodhound = new Bloodhound({
datumTokenizer: datumTokenizer,
queryTokenizer: queryTokenizer,
remote: '/test?q=%QUERY',
local: { value: 'not an animal' }
});

this.bloodhound = new Bloodhound({
datumTokenizer: datumTokenizer,
queryTokenizer: queryTokenizer,
remote: '/test?q=%QUERY'
this.bloodhound.initialize();
});
this.bloodhound.initialize();
this.bloodhound.transport.get.andCallFake(fakeGet);

this.bloodhound.get('dog', spy);
it('should call #get callback once if there is a cache hit', function() {
var spy = jasmine.createSpy();

expect(spy.callCount).toBe(1);
this.bloodhound.transport.get.andCallFake(fakeGetWithCacheHit);
this.bloodhound.get('dog', spy);

function fakeGet(url, o, cb) {
cb(fixtures.data.animals);
return true;
}
expect(spy.callCount).toBe(1);

function fakeGetWithCacheHit(url, o, cb) {
cb(fixtures.data.animals);
return true;
}
});

it('should call #get callback once if there is a cache miss', function() {
var spy = jasmine.createSpy();

this.bloodhound.transport.get.andCallFake(fakeGetWithCacheMiss);
this.bloodhound.get('dog', spy);

expect(spy.callCount).toBe(1);

function fakeGetWithCacheMiss(url, o, cb) {
cb(fixtures.data.animals);
return false;
}
});

});

describe('when there is matching data in the search index', function() {
beforeEach(function() {
this.bloodhound = new Bloodhound({
datumTokenizer: datumTokenizer,
queryTokenizer: queryTokenizer,
remote: '/test?q=%QUERY',
local: { value: 'dog' }
});

this.bloodhound.initialize();
});

it('should call the #get callback twice if there is a cache miss', function() {
var spy = jasmine.createSpy();

this.bloodhound.transport.get.andCallFake(fakeGetWithCacheMiss);
this.bloodhound.get('dog', spy);

expect(spy.callCount).toBe(2);

function fakeGetWithCacheMiss(url, o, cb) {
cb(fixtures.data.animals);
return false;
}
});

it('should call the #get callback once if there is a cache hit', function() {
var spy = jasmine.createSpy();

this.bloodhound.transport.get.andCallFake(fakeGetWithCacheHit);
this.bloodhound.get('dog', spy);

expect(spy.callCount).toBe(1);

function fakeGetWithCacheHit(url, o, cb) {
cb(fixtures.data.animals);
return true;
}
});
});
});

Expand Down
12 changes: 12 additions & 0 deletions test/search_index_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ describe('SearchIndex', function() {
expect(this.searchIndex.get('wtf')).toEqual([]);
});

it('#isEmpty should return false if the index is not empty', function() {
expect(this.searchIndex.isEmpty()).not.toBeTruthy();
});

it('#isEmpty should return true if the index is empty', function() {
this.searchIndex = new SearchIndex({
datumTokenizer: datumTokenizer,
queryTokenizer: queryTokenizer
});

expect(this.searchIndex.isEmpty()).toBeTruthy();
});
// helper functions
// ----------------

Expand Down
10 changes: 8 additions & 2 deletions test/typeahead_view_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,18 @@ describe('Typeahead', function() {
expect(this.input.clearHint).toHaveBeenCalled();
});

it('should empty dropdown', function() {
this.input.trigger('queryChanged', testDatum.value);
it('should empty dropdown if the query is empty', function() {
this.input.trigger('queryChanged', '');

expect(this.dropdown.empty).toHaveBeenCalled();
});

it('should empty dropdown if the query is non-empty', function() {
this.input.trigger('queryChanged', testDatum.value);

expect(this.dropdown.empty).not.toHaveBeenCalled();
});

it('should update dropdown', function() {
this.input.trigger('queryChanged', testDatum.value);

Expand Down

0 comments on commit 8702694

Please sign in to comment.