Show/hide ink drop highlight when button enable state changes.

Show/hide ink drop highlight when button enable state changes.

BUG=677818
Committed: https://crrev.com/558f3caea4afb4a714db8f46994e60b3742e8dab
Cr-Commit-Position: refs/heads/master@{#441210}

[Evan Stade, 2017-01-03 16:35:31]

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

[commit-bot: I haz the power, 2017-01-03 16:35:45]

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

[Evan Stade, 2017-01-03 16:36:34]

estade@chromium.org changed reviewers:
+ bruthig@chromium.org, sadrul@chromium.org

[sadrul, 2017-01-03 17:13:25]

lgtm

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
File ui/views/controls/button/custom_button.cc (right):

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
ui/views/controls/button/custom_button.cc:142: GetInkDrop()->SetHovered(false);
Can some of these move into CustomButton::StateChanged()?

[commit-bot: I haz the power, 2017-01-03 17:22:41]

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

[commit-bot: I haz the power, 2017-01-03 17:22:42]

Dry run: This issue passed the CQ dry run.

[bruthig, 2017-01-03 20:05:21]

lgtm with one consideration.

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
File ui/views/controls/button/custom_button.cc (right):

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
ui/views/controls/button/custom_button.cc:142: GetInkDrop()->SetHovered(false);
On 2017/01/03 17:13:25, sadrul wrote:
> Can some of these move into CustomButton::StateChanged()?

Would it make more sense to move this to InkDropHostView::OnEnabledChanged() ?

[Evan Stade, 2017-01-03 21:19:38]

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
File ui/views/controls/button/custom_button.cc (right):

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
ui/views/controls/button/custom_button.cc:142: GetInkDrop()->SetHovered(false);
On 2017/01/03 17:13:25, sadrul wrote:
> Can some of these move into CustomButton::StateChanged()?

Not easily. For one thing, all the 19 overrides would suddenly have to call
through to the parent OnStateChanged. I think it's unfortunate/undesirable to
have these two separate, parallel versions of button state, and it's on Ben's
radar but non trivial to fix.

https://codereview.chromium.org/2603373002/diff/1/ui/views/controls/button/cu...
ui/views/controls/button/custom_button.cc:142: GetInkDrop()->SetHovered(false);
On 2017/01/03 20:05:21, bruthig wrote:
> On 2017/01/03 17:13:25, sadrul wrote:
> > Can some of these move into CustomButton::StateChanged()?
> 
> Would it make more sense to move this to InkDropHostView::OnEnabledChanged() ?

I don't know, maybe, but I'd rather keep it near the parallel branch (i.e. the
enabled case). I guess it might be nice if both of them moved to InkDropHostView
but that would require a virtual ShouldEnterHoveredState or something.

[Evan Stade, 2017-01-03 21:19:42]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-01-03 21:20:13]

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, 2017-01-03 21:24:00]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1483478382307770, "parent_rev":
"fbe56769f8b05a7772990a3f8dab28d35242a93e", "commit_rev":
"df2ab937041ed6320c0e17fba1eae715f4857837"}

[commit-bot: I haz the power, 2017-01-03 21:24:26]

Description was changed from

==========
Show/hide ink drop highlight when button enable state changes.

BUG=677818
==========

to

==========
Show/hide ink drop highlight when button enable state changes.

BUG=677818

Review-Url: https://codereview.chromium.org/2603373002
==========

[commit-bot: I haz the power, 2017-01-03 21:24:27]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2017-01-03 21:27:25]

Description was changed from

==========
Show/hide ink drop highlight when button enable state changes.

BUG=677818

Review-Url: https://codereview.chromium.org/2603373002
==========

to

==========
Show/hide ink drop highlight when button enable state changes.

BUG=677818

Committed: https://crrev.com/558f3caea4afb4a714db8f46994e60b3742e8dab
Cr-Commit-Position: refs/heads/master@{#441210}
==========

[commit-bot: I haz the power, 2017-01-03 21:27:26]

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