Add a cr-button element, a thin wrapper around paper-button.

Add a cr-button element, a thin wrapper around paper-button.

R=dbeam@chromium.org
BUG=

Committed: https://crrev.com/3e322e059271f01f38ce7176664c1ccdab10e447
Cr-Commit-Position: refs/heads/master@{#318641}

[Jeremy Klein, 2015-02-17 23:28:08]

jlklein@chromium.org changed reviewers:
+ michaelpg@chromium.org

[michaelpg, 2015-02-19 00:20:17]

+dmazzoni for shadow DOM accessibility question: if a custom element displays
only another custom element whose role=button, what if anything should the role
of the wrapping custom element be?

https://codereview.chromium.org/939533002/diff/1/chrome/browser/resources/md_...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/939533002/diff/1/chrome/browser/resources/md_...
chrome/browser/resources/md_settings/md_settings.html:11: <style
type="text/css">
Let's add md_settings.css.

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
File ui/webui/resources/cr_elements/cr_button/cr_button.html (right):

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
ui/webui/resources/cr_elements/cr_button/cr_button.html:4: <polymer-element
name="cr-button" role="button">
since the <paper-button> already has role=button, I believe this is redundant
and may cause issues. I could be wrong, it's hard to tell empirically because
support for shadow DOM isn't great yet anyway.

[Jeremy Klein, 2015-02-20 23:20:33]

https://codereview.chromium.org/939533002/diff/1/chrome/browser/resources/md_...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/939533002/diff/1/chrome/browser/resources/md_...
chrome/browser/resources/md_settings/md_settings.html:11: <style
type="text/css">
On 2015/02/19 00:20:17, michaelpg wrote:
> Let's add md_settings.css.

Done.

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
File ui/webui/resources/cr_elements/cr_button/cr_button.html (right):

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
ui/webui/resources/cr_elements/cr_button/cr_button.html:4: <polymer-element
name="cr-button" role="button">
On 2015/02/19 00:20:17, michaelpg wrote:
> since the <paper-button> already has role=button, I believe this is redundant
> and may cause issues. I could be wrong, it's hard to tell empirically because
> support for shadow DOM isn't great yet anyway.

Yeah you might be right. I wasn't 100% sure on this. Pinging dmazzoni again on
this one.

[Jeremy Klein, 2015-02-21 00:18:12]

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
File ui/webui/resources/cr_elements/cr_button/cr_button.html (right):

https://codereview.chromium.org/939533002/diff/1/ui/webui/resources/cr_elemen...
ui/webui/resources/cr_elements/cr_button/cr_button.html:4: <polymer-element
name="cr-button" role="button">
On 2015/02/20 23:20:33, Jeremy Klein wrote:
> On 2015/02/19 00:20:17, michaelpg wrote:
> > since the <paper-button> already has role=button, I believe this is
redundant
> > and may cause issues. I could be wrong, it's hard to tell empirically
because
> > support for shadow DOM isn't great yet anyway.
> 
> Yeah you might be right. I wasn't 100% sure on this. Pinging dmazzoni again on
> this one.

Verified off-thread with Dominic that you're right here. Removed the role.

[Dan Beam, 2015-02-21 00:57:19]

dbeam@chromium.org changed reviewers:
- dbeam@chromium.org

[Dan Beam, 2015-02-21 00:57:29]

R=-dbeam

[michaelpg, 2015-02-21 01:23:05]

https://codereview.chromium.org/939533002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.css (right):

https://codereview.chromium.org/939533002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.css:6: unicode-range:
U+0460-052F, U+20B4, U+2DE0-2DFF, U+A640-A69F;
what is this exactly? why do we need the symbol for the Ukrainian hryvnia
currency?

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
File ui/webui/resources/cr_elements/cr_button/demo.html (right):

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:1: <!DOCTYPE html>
nit: lowercase for consistency

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:12: <cr-button><core-icon
icon="mail"></core-icon>&nbsp;Custom button</cr-button>
ouch, does it really require the &nbsp; encoded like that?

[Jeremy Klein, 2015-02-21 01:52:22]

https://codereview.chromium.org/939533002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.css (right):

https://codereview.chromium.org/939533002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.css:6: unicode-range:
U+0460-052F, U+20B4, U+2DE0-2DFF, U+A640-A69F;
On 2015/02/21 01:23:04, michaelpg wrote:
> what is this exactly? why do we need the symbol for the Ukrainian hryvnia
> currency?

Pulled this from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

Do you know if the unicode range matters at all?

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
File ui/webui/resources/cr_elements/cr_button/demo.html (right):

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:1: <!DOCTYPE html>
On 2015/02/21 01:23:05, michaelpg wrote:
> nit: lowercase for consistency

Done.

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:12: <cr-button><core-icon
icon="mail"></core-icon>&nbsp;Custom button</cr-button>
On 2015/02/21 01:23:04, michaelpg wrote:
> ouch, does it really require the &nbsp; encoded like that?

Well it isn't the only way to get the spacing (I could add a margin-right to the
icon), but a normal space will be stripped and I felt like this was ok for a
demo. If you'd rather I do the margin, that's fine with me too.

[James Hawkins, 2015-02-23 20:08:50]

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

[James Hawkins, 2015-02-23 20:08:51]

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
File ui/webui/resources/cr_elements/cr_button/demo.html (right):

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:1: <!DOCTYPE html>
On 2015/02/21 01:52:22, Jeremy Klein wrote:
> On 2015/02/21 01:23:05, michaelpg wrote:
> > nit: lowercase for consistency
> 
> Done.

For future reference, the Google HTML style guide requires all lowercase (for
HTML element names, attributes, attribute values (unless text/CDATA), CSS
selectors, properties, and property values (except strings)).

[michaelpg, 2015-02-25 00:35:00]

LGTM. We'll need to figure out precisely what determines the Unicode range for
the font in md-settings.css, but that can come later.

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
File ui/webui/resources/cr_elements/cr_button/demo.html (right):

https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
ui/webui/resources/cr_elements/cr_button/demo.html:12: <cr-button><core-icon
icon="mail"></core-icon>&nbsp;Custom button</cr-button>
On 2015/02/21 01:52:22, Jeremy Klein wrote:
> On 2015/02/21 01:23:04, michaelpg wrote:
> > ouch, does it really require the &nbsp; encoded like that?
> 
> Well it isn't the only way to get the spacing (I could add a margin-right to
the
> icon), but a normal space will be stripped and I felt like this was ok for a
> demo. If you'd rather I do the margin, that's fine with me too.

i see. maybe we should consider defining the margin in cr_button.css? but we
should wait and see how we use it in real life.

[Jeremy Klein, 2015-02-25 20:02:41]

On 2015/02/25 00:35:00, michaelpg wrote:
> LGTM. We'll need to figure out precisely what determines the Unicode range for
> the font in md-settings.css, but that can come later.
> 
>
https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
> File ui/webui/resources/cr_elements/cr_button/demo.html (right):
> 
>
https://codereview.chromium.org/939533002/diff/60001/ui/webui/resources/cr_el...
> ui/webui/resources/cr_elements/cr_button/demo.html:12: <cr-button><core-icon
> icon="mail"></core-icon>&nbsp;Custom button</cr-button>
> On 2015/02/21 01:52:22, Jeremy Klein wrote:
> > On 2015/02/21 01:23:04, michaelpg wrote:
> > > ouch, does it really require the &nbsp; encoded like that?
> > 
> > Well it isn't the only way to get the spacing (I could add a margin-right to
> the
> > icon), but a normal space will be stripped and I felt like this was ok for a
> > demo. If you'd rather I do the margin, that's fine with me too.
> 
> i see. maybe we should consider defining the margin in cr_button.css? but we
> should wait and see how we use it in real life.

Sounds good. If the icons are a really common use case, I agree we should add in
the margin in the css later.

[Jeremy Klein, 2015-02-28 01:36:44]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-02-28 01:36:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps160001
(title: "Fix a couple conflicts")

[commit-bot: I haz the power, 2015-02-28 01:37:16]

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

[commit-bot: I haz the power, 2015-02-28 02:18:01]

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

[commit-bot: I haz the power, 2015-02-28 02:18:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Jeremy Klein, 2015-02-28 02:21:29]

On 2015/02/28 02:18:02, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Gah, still need OWNER approval for:

chrome/browser/browser_resources.grd
chrome/browser/ui/webui/md_settings_ui.cc

James, can you help me out?

[James Hawkins, 2015-02-28 22:21:59]

LGTM with nit.

https://codereview.chromium.org/939533002/diff/160001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_button/cr_button.js (right):

https://codereview.chromium.org/939533002/diff/160001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_button/cr_button.js:14: <core-icon
icon="favorite"></core-icon> Custom button
nit: Missing comment asterisks on the left.

[Jeremy Klein, 2015-02-28 23:57:49]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-02-28 23:57:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps180001
(title: "Fix a comment style issue.")

[Jeremy Klein, 2015-02-28 23:58:12]

https://codereview.chromium.org/939533002/diff/160001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_button/cr_button.js (right):

https://codereview.chromium.org/939533002/diff/160001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_button/cr_button.js:14: <core-icon
icon="favorite"></core-icon> Custom button
On 2015/02/28 22:21:58, James Hawkins wrote:
> nit: Missing comment asterisks on the left.

Nice catch. Done.

[commit-bot: I haz the power, 2015-02-28 23:58:26]

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

[commit-bot: I haz the power, 2015-03-01 00:03:15]

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

[commit-bot: I haz the power, 2015-03-01 00:03:17]

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

[Jeremy Klein, 2015-03-01 00:29:56]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-03-01 00:29:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps200001
(title: "Rebase")

[commit-bot: I haz the power, 2015-03-01 00:31:11]

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

[Jeremy Klein, 2015-03-01 00:33:16]

The CQ bit was unchecked by jlklein@chromium.org

[Jeremy Klein, 2015-03-01 00:33:36]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-03-01 00:33:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps220001
(title: "Add licenses to css files.")

[commit-bot: I haz the power, 2015-03-01 00:34:02]

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

[commit-bot: I haz the power, 2015-03-01 00:39:20]

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

[commit-bot: I haz the power, 2015-03-01 00:39:21]

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

[Jeremy Klein, 2015-03-01 22:46:35]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-03-01 22:46:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps240001
(title: "Another rebase...")

[commit-bot: I haz the power, 2015-03-01 22:47:44]

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

[commit-bot: I haz the power, 2015-03-01 22:53:11]

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

[commit-bot: I haz the power, 2015-03-01 22:53:12]

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

[Jeremy Klein, 2015-03-01 23:25:44]

On 2015/03/01 22:53:12, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

I'm having trouble figuring out why these bots keep failing. Does anyone else
see what's going on here? The logs haven't really given me anything I
understand. Building and running works totally fine locally.

[Jeremy Klein, 2015-03-02 01:48:01]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-03-02 01:48:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/939533002/#ps260001
(title: "Rebase with suspected fixes for bots")

[commit-bot: I haz the power, 2015-03-02 01:48:24]

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

[Jeremy Klein, 2015-03-02 02:28:04]

On 2015/03/02 01:48:24, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/939533002/260001

chrome-troopers were able to help out here and find the breaking CL and revert
it. This looks like it will go through now.

[commit-bot: I haz the power, 2015-03-02 03:08:46]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-03-02 03:09:23]

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