Commit ad86c212 authored by dbeam's avatar dbeam Committed by Commit bot

MD Downloads: fix search spinner when terms are the same

R=tsergeant@chromium.org
BUG=640893
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2284463002
Cr-Commit-Position: refs/heads/master@{#414643}
parent a9af01ed
...@@ -100,7 +100,10 @@ cr.define('downloads', function() { ...@@ -100,7 +100,10 @@ cr.define('downloads', function() {
*/ */
saveDangerous: chromeSendWithId('saveDangerous'), saveDangerous: chromeSendWithId('saveDangerous'),
/** @param {string} searchText What to search for. */ /**
* @param {string} searchText What to search for.
* @return {boolean} Whether |searchText| resulted in new search terms.
*/
search: function(searchText) { search: function(searchText) {
var searchTerms = ActionService.splitTerms(searchText); var searchTerms = ActionService.splitTerms(searchText);
var sameTerms = searchTerms.length == this.searchTerms_.length; var sameTerms = searchTerms.length == this.searchTerms_.length;
...@@ -111,10 +114,11 @@ cr.define('downloads', function() { ...@@ -111,10 +114,11 @@ cr.define('downloads', function() {
} }
if (sameTerms) if (sameTerms)
return; return false;
this.searchTerms_ = searchTerms; this.searchTerms_ = searchTerms;
this.loadMore(); this.loadMore();
return true;
}, },
/** /**
......
...@@ -37,3 +37,23 @@ TEST_F('ActionServiceUnitTest', 'splitTerms', function() { ...@@ -37,3 +37,23 @@ TEST_F('ActionServiceUnitTest', 'splitTerms', function() {
assertEquals(str(['a', 'b b', 'c']), assertEquals(str(['a', 'b b', 'c']),
str(ActionService.splitTerms('a "b b" c'))); str(ActionService.splitTerms('a "b b" c')));
}); });
TEST_F('ActionServiceUnitTest', 'searchWithSimilarTerms', function() {
/**
* @extends {downloads.ActionService}
* @constructor
*/
function TestActionService() {
downloads.ActionService.call(this);
}
TestActionService.prototype = {
__proto__: downloads.ActionService.prototype,
loadMore: function() { /* No chrome.send() for you! */ },
};
var actionService = new TestActionService();
assertTrue(actionService.search('a'));
assertFalse(actionService.search('a ')); // Same term + space should no-op.
});
...@@ -2250,9 +2250,10 @@ cr.define('downloads', function() { ...@@ -2250,9 +2250,10 @@ cr.define('downloads', function() {
for (var i = 0; sameTerms && i < searchTerms.length; ++i) { for (var i = 0; sameTerms && i < searchTerms.length; ++i) {
if (searchTerms[i] != this.searchTerms_[i]) sameTerms = false; if (searchTerms[i] != this.searchTerms_[i]) sameTerms = false;
} }
if (sameTerms) return; if (sameTerms) return false;
this.searchTerms_ = searchTerms; this.searchTerms_ = searchTerms;
this.loadMore(); this.loadMore();
return true;
}, },
show: chromeSendWithId('show'), show: chromeSendWithId('show'),
undo: chrome.send.bind(chrome, 'undo') undo: chrome.send.bind(chrome, 'undo')
...@@ -6802,8 +6803,7 @@ cr.define('downloads', function() { ...@@ -6802,8 +6803,7 @@ cr.define('downloads', function() {
}, },
onSearchChanged_: function(event) { onSearchChanged_: function(event) {
var actionService = downloads.ActionService.getInstance(); var actionService = downloads.ActionService.getInstance();
actionService.search(event.detail); if (actionService.search(event.detail)) this.spinnerActive = actionService.isSearching();
this.spinnerActive = actionService.isSearching();
this.updateClearAll_(); this.updateClearAll_();
}, },
onOpenDownloadsFolderTap_: function() { onOpenDownloadsFolderTap_: function() {
......
<link rel="import" href="chrome://downloads/action_service.html">
<link rel="import" href="chrome://resources/html/assert.html"> <link rel="import" href="chrome://resources/html/assert.html">
<link rel="import" href="chrome://resources/html/cr.html"> <link rel="import" href="chrome://resources/html/cr.html">
<link rel="import" href="chrome://resources/html/polymer.html"> <link rel="import" href="chrome://resources/html/polymer.html">
......
...@@ -87,8 +87,8 @@ cr.define('downloads', function() { ...@@ -87,8 +87,8 @@ cr.define('downloads', function() {
*/ */
onSearchChanged_: function(event) { onSearchChanged_: function(event) {
var actionService = downloads.ActionService.getInstance(); var actionService = downloads.ActionService.getInstance();
actionService.search(/** @type {string} */ (event.detail)); if (actionService.search(/** @type {string} */ (event.detail)))
this.spinnerActive = actionService.isSearching(); this.spinnerActive = actionService.isSearching();
this.updateClearAll_(); this.updateClearAll_();
}, },
......
...@@ -2854,17 +2854,10 @@ paper-button { ...@@ -2854,17 +2854,10 @@ paper-button {
width: 100%; width: 100%;
} }
/* :host-context([dir=ltr]):host([narrow][showing-search]) #icon,
* Margin needs to be animated to prevent jumping around during :host-context([dir=ltr]):host([narrow][showing-search])
* opening/closing. -webkit-margin-start is not animatable, so we have to paper-spinner-lite {
* use regular margin-left/right instead. -webkit-margin-start: 18px;
*/
:host-context([dir=ltr]):host([narrow][showing-search]) #icon {
margin-left: 18px;
}
:host-context([dir=rtl]):host([narrow][showing-search]) #icon {
margin-right: 18px;
} }
</style> </style>
<paper-spinner-lite active="[[isSpinnerShown_(spinnerActive, showingSearch)]]"> <paper-spinner-lite active="[[isSpinnerShown_(spinnerActive, showingSearch)]]">
......
...@@ -7,7 +7,21 @@ suite('toolbar tests', function() { ...@@ -7,7 +7,21 @@ suite('toolbar tests', function() {
var toolbar; var toolbar;
setup(function() { setup(function() {
/**
* @constructor
* @extends {downloads.ActionService}
*/
function TestActionService() {
downloads.ActionService.call(this);
}
TestActionService.prototype = {
__proto__: downloads.ActionService.prototype,
loadMore: function() { /* Prevent chrome.send(). */ },
};
toolbar = document.createElement('downloads-toolbar'); toolbar = document.createElement('downloads-toolbar');
downloads.ActionService.instance_ = new TestActionService;
document.body.appendChild(toolbar); document.body.appendChild(toolbar);
}); });
...@@ -18,4 +32,15 @@ suite('toolbar tests', function() { ...@@ -18,4 +32,15 @@ suite('toolbar tests', function() {
window.dispatchEvent(new CustomEvent('resize')); window.dispatchEvent(new CustomEvent('resize'));
assertFalse(toolbar.$.more.opened); assertFalse(toolbar.$.more.opened);
}); });
test('search starts spinner', function() {
toolbar.$.toolbar.fire('search-changed', 'a');
assertTrue(toolbar.spinnerActive);
// Pretend the manager got results and set this to false.
toolbar.spinnerActive = false;
toolbar.$.toolbar.fire('search-changed', 'a '); // Same term plus a space.
assertFalse(toolbar.spinnerActive);
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment