MD History: Update button style for CBD in sidenav

MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[lshang, 2016-09-21 08:00:35]

Description was changed from

==========
vulcanize


add icon to cbd

BUG=
==========

to

==========
vulcanize


add icon to cbd

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

[lshang, 2016-09-21 08:02:32]

Description was changed from

==========
vulcanize


add icon to cbd

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

to

==========
MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[lshang, 2016-09-21 08:03:04]

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

[lshang, 2016-09-21 08:03:05]

PTAL thanks!

[tsergeant, 2016-09-21 23:51:47]

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:30: #clear-browsing-data-icon
{
Nit: Remove the id from the icon and use the selector

#clear-browsing-data iron-icon

instead

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
I think the sizes in the mock were for 2x screens: It matches the preview image
more closely if you halve them to 10px 24px 10px 0;

Also, all our icons are 20px by 20px, so add:

height: 20px;
width: 20px;

here

[Dan Beam, 2016-09-22 00:14:39]

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/icons.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/icons.html:14: <g id="open-in-new"><path
d="M19 19H5V5h7V3H5c-1.11 0-2 .9-2 2v14c0 1.1.89 2 2 2h14c1.1 0 2-.9
2-2v-7h-2v7zM14 3v2h3.59l-9.83 9.83 1.41 1.41L19 6.41V10h2V3h-7z"></path></g>
settings also uses this icon:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/icons....

which means it should go in the shared file:
ui/webui/resources/cr_elements/icons.html

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
this probably wont work in TRL

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:104: <iron-icon
icon="history:open-in-new" id="clear-browsing-data-icon"></iron-icon>
80 char wrap

[lshang, 2016-09-22 04:47:12]

lshang@chromium.org changed reviewers:
+ dbeam@chromium.org

[lshang, 2016-09-22 04:47:13]

PTALA thanks!  (●'◡'●)

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/icons.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/icons.html:14: <g id="open-in-new"><path
d="M19 19H5V5h7V3H5c-1.11 0-2 .9-2 2v14c0 1.1.89 2 2 2h14c1.1 0 2-.9
2-2v-7h-2v7zM14 3v2h3.59l-9.83 9.83 1.41 1.41L19 6.41V10h2V3h-7z"></path></g>
On 2016/09/22 00:14:39, Dan Beam wrote:
> settings also uses this icon:
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/icons....
> 
> which means it should go in the shared file:
> ui/webui/resources/cr_elements/icons.html
> 

Done. Also removed it from setting's icon set.

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:30: #clear-browsing-data-icon
{
On 2016/09/21 23:51:47, tsergeant wrote:
> Nit: Remove the id from the icon and use the selector
> 
> #clear-browsing-data iron-icon
> 
> instead

Done.

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
On 2016/09/21 23:51:47, tsergeant wrote:
> I think the sizes in the mock were for 2x screens: It matches the preview
image
> more closely if you halve them to 10px 24px 10px 0;
> 
> Also, all our icons are 20px by 20px, so add:
> 
> height: 20px;
> width: 20px;
> 
> here

Done. I saw the more-vert-button in history item is 24px by 24px, chose the
wrong icon as referrence :P

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
On 2016/09/22 00:14:39, Dan Beam wrote:
> this probably wont work in TRL

I added 24px margin to both left and right, in case in right-to-left mode. Is
that OK?

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:104: <iron-icon
icon="history:open-in-new" id="clear-browsing-data-icon"></iron-icon>
On 2016/09/22 00:14:39, Dan Beam wrote:
> 80 char wrap

NaN after remove the id XD

[tsergeant, 2016-09-22 06:34:30]

lgtm with one more comment

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
On 2016/09/22 04:47:13, lshang wrote:
> On 2016/09/22 00:14:39, Dan Beam wrote:
> > this probably wont work in TRL
> 
> I added 24px margin to both left and right, in case in right-to-left mode. Is
> that OK?

I think it would be better to use -webkit-margin-end to set the margin. This
leaves more space on the left side for translated text, which can be longer than
English text.

[lshang, 2016-09-22 07:27:07]

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
On 2016/09/22 06:34:30, tsergeant wrote:
> On 2016/09/22 04:47:13, lshang wrote:
> > On 2016/09/22 00:14:39, Dan Beam wrote:
> > > this probably wont work in TRL
> > 
> > I added 24px margin to both left and right, in case in right-to-left mode.
Is
> > that OK?
> 
> I think it would be better to use -webkit-margin-end to set the margin. This
> leaves more space on the left side for translated text, which can be longer
than
> English text.

Done.

[Dan Beam, 2016-09-23 06:22:30]

lgtm

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/side_bar.html (right):

https://codereview.chromium.org/2351363003/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/side_bar.html:32: margin: 20px 48px 20px 0;
On 2016/09/22 04:47:13, lshang wrote:
> On 2016/09/22 00:14:39, Dan Beam wrote:
> > this probably wont work in TRL
> 
> I added 24px margin to both left and right, in case in right-to-left mode. Is
> that OK?

yes

[lshang, 2016-09-25 23:52:31]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-09-25 23:52:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org
Link to the patchset: https://codereview.chromium.org/2351363003/#ps40001
(title: "vulcanize")

[commit-bot: I haz the power, 2016-09-25 23:52:46]

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-09-25 23:54:59]

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

[commit-bot: I haz the power, 2016-09-25 23:55:00]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lshang, 2016-09-26 00:11:21]

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

[commit-bot: I haz the power, 2016-09-26 00:11:29]

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-09-26 01:17:20]

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

[commit-bot: I haz the power, 2016-09-26 01:17:21]

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_...)

[lshang, 2016-09-27 01:02:36]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-09-27 01:02:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2351363003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-27 01:03:41]

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-09-27 02:41:47]

Description was changed from

==========
MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-27 02:41:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-27 02:43:49]

Description was changed from

==========
MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD History: Update button style for CBD in sidenav

Remove the UPPPERCASE styling and use sentence casing: "Clear browsing Data".

Append a "open-in-new" icon to the end of the list item.

BUG=648806
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-09-27 02:43:50]

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