MenuButtons enter Hover state upon Tap Down

MenuButtons enter Hover state upon Tap Down

MenuButton will now respond to TAP_DOWN gestures, entering the Hover state. It will return to a normal state if the gesture is cancelled. Tap gestures that activate the menu will still cause it to enter a Pressed state.

This experiment is behind the flag switches::kEnableTouchFeedback

TEST=MenuButtonTest.TouchFeedbackDuringTap, MenuButtonTest.TouchFeedbackDuringTapCancel
BUG=398404
Committed: https://crrev.com/c22434d2969cfaad4738219e8fb137613100d3c4
Cr-Commit-Position: refs/heads/master@{#301488}

[jonross, 2014-10-24 17:03:35]

jonross@chromium.org changed reviewers:
+ flackr@chromium.org

[jonross, 2014-10-24 17:03:35]

Hey Rob,

Could you review this change to add touch feedback to MenuButton on TapDown
events?

[flackr, 2014-10-27 17:51:27]

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
File ui/views/controls/button/menu_button.cc (right):

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
ui/views/controls/button/menu_button.cc:210: if
(switches::IsTouchFeedbackEnabled()) {
Should this be done after line 220 so that the menu gets first crack at handling
the event?

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
ui/views/controls/button/menu_button.cc:212: event->SetHandled();
This doesn't prevent context menu generation, does it?

[jonross, 2014-10-27 18:51:39]

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
File ui/views/controls/button/menu_button.cc (right):

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
ui/views/controls/button/menu_button.cc:210: if
(switches::IsTouchFeedbackEnabled()) {
On 2014/10/27 17:51:27, flackr wrote:
> Should this be done after line 220 so that the menu gets first crack at
handling
> the event?

Good call, that way any classes extending get the first shot.

Done.

https://codereview.chromium.org/674163003/diff/1/ui/views/controls/button/men...
ui/views/controls/button/menu_button.cc:212: event->SetHandled();
On 2014/10/27 17:51:27, flackr wrote:
> This doesn't prevent context menu generation, does it?

No it does not, it just keeps this view as the event target for future events,
including ones that go out of bounds. Allows it to receive the cancel/end on
moves out of the view. Show Press still causes the context menu to execute.

[flackr, 2014-10-27 20:08:04]

LGTM though I'm surprised we didn't want the pressed state.

[jonross, 2014-10-27 20:24:02]

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

[jonross, 2014-10-27 20:24:03]

sky@chromium.org: Please review changes in 

ui/views/controls/button/menu_button.cc
ui/views/controls/button/menu_button_unittest.cc

This is a follow up to https://codereview.chromium.org/639893003/ where we
unitfied the MenuButton pushed state logic.

Here I add touch feedback in response to tap down events.

[sky, 2014-10-27 22:02:30]

LGTM

[jonross, 2014-10-27 22:03:54]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-10-27 22:06:32]

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

[commit-bot: I haz the power, 2014-10-27 23:52:25]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-27 23:53:10]

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