MD text buttons - implement focus ring.

MD text buttons - implement focus ring.

BUG=571500, 593614
Committed: https://crrev.com/603bbeb5d9a92d49141dbe1c4b0f511f3e41ca46
Cr-Commit-Position: refs/heads/master@{#381377}

[Evan Stade, 2016-03-15 01:47:01]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-03-15 16:49:26]

https://codereview.chromium.org/1800973002/diff/1/ui/views/controls/button/md...
File ui/views/controls/button/md_text_button.cc (right):

https://codereview.chromium.org/1800973002/diff/1/ui/views/controls/button/md...
ui/views/controls/button/md_text_button.cc:33: class MdFocusRing : public
views::View {
Seems rather heavy to create a view for the focus indicator that has a layer.
How could you need a layer?

[Evan Stade, 2016-03-15 19:09:43]

https://codereview.chromium.org/1800973002/diff/1/ui/views/controls/button/md...
File ui/views/controls/button/md_text_button.cc (right):

https://codereview.chromium.org/1800973002/diff/1/ui/views/controls/button/md...
ui/views/controls/button/md_text_button.cc:33: class MdFocusRing : public
views::View {
On 2016/03/15 16:49:26, sky wrote:
> Seems rather heavy to create a view for the focus indicator that has a layer.
> How could you need a layer?

The focus ring sits outside the button. We can't achieve that without layers or
making the button bigger. This seemed a lot simpler than the latter, which would
require changing the background, the ripple, the click targeting, and padding
around various instances of this type of button (which is usually specified in
terms of the normal border/bg not the focus ring).

If you are worried about the cost of creating this, I could instantiate it
lazily (most buttons will never get tabbed to).

[sky, 2016-03-15 20:40:21]

Go with this. We can optimize if that proves necessary. LGTM

[Evan Stade, 2016-03-15 21:08:40]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-03-15 21:09:04]

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

[commit-bot: I haz the power, 2016-03-15 22:54:11]

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

[commit-bot: I haz the power, 2016-03-15 22:54:12]

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

[Evan Stade, 2016-03-16 00:44:58]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-03-16 00:45:23]

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

[commit-bot: I haz the power, 2016-03-16 01:07:28]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-16 01:09:45]

Description was changed from

==========
MD text buttons - implement focus ring.

BUG=571500,593614
==========

to

==========
MD text buttons - implement focus ring.

BUG=571500,593614

Committed: https://crrev.com/603bbeb5d9a92d49141dbe1c4b0f511f3e41ca46
Cr-Commit-Position: refs/heads/master@{#381377}
==========

[commit-bot: I haz the power, 2016-03-16 01:09:45]

Patchset 1 (id:??) landed as
https://crrev.com/603bbeb5d9a92d49141dbe1c4b0f511f3e41ca46
Cr-Commit-Position: refs/heads/master@{#381377}