Merge pull request #2708 from CartoDB/searchbutton

Contrast and small UX fix for the search button
CartoDB · Mar 11, 2015 · 86a7ef7 · 86a7ef7
2 parents 592af90 + 29beaef
commit 86a7ef7
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 12 deletions.
diff --git a/app/assets/stylesheets/new_common/filters.css.scss b/app/assets/stylesheets/new_common/filters.css.scss
@@ -79,7 +79,7 @@
   padding-right: 2px;
   font-size: 14px;
   line-height: 80px;
-  color: $cTypography-help;
+  color: $cTypography-linkSelected;
   background: transparent;
   border: none;
   outline: none;

diff --git a/lib/assets/javascripts/cartodb/new_dashboard/filters_view.js b/lib/assets/javascripts/cartodb/new_dashboard/filters_view.js
@@ -23,6 +23,7 @@ module.exports = cdb.core.View.extend({
 
   events: {
     'submit .js-search-form':   '_submitSearch',
+    'keydown .js-search-form':  '_onSearchKeyDown',
     'click .js-search-form':    'killEvent',
     'click .js-search-link':    '_onSearchClick',
     'click .js-clean-search':   '_onCleanSearchClick',
@@ -116,7 +117,7 @@ module.exports = cdb.core.View.extend({
     var shared = this.router.model.get('shared');
     var locked = this.router.model.get('locked');
     var liked = this.router.model.get('liked');
-    var search = this.router.model.get('q') || this.router.model.get('tag');
+    var search = this.router.model.isSearching();
     var total_entries = this.collection.total_entries;
 
     // Bind scroll
@@ -166,8 +167,7 @@ module.exports = cdb.core.View.extend({
     this.$('.Filters-inner')[ selectedItemsCount > 0 ? 'addClass' : 'removeClass' ]('items--selected');
 
     // Check if any search is applied
-    var search = this.router.model.get('q') || this.router.model.get('tag');
-    this.$('.Filters-inner')[ search ? 'addClass' : 'removeClass' ]('search--enabled');
+    this.$('.Filters-inner')[ this.router.model.isSearching() ? 'addClass' : 'removeClass' ]('search--enabled');
   },
 
   _selectAll: function(e) {
@@ -232,9 +232,22 @@ module.exports = cdb.core.View.extend({
   },
 
   _onSearchClick: function(e) {
-    if (e) this.killEvent(e);
-    this.$('.Filters-inner').addClass('search--enabled');
-    this.$('.js-search-input').focus();
+    this.killEvent(e);
+    var wasSearchInputVisible = this.$('.Filters-inner').hasClass('search--enabled');
+    this.$('.Filters-inner').toggleClass('search--enabled', !wasSearchInputVisible);
+
+    if (this.router.model.isSearching()) {
+      this._cleanSearch();
+    } else if (!wasSearchInputVisible) {
+      this.$('.js-search-input').val('');
+      this.$('.js-search-input').focus();
+    }
+  },
+
+  _onSearchKeyDown: function(e) {
+    if (e.keyCode === 27) {
+      this._onSearchClick(e);
+    }
   },
 
   // Creation actions
@@ -276,18 +289,22 @@ module.exports = cdb.core.View.extend({
   // Filter actions
 
   _onCleanSearchClick: function(e) {
-    if (e) e.preventDefault();
+    this.killEvent(e);
+    this._cleanSearch();
+  },
+
+  _submitSearch: function(e) {
+    this.killEvent(e);
     this._navigateToUrl({
-      search: '',
+      search: this.$('.js-search-input').val().trim(),
       library: this.router.model.get('library'),
       shared: this.router.model.get('shared')
     });
   },
 
-  _submitSearch: function(e) {
-    if (e) e.preventDefault();
+  _cleanSearch: function() {
     this._navigateToUrl({
-      search: this.$('.js-search-input').val().trim(),
+      search: '',
       library: this.router.model.get('library'),
       shared: this.router.model.get('shared')
     });

diff --git a/lib/assets/javascripts/cartodb/new_dashboard/router/model.js b/lib/assets/javascripts/cartodb/new_dashboard/router/model.js
@@ -63,6 +63,10 @@ module.exports = cdb.core.Model.extend({
     });
   },
 
+  isSearching: function() {
+    return this.get('q') || this.get('tag');
+  },
+
   /**
    * Append pagination page item to given array.
    */

diff --git a/lib/assets/test/spec/cartodb/new_dashboard/filters_view.spec.js b/lib/assets/test/spec/cartodb/new_dashboard/filters_view.spec.js
@@ -255,6 +255,33 @@ describe('new_dashboard/filters_view', function() {
     });
   });
 
+  describe('when click search', function() {
+    beforeEach(function() {
+      spyOn(this.router, 'navigate');
+
+      this.click = function() {
+        this.view.$('.js-search-link').click();
+      };
+    });
+
+    it('should toggle search', function() {
+      this.click();
+      expect(this.router.navigate).not.toHaveBeenCalled();
+      expect(this.innerHTML()).toContain('search--enabled');
+
+      this.click();
+      expect(this.router.navigate).not.toHaveBeenCalled();
+      expect(this.innerHTML()).not.toContain('search--enabled');
+
+      this.click(); // enable search again
+      this.router.model.set({ q: 'test-search-clean' }, { silent: true });
+      this.click();
+      expect(this.router.navigate).toHaveBeenCalled();
+      expect(this.router.navigate.calls.argsFor(0)[0]).not.toContain('search/');
+      expect(this.innerHTML()).not.toContain('search--enabled');
+    });
+  });
+
   it('should have no leaks', function() {
     this.view.render();
     expect(this.view).toHaveNoLeaks();

diff --git a/lib/assets/test/spec/cartodb/new_dashboard/router/model.spec.js b/lib/assets/test/spec/cartodb/new_dashboard/router/model.spec.js
@@ -148,4 +148,26 @@ describe("new_dashboard/router/model", function() {
       });
     });
   });
+
+  describe('.isSearching', function() {
+    it('should return true if set to a search or tag query', function() {
+      this.model.set({
+        q: '',
+        tag: ''
+      });
+      expect(this.model.isSearching()).toBeFalsy();
+
+      this.model.set({
+        q: 'foobar',
+        tag: ''
+      });
+      expect(this.model.isSearching()).toBeTruthy();
+
+      this.model.set({
+        q: '',
+        tag: 'some-tag'
+      });
+      expect(this.model.isSearching()).toBeTruthy();
+    });
+  });
 });