MD Downloads: use "big search" box like MD history

MD Downloads: use "big search" box like MD history

These make the UIs look more consistent. It also makes downloads look
way better, IMO.

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

Committed: https://crrev.com/ffabcb7320d61727b552d23999d82f8aa40ed2ad
Cr-Commit-Position: refs/heads/master@{#414022}

[Dan Beam, 2016-08-23 19:00:55]

Description was changed from

==========
MD Downloads: use "big search" box like MD history

These make the UIs look more consistent. It also makes downloads look
way better, IMO.

R=tsergeant@chromium.org
BUG=588324
==========

to

==========
MD Downloads: use "big search" box like MD history

These make the UIs look more consistent. It also makes downloads look
way better, IMO.

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

[Dan Beam, 2016-08-23 22:41:42]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 22:42:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 22:55:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 22:55:03]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Dan Beam, 2016-08-23 23:12:09]

screenshots: http://imgur.com/a/t2qRH

[Dan Beam, 2016-08-23 23:53:22]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 23:54:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 00:44:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 00:44:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tsergeant, 2016-08-24 01:09:37]

https://codereview.chromium.org/2275653002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/toolbar.html:17: show-menu="{{showMenu_}}"
on-search-changed="onSearchChanged_">
showMenu_ and showingSearch_ don't appear to be used, can they be removed/made
constant values?

(plus, I don't think showingSearch is publically exposed from cr-toolbar)

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:77: <template
is="dom-if" if="[[menuLabel]]">
We already have the button hiding from the [show-menu] attribute in CSS, but
switching to a dom-if is a good idea.

You can delete the `:host(:not([show-menu])) #menuButton` block above, use
[[showMenu]] in this dom-if, and remove the `reflectToAttribute` from showMenu
in cr_toolbar.js

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:96: <content
select=".more-actions"></content>
Cool, I was just about to implement something similar for the (i) button in
history.

[Dan Beam, 2016-08-24 02:48:04]

tests still don't pass

https://codereview.chromium.org/2275653002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/toolbar.html:17: show-menu="{{showMenu_}}"
on-search-changed="onSearchChanged_">
On 2016/08/24 01:09:37, tsergeant wrote:
> showMenu_ and showingSearch_ don't appear to be used, can they be removed/made
> constant values?

Done.

> 
> (plus, I don't think showingSearch is publically exposed from cr-toolbar)

was in a local patch, didn't end up needing it.

an aside: [I still stand by the fact that] 2 custom elements named
<cr-search-field> and <cr-toolbar-search-field> share a common behavior is
confusing

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:77: <template
is="dom-if" if="[[menuLabel]]">
On 2016/08/24 01:09:37, tsergeant wrote:
> We already have the button hiding from the [show-menu] attribute in CSS, but
> switching to a dom-if is a good idea.

Acknowledged.

> 
> You can delete the `:host(:not([show-menu])) #menuButton` block above, use
> [[showMenu]] in this dom-if, and remove the `reflectToAttribute` from showMenu
> in cr_toolbar.js

md_history/ depended on [show-menu] but it mirrored that state into [has-drawer]
(which is also reflected), so I just had to tweak the selector to get the same
result.  Done.

https://codereview.chromium.org/2275653002/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_toolbar/cr_toolbar.html:96: <content
select=".more-actions"></content>
On 2016/08/24 01:09:37, tsergeant wrote:
> Cool, I was just about to implement something similar for the (i) button in
> history.

Acknowledged.

[Dan Beam, 2016-08-24 02:49:33]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 02:49:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-08-24 03:03:24]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 03:04:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 04:12:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 04:12:16]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[tsergeant, 2016-08-24 04:36:06]

I think the tests are failing because `menuButton` is only defined after
`app.hasDrawer_ = true`.

In certain configurations, hasDrawer_ will be true because of the window size,
but in others the menu button will only be available after line 19 in the test.

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/manager.css (right):

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/manager.css:21: overflow-y: overlay
!important;
Chris and I are moving away from overflow: overlay due to weird issues with
horizontal scroll that we weren't able to work around:
https://codereview.chromium.org/2273523002/

Obviously, with the new layout in Downloads, having both elements be centered
correctly is important, so...let us know if you find a way to disable the extra
horizontal scroll?

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/toolbar.html:27: <paper-item
on-tap="onOpenDownloadsFolderTap_" on-blur="onItemBlur_">
Nit: Wrap to 80 characters

[Dan Beam, 2016-08-24 06:00:41]

On 2016/08/24 04:36:06, tsergeant wrote:
> I think the tests are failing because `menuButton` is only defined after
> `app.hasDrawer_ = true`.
> 
> In certain configurations, hasDrawer_ will be true because of the window size,
> but in others the menu button will only be available after line 19 in the
test.

makes sense, thanks for the tip.

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/manager.css (right):

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/manager.css:21: overflow-y: overlay
!important;
On 2016/08/24 04:36:06, tsergeant wrote:
> Chris and I are moving away from overflow: overlay due to weird issues with
> horizontal scroll that we weren't able to work around:
> https://codereview.chromium.org/2273523002/
> 
> Obviously, with the new layout in Downloads, having both elements be centered
> correctly is important, so...let us know if you find a way to disable the
extra
> horizontal scroll?

overflow-y: overlay is the way we found to ignore dynamically calculating the
scrollbar width

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2275653002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/toolbar.html:27: <paper-item
on-tap="onOpenDownloadsFolderTap_" on-blur="onItemBlur_">
On 2016/08/24 04:36:06, tsergeant wrote:
> Nit: Wrap to 80 characters

Done.

[Dan Beam, 2016-08-24 06:00:53]

Patchset #6 (id:100001) has been deleted

[Dan Beam, 2016-08-24 06:01:09]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2016-08-24 06:27:55]

lgtm

[Dan Beam, 2016-08-24 06:31:57]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-08-24 06:33:31]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-08-24 06:34:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 08:36:15]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-08-24 08:37:55]

Description was changed from

==========
MD Downloads: use "big search" box like MD history

These make the UIs look more consistent. It also makes downloads look
way better, IMO.

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

to

==========
MD Downloads: use "big search" box like MD history

These make the UIs look more consistent. It also makes downloads look
way better, IMO.

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

Committed: https://crrev.com/ffabcb7320d61727b552d23999d82f8aa40ed2ad
Cr-Commit-Position: refs/heads/master@{#414022}
==========

[commit-bot: I haz the power, 2016-08-24 08:37:56]

Patchset 6 (id:??) landed as
https://crrev.com/ffabcb7320d61727b552d23999d82f8aa40ed2ad
Cr-Commit-Position: refs/heads/master@{#414022}