From 6f8504d9b838f1dc34ff8f362493525a0261e9cb Mon Sep 17 00:00:00 2001 From: Eddie Ramirez Date: Wed, 19 Oct 2016 21:01:41 +0000 Subject: [PATCH] Bug fixes Magic Search This patch fixes two bugs and minor UX enhancements: - User can now delete last character in searchInput without causing the facetSelected to disappear - User can now use arrow keys normally, preventing the table to be reloaded - Menu follows search input position as the user adds more facets Closes-bug: 1618235 Closes-bug: 1635505 Change-Id: I56b980f7c6739fc4fba06c0028764b2e4e908de6 --- .../magic-search/magic-search.controller.js | 114 +++++++++++------- .../magic-search.controller.spec.js | 114 ++++++++++++------ .../widgets/magic-search/magic-search.scss | 4 + .../notes/bug-1618235-59865fa0e5991e63.yaml | 8 ++ .../notes/bug-1635505-3807fd0151702a5f.yaml | 7 ++ 5 files changed, 166 insertions(+), 81 deletions(-) create mode 100644 releasenotes/notes/bug-1618235-59865fa0e5991e63.yaml create mode 100644 releasenotes/notes/bug-1635505-3807fd0151702a5f.yaml diff --git a/horizon/static/framework/widgets/magic-search/magic-search.controller.js b/horizon/static/framework/widgets/magic-search/magic-search.controller.js index 331ba869d..47a29ae58 100644 --- a/horizon/static/framework/widgets/magic-search/magic-search.controller.js +++ b/horizon/static/framework/widgets/magic-search/magic-search.controller.js @@ -109,6 +109,8 @@ var key = service.getEventCode($event); if (key === 9) { // prevent default when we can. $event.preventDefault(); + } else if (key === 8) { + backspaceKeyDown(); } } @@ -118,7 +120,6 @@ return; } ctrl.facetClicked(0, '', ctrl.filteredObj[0].name); - setSearchInput(''); } else { if (angular.isUndefined(ctrl.filteredOptions) || ctrl.filteredOptions.length !== 1) { @@ -130,7 +131,11 @@ } function escapeKeyUp() { - setMenuOpen(false); + if (angular.isDefined(ctrl.facetSelected)) { + setMenuOpen(true); + } else { + setMenuOpen(false); + } resetState(); var textFilter = ctrl.textSearch; if (angular.isUndefined(textFilter)) { @@ -142,43 +147,71 @@ function enterKeyUp() { var searchVal = searchInput.val(); // if tag search, treat as regular facet - if (ctrl.facetSelected && angular.isUndefined(ctrl.facetSelected.options)) { - var curr = ctrl.facetSelected; - curr.name = curr.name.split('=')[0] + '=' + searchVal; - curr.label[1] = searchVal; - ctrl.currentSearch.push(curr); - resetState(); - emitQuery(); + if (searchVal !== '') { + if (ctrl.facetSelected) { + var curr = ctrl.facetSelected; + curr.name = curr.name.split('=')[0] + '=' + searchVal; + curr.label[1] = searchVal; + ctrl.currentSearch.push(curr); + resetState(); + emitQuery(); + setMenuOpen(true); + } else { + // if text search treat as search + ctrl.currentSearch = ctrl.currentSearch.filter(notTextSearch); + ctrl.currentSearch.push(service.getTextFacet(searchVal, $scope.strings.text)); + $scope.$apply(); + setMenuOpen(true); + setSearchInput(''); + emitTextSearch(searchVal); + ctrl.textSearch = searchVal; + } + } else if (ctrl.isMenuOpen) { setMenuOpen(false); } else { - // if text search treat as search - ctrl.currentSearch = ctrl.currentSearch.filter(notTextSearch); - ctrl.currentSearch.push(service.getTextFacet(searchVal, $scope.strings.text)); - $scope.$apply(); - setMenuOpen(false); - setSearchInput(''); - emitTextSearch(searchVal); - ctrl.textSearch = searchVal; + setMenuOpen(true); } ctrl.filteredObj = ctrl.unusedFacetChoices; } + function backspaceKeyDown() { + var searchVal = searchInput.val(); + if (searchVal === '') { + if (ctrl.currentSearch.length > 0 && angular.isUndefined(ctrl.facetSelected)) { + ctrl.removeFacet(ctrl.currentSearch.length - 1); + setMenuOpen(true); + } else { + escapeKeyUp(); + } + } + } + + function backspaceKeyUp() { + var searchVal = searchInput.val(); + // if there's no current search and facet selected, then clear all search + if (searchVal === '' && angular.isUndefined(ctrl.facetSelected)) { + if (ctrl.currentSearch.length === 0) { + ctrl.clearSearch(); + } else { + resetState(); + emitTextSearch(ctrl.textSearch || ''); + } + } else { + filterFacets(searchVal); + } + } + + function deleteKeyUp() { + return backspaceKeyUp(); + } + function notTextSearch(item) { return item.name.indexOf('text') !== 0; } function defaultKeyUp() { var searchVal = searchInput.val(); - if (searchVal === '') { - ctrl.filteredObj = ctrl.unusedFacetChoices; - $scope.$apply(); - emitTextSearch(''); - if (ctrl.facetSelected && angular.isUndefined(ctrl.facetSelected.options)) { - resetState(); - } - } else { - filterFacets(searchVal); - } + filterFacets(searchVal); } function keyUpHandler($event) { // handle ctrl-char input @@ -186,7 +219,13 @@ return; } var key = service.getEventCode($event); - var handlers = { 9: tabKeyUp, 27: escapeKeyUp, 13: enterKeyUp }; + var handlers = { + 8: backspaceKeyUp, + 9: tabKeyUp, + 27: escapeKeyUp, + 13: enterKeyUp, + 46: deleteKeyUp + }; if (handlers[key]) { handlers[key](); } else { @@ -208,16 +247,10 @@ return; } if (searchVal === '') { - ctrl.filteredObj = ctrl.unusedFacetChoices; - $scope.$apply(); - emitTextSearch(''); - if (ctrl.facetSelected && angular.isUndefined(ctrl.facetSelected.options)) { - resetState(); - } return; } - // Backspace, Delete - if (key !== 8 && key !== 46) { + // Backspace, Delete and arrow keys + if (key !== 8 && key !== 46 && !(key >= 37 && key <= 40)) { filterFacets(searchVal); } } @@ -257,7 +290,6 @@ } function facetClickHandler($index) { - setMenuOpen(false); var facet = ctrl.filteredObj[$index]; var label = facet.label; if (angular.isArray(label)) { @@ -268,12 +300,8 @@ if (angular.isDefined(facet.options)) { ctrl.filteredOptions = ctrl.facetOptions = facet.options; setMenuOpen(true); - } - var searchVal = searchInput.val(); - if (searchVal) { - ctrl.currentSearch = ctrl.currentSearch.filter(notTextSearch); - ctrl.currentSearch.push(service.getTextFacet(searchVal, $scope.strings.text)); - ctrl.textSearch = searchVal; + } else { + setMenuOpen(false); } setSearchInput(''); setPrompt(''); diff --git a/horizon/static/framework/widgets/magic-search/magic-search.controller.spec.js b/horizon/static/framework/widgets/magic-search/magic-search.controller.spec.js index a5c4adc03..2652a53b5 100644 --- a/horizon/static/framework/widgets/magic-search/magic-search.controller.spec.js +++ b/horizon/static/framework/widgets/magic-search/magic-search.controller.spec.js @@ -148,7 +148,7 @@ expect(keyDownHandler).toBeDefined(); }); - it("does nothing with keys other than 9", function() { + it("does nothing with keys other than 9 and 8", function() { spyOn(evt, 'preventDefault'); keyDownHandler(evt); expect(evt.preventDefault).not.toHaveBeenCalled(); @@ -160,6 +160,30 @@ keyDownHandler(evt); expect(evt.preventDefault).toHaveBeenCalled(); }); + + describe("'Backspace' key", function() { + beforeEach(function() { + evt.keyCode = 8; + }); + + it("removes last facet if length larger than 1 and searchVal empty", function() { + spyOn(searchInput, 'val').and.returnValue(''); + spyOn(ctrl, 'removeFacet'); + delete ctrl.facetSelected; + ctrl.currentSearch = [{name: 'name=foo'}, {name: 'flavor=m1'}, {name: 'key=value'}]; + keyDownHandler(evt); + $timeout.flush(); + expect(ctrl.removeFacet).toHaveBeenCalledWith(2); + }); + + it("removes selectedFacet if searchVal is empty", function() { + spyOn(searchInput, 'val').and.returnValue(''); + ctrl.facetSelected = {name: 'waldo=undefined', label: ['a']}; + keyDownHandler(evt); + $timeout.flush(); + expect(ctrl.facetSelect).toBeUndefined(); + }); + }); }); describe("keyup handler", function() { @@ -181,6 +205,34 @@ expect(scope.$emit).not.toHaveBeenCalled(); }); + describe("'Backspace' key", function() { + + beforeEach(function() { + evt.keyCode = 8; + }); + + it("calls clearSearch if facetSelected undefined and currentSearch empty", function() { + spyOn(searchInput, 'val').and.returnValue(''); + spyOn(ctrl, 'clearSearch'); + delete ctrl.facetSelected; + ctrl.currentSearch = []; + keyUpHandler(evt); + expect(ctrl.clearSearch).toHaveBeenCalled(); + }); + + it("emits textSearch if facetSeleted undefined and currentSearch not empty", function() { + spyOn(searchInput, 'val').and.returnValue(''); + spyOn(scope, '$emit'); + delete ctrl.facetSelected; + ctrl.currentSearch = [{name: 'textstuff'}, {name: 'texting'}]; + scope.filter_keys = [1,2,3]; + keyUpHandler(evt); + expectResetState(); + expect(scope.$emit).toHaveBeenCalledWith(magicSearchEvents.TEXT_SEARCH, '', [1, 2, 3]); + }); + + }); + describe("'Escape' key", function() { beforeEach(function() { evt.keyCode = 27; @@ -298,6 +350,26 @@ expect(ctrl.currentSearch).toEqual([{name: 'nontext'}, {name: 'nottext'}, {name: 'text=searchval', label: ['stringtext', 'searchval']}]); }); + + it("opens menu when searchVal is an empty string", function() { + ctrl.isMenuOpen = false; + spyOn(searchInput, 'val').and.returnValue(''); + spyOn(scope, '$emit'); + scope.filter_keys = [1,2,3]; + keyUpHandler(evt); + $timeout.flush(); + expect(ctrl.isMenuOpen).toBe(true); + }); + + it("emits a Query when not empty string and a facet is selected", function() { + spyOn(searchInput, 'val').and.returnValue('foo'); + ctrl.currentSearch = []; + keyUpHandler(evt); + $timeout.flush(); + expect(ctrl.currentSearch).toEqual([{name: 'waldo=foo', label: ['a', 'foo']}]); + expectResetState(); + expect(ctrl.isMenuOpen).toBe(true); + }); }); describe("Any other key", function() { @@ -314,14 +386,6 @@ magicSearchEvents.TEXT_SEARCH, '', ['a', 'b', 'c']); }); - it("resets state if facetSelected and no options", function() { - spyOn(searchInput, 'val').and.returnValue(''); - scope.filter_keys = ['a', 'b', 'c']; - ctrl.facetSelected = {}; - keyUpHandler(evt); - expectResetState(); - }); - it("filters if there is a search term", function() { spyOn(searchInput, 'val').and.returnValue('searchterm'); spyOn(scope, '$emit'); @@ -357,36 +421,9 @@ it("opens menu when searchVal is a space", function() { evt.which = 32; - spyOn(searchInput, 'val').and.returnValue(' '); - spyOn(scope, '$emit'); - scope.filter_keys = [1,2,3]; keyPressHandler(evt); - expect(scope.$emit).toHaveBeenCalledWith( - magicSearchEvents.TEXT_SEARCH, ' ', [1,2,3]); - }); - - it("opens menu when searchVal is an empty string", function() { - spyOn(searchInput, 'val').and.returnValue(''); - spyOn(scope, '$emit'); - evt.which = 13; // not alter search - scope.filter_keys = [1,2,3]; - keyPressHandler(evt); - expect(scope.$emit).toHaveBeenCalledWith( - magicSearchEvents.TEXT_SEARCH, '', [1,2,3]); - }); - - it("resets state when ctrl.facetSelected exists but has no options", function() { - spyOn(searchInput, 'val').and.returnValue(''); - spyOn(scope, '$emit'); - evt.which = 13; // not alter search - scope.filter_keys = [1,2,3]; - ctrl.facetSelected = {}; - ctrl.facetOptions = {}; - ctrl.filteredOptions = {}; - keyPressHandler(evt); - expect(scope.$emit).toHaveBeenCalledWith( - magicSearchEvents.TEXT_SEARCH, '', [1,2,3]); - expectResetState(); + $timeout.flush(); + expect(ctrl.isMenuOpen).toBe(true); }); it("filters when searchval has content and key is not delete/backspace", function() { @@ -406,6 +443,7 @@ keyPressHandler(evt); expect(scope.$emit).not.toHaveBeenCalled(); }); + }); describe("optionClicked", function() { diff --git a/horizon/static/framework/widgets/magic-search/magic-search.scss b/horizon/static/framework/widgets/magic-search/magic-search.scss index 8fcc4791c..17630776a 100644 --- a/horizon/static/framework/widgets/magic-search/magic-search.scss +++ b/horizon/static/framework/widgets/magic-search/magic-search.scss @@ -89,6 +89,10 @@ .search-entry { flex: 1 0 auto; } + + .dropdown-menu { + left: initial; + } } .fi-filter { diff --git a/releasenotes/notes/bug-1618235-59865fa0e5991e63.yaml b/releasenotes/notes/bug-1618235-59865fa0e5991e63.yaml new file mode 100644 index 000000000..bd1aa4ef9 --- /dev/null +++ b/releasenotes/notes/bug-1618235-59865fa0e5991e63.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + [`bug 1618235 `__] + User can now delete all characters typed in input search without causing + the selected facet to disappear when the last character is deleted. +other: + - Menu follows the search input position as the user adds more facets diff --git a/releasenotes/notes/bug-1635505-3807fd0151702a5f.yaml b/releasenotes/notes/bug-1635505-3807fd0151702a5f.yaml new file mode 100644 index 000000000..594e34c03 --- /dev/null +++ b/releasenotes/notes/bug-1635505-3807fd0151702a5f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1635505 `__] + Horizon now properly allows to use arrow keys inside of the input search, + without triggering a new text search that refreshes the content of the + table below.