MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

Adds a processing spinner which is visible when the search bar is open,
and hooks it into MD History so that it is shown when loading data from
a search. It is not shown when incrementally loading new data in the
infinite list.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/99778ce52abf223bd279c42669a9d9049f0f9fdf
Cr-Commit-Position: refs/heads/master@{#398476}

[tsergeant, 2016-06-03 06:00:26]

Description was changed from

==========
MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

Adds a processing spinner which is visible when the search bar is open,
and hooks it into MD History so that it is shown when loading data from
a search. It is not shown when incrementally loading new data in the
infinite list.

BUG=610609
==========

to

==========
MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

Adds a processing spinner which is visible when the search bar is open,
and hooks it into MD History so that it is shown when loading data from
a search. It is not shown when incrementally loading new data in the
infinite list.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2016-06-06 04:32:02]

Patchset #1 (id:1) has been deleted

[tsergeant, 2016-06-06 04:33:15]

tsergeant@chromium.org changed reviewers:
+ calamity@chromium.org

[tsergeant, 2016-06-06 04:33:16]

+calamity

Note that I've changed it to only show the spinner on a new, non-incremental
search.

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:214:
syncedTabsSelected_(selectedPage) {
From the Dept. Of Why Did This Ever Work, apparently this is valid syntax to
declare a function?

[calamity, 2016-06-07 04:46:05]

lgtm.

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:214:
syncedTabsSelected_(selectedPage) {
On 2016/06/06 04:33:16, tsergeant wrote:
> From the Dept. Of Why Did This Ever Work, apparently this is valid syntax to
> declare a function?

Oops + wat

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:222: * True if the backend is
searching using a new search term.
This is a bit weird. It's not immediately obvious from the method name that we
care about new non-incremental searches. isSearchSpinnerShown_ or
shouldShowSpinner_ perhaps? Then it makes sense that there would be some policy
described in the definition.

https://codereview.chromium.org/2032313002/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html
(right):

https://codereview.chromium.org/2032313002/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar_search_field.html:5: <link
rel="import"
href="chrome://resources/polymer/v1_0/paper-spinner/paper-spinner-lite.html">
Wew.

[tsergeant, 2016-06-07 06:50:28]

Patchset #2 (id:40001) has been deleted

[tsergeant, 2016-06-07 06:54:55]

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:222: * True if the backend is
searching using a new search term.
On 2016/06/07 04:46:04, calamity wrote:
> This is a bit weird. It's not immediately obvious from the method name that we
> care about new non-incremental searches. isSearchSpinnerShown_ or
> shouldShowSpinner_ perhaps? Then it makes sense that there would be some
policy
> described in the definition.

Done. As discussed, also renamed the `processing` property.

[tsergeant, 2016-06-07 06:57:30]

tsergeant@chromium.org changed reviewers:
+ dpapad@chromium.org

[tsergeant, 2016-06-07 06:57:31]

+dpapad for cr_toolbar/

[tsergeant, 2016-06-07 07:22:54]

Patchset #3 (id:80001) has been deleted

[dpapad, 2016-06-08 00:07:36]

LGTM with nits.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:44: incremental: false,
Shouldn't the QueryState typedef also be upadated?

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:69: ready: function() {
Nit:
/** @override */

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:210: getSelectedPage:
function(selectedPage, range) {
@param, @return missing

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:217: syncedTabsSelected_:
function(selectedPage) {
Same here.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:227: * @return {boolean}
Nit: Move the comment at lines 222-223 here, as follows.

@return {boolean} Whether a spinner should be shown
    (implies that the backend is querying a new search term).

[Dan Beam, 2016-06-08 01:18:39]

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:214:
syncedTabsSelected_(selectedPage) {
On 2016/06/07 04:46:04, calamity wrote:
> On 2016/06/06 04:33:16, tsergeant wrote:
> > From the Dept. Of Why Did This Ever Work, apparently this is valid syntax to
> > declare a function?
> 
> Oops + wat

http://es6-features.org/#MethodProperties

[tsergeant, 2016-06-08 02:52:18]

Patchset #3 (id:100001) has been deleted

[tsergeant, 2016-06-08 02:53:54]

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (left):

https://codereview.chromium.org/2032313002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:214:
syncedTabsSelected_(selectedPage) {
On 2016/06/08 01:18:38, Dan Beam wrote:
> On 2016/06/07 04:46:04, calamity wrote:
> > On 2016/06/06 04:33:16, tsergeant wrote:
> > > From the Dept. Of Why Did This Ever Work, apparently this is valid syntax
to
> > > declare a function?
> > 
> > Oops + wat
> 
> http://es6-features.org/#MethodProperties

Aaaaahhhh, interesting.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:44: incremental: false,
On 2016/06/08 00:07:35, dpapad wrote:
> Shouldn't the QueryState typedef also be upadated? 

Done.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:69: ready: function() {
On 2016/06/08 00:07:35, dpapad wrote:
> Nit:
> /** @override */

Done.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:210: getSelectedPage:
function(selectedPage, range) {
On 2016/06/08 00:07:35, dpapad wrote:
> @param, @return missing

Done.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:217: syncedTabsSelected_:
function(selectedPage) {
On 2016/06/08 00:07:35, dpapad wrote:
> Same here.

Done.

https://codereview.chromium.org/2032313002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:227: * @return {boolean}
On 2016/06/08 00:07:36, dpapad wrote:
> Nit: Move the comment at lines 222-223 here, as follows.
> 
> @return {boolean} Whether a spinner should be shown
>     (implies that the backend is querying a new search term).

Done.

[tsergeant, 2016-06-08 02:54:03]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2016-06-08 02:54:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org, dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2032313002/#ps120001
(title: "Closure comments")

[commit-bot: I haz the power, 2016-06-08 02:54:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2032313002/120001

[commit-bot: I haz the power, 2016-06-08 04:08:45]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-06-08 04:10:34]

Description was changed from

==========
MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

Adds a processing spinner which is visible when the search bar is open,
and hooks it into MD History so that it is shown when loading data from
a search. It is not shown when incrementally loading new data in the
infinite list.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD WebUI: Add loading spinner to cr-toolbar, hook into MD History toolbar

Adds a processing spinner which is visible when the search bar is open,
and hooks it into MD History so that it is shown when loading data from
a search. It is not shown when incrementally loading new data in the
infinite list.

BUG=610609
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/99778ce52abf223bd279c42669a9d9049f0f9fdf
Cr-Commit-Position: refs/heads/master@{#398476}
==========

[commit-bot: I haz the power, 2016-06-08 04:10:36]

Patchset 3 (id:??) landed as
https://crrev.com/99778ce52abf223bd279c42669a9d9049f0f9fdf
Cr-Commit-Position: refs/heads/master@{#398476}