MD Downloads/History: unify page and toolbar background colors

MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451
Committed: https://crrev.com/f0ed64b085f382514999e686b62cb08d10968d18
Cr-Commit-Position: refs/heads/master@{#388801}

[Dan Beam, 2016-04-19 20:47:48]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2016-04-19 22:31:42]

Description was changed from

==========
MD Downloads/History: unify page and toolbar background colors

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

to

==========
MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451
==========

[Dan Beam, 2016-04-19 22:31:42]

dbeam@chromium.org changed reviewers:
+ dschuyler@chromium.org
- tsergeant@chromium.org

[Dan Beam, 2016-04-19 22:32:02]

-tsergeant (ooo), +dschuyler

[tsergeant, 2016-04-20 06:29:39]

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

[tsergeant, 2016-04-20 06:29:41]

I'm back in office as of this afternoon, so here's a quick driveby of the
history stuff. Generally seems like a good solution to me.

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history.html (right):

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:10: <style is="custom-style"
include="md-shared-styles">
This adds a brief flash of white background (about 100ms on my machine), since
the background can't be set until polymer is ready.

I'm happy to ignore this for now and work around it later. I'm planning on
vulcanizing the page, which seems improve the flash (as well as improve page
load time overall).

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:58: background: rgb(63, 85,
102);
This should be updated to var(--md-toolbar-color) too.

[dschuyler, 2016-04-20 20:44:46]

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history.html (right):

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:67: color: #b4b4b4;
Does it make sense to change this as well
in this CL?  Maybe --paper-grey-400 or 
a custom color variable.

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.html (right):

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:30: background: rgb(68,
136, 255);
Should using a var for this background color
be part of the this CL?

https://codereview.chromium.org/1899043003/diff/40001/ui/webui/resources/html...
File ui/webui/resources/html/md_shared_styles.html (right):

https://codereview.chromium.org/1899043003/diff/40001/ui/webui/resources/html...
ui/webui/resources/html/md_shared_styles.html:4: :root {
Maybe these could have a comment saying why
we are not using google or paper color constants.

[Dan Beam, 2016-04-21 01:41:12]

Patchset #3 (id:60001) has been deleted

[Dan Beam, 2016-04-21 01:42:35]

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history.html (right):

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:10: <style is="custom-style"
include="md-shared-styles">
On 2016/04/20 06:29:40, tsergeant wrote:
> This adds a brief flash of white background (about 100ms on my machine), since
> the background can't be set until polymer is ready.
> 
> I'm happy to ignore this for now and work around it later. I'm planning on
> vulcanizing the page, which seems improve the flash (as well as improve page
> load time overall).

Let's just native props and drop the custom-style for now. <3 chrome-only dev

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:58: background: rgb(63, 85,
102);
On 2016/04/20 06:29:40, tsergeant wrote:
> This should be updated to var(--md-toolbar-color) too.

Done.

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history.html:67: color: #b4b4b4;
On 2016/04/20 20:44:46, dschuyler wrote:
> Does it make sense to change this as well
> in this CL?  Maybe --paper-grey-400 or 
> a custom color variable.

IMO, no

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/history_toolbar.html (right):

https://codereview.chromium.org/1899043003/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/history_toolbar.html:30: background: rgb(68,
136, 255);
On 2016/04/20 20:44:46, dschuyler wrote:
> Should using a var for this background color
> be part of the this CL?

I don't think it's shared anywhere else yet

https://codereview.chromium.org/1899043003/diff/40001/ui/webui/resources/html...
File ui/webui/resources/html/md_shared_styles.html (right):

https://codereview.chromium.org/1899043003/diff/40001/ui/webui/resources/html...
ui/webui/resources/html/md_shared_styles.html:4: :root {
On 2016/04/20 20:44:46, dschuyler wrote:
> Maybe these could have a comment saying why
> we are not using google or paper color constants.

Done.

[tsergeant, 2016-04-21 01:56:33]

Seems like md_colors.css is missing from this patchset?

[Dan Beam, 2016-04-21 03:02:43]

On 2016/04/21 01:56:33, tsergeant wrote:
> Seems like md_colors.css is missing from this patchset?

~ just imagine ~

(Done.)

[tsergeant, 2016-04-21 06:33:35]

md_history lgtm

[Dan Beam, 2016-04-21 17:15:47]

dschuyler@: comment here and I'll make any follow-up changes necessary

[Dan Beam, 2016-04-21 17:15:58]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-04-21 17:16:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1899043003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1899043003/100001

[commit-bot: I haz the power, 2016-04-21 17:22:19]

Description was changed from

==========
MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451
==========

to

==========
MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451
==========

[commit-bot: I haz the power, 2016-04-21 17:22:20]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-04-22 19:35:42]

Description was changed from

==========
MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451
==========

to

==========
MD Downloads/History: unify page and toolbar background colors

R=dschuyler@chromium.org
BUG=598451

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

[commit-bot: I haz the power, 2016-04-22 19:35:44]

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