Reduces dropdown flicker during a remote search.

This provides a fix for [twitter#176](/../..twitter/issues/176). 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.
mjstallard · Feb 24, 2014 · 9cd31b4 · 9cd31b4
1 parent 7a58770
commit 9cd31b4
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 34 deletions.
diff --git a/dist/bloodhound.js b/dist/bloodhound.js
@@ -396,6 +396,9 @@
                     datums: this.datums,
                     trie: this.trie
                 };
+            },
+            isEmpty: function isEmpty() {
+                return this.datums.length === 0;
             }
         });
         return SearchIndex;
@@ -610,13 +613,15 @@
                 this.index.add(data);
             },
             get: function get(query, cb) {
-                var that = this, matches, cacheHit = false;
-                matches = this.index.get(query);
-                matches = this.sorter(matches).slice(0, this.limit);
+                var that = this, matches = [], cacheHit = false;
+                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);
                 }
-                !cacheHit && cb && cb(matches);
+                !cacheHit && (matches.length > 0 || !this.transport) && cb && cb(matches);
                 function returnRemoteMatches(remoteMatches) {
                     var matchesWithBackfill = matches.slice(0);
                     _.each(remoteMatches, function(remoteMatch) {

diff --git a/dist/typeahead.bundle.js b/dist/typeahead.bundle.js
@@ -396,6 +396,9 @@
                     datums: this.datums,
                     trie: this.trie
                 };
+            },
+            isEmpty: function isEmpty() {
+                return this.datums.length === 0;
             }
         });
         return SearchIndex;
@@ -610,13 +613,15 @@
                 this.index.add(data);
             },
             get: function get(query, cb) {
-                var that = this, matches, cacheHit = false;
-                matches = this.index.get(query);
-                matches = this.sorter(matches).slice(0, this.limit);
+                var that = this, matches = [], cacheHit = false;
+                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);
                 }
-                !cacheHit && cb && cb(matches);
+                !cacheHit && (matches.length > 0 || !this.transport) && cb && cb(matches);
                 function returnRemoteMatches(remoteMatches) {
                     var matchesWithBackfill = matches.slice(0);
                     _.each(remoteMatches, function(remoteMatch) {
@@ -1457,7 +1462,9 @@
             },
             _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();

diff --git a/dist/typeahead.bundle.min.js b/dist/typeahead.bundle.min.js
diff --git a/dist/typeahead.jquery.js b/dist/typeahead.jquery.js
@@ -929,7 +929,9 @@
             },
             _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();

diff --git a/src/bloodhound/bloodhound.js b/src/bloodhound/bloodhound.js
@@ -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);
@@ -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));
       }
     },

diff --git a/src/bloodhound/search_index.js b/src/bloodhound/search_index.js
@@ -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;
     }
   });
 

diff --git a/src/typeahead/typeahead.js b/src/typeahead/typeahead.js
@@ -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();

diff --git a/test/bloodhound_spec.js b/test/bloodhound_spec.js
@@ -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;
+        }
+      });
     });
   });
 

diff --git a/test/search_index_spec.js b/test/search_index_spec.js
@@ -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
   // ----------------
 

diff --git a/test/typeahead_view_spec.js b/test/typeahead_view_spec.js
@@ -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);