Add touch gestures to views menu controls.

Add touch gestures to views menu controls.

BUG=131200
TEST=Run chrome with --aura-disable-mouse-events-from-touch --enable-touch-events and navigate the wrench menu, selecting an item.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141796

[flackr, 2012-06-11 15:19:20]

PTAL, thanks!

[sadrul, 2012-06-11 16:00:36]

LGTM with nits (but wait for sky@s full review too)

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/butt...
File ui/views/controls/button/menu_button.cc (right):

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/butt...
ui/views/controls/button/menu_button.cc:241: ui::GestureStatus
MenuButton::OnGestureEvent(const GestureEvent& event) {
This should be before OnKeyPressed (order in cc file should match the other in
h)

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/menu...
File ui/views/controls/menu/menu_controller.cc (right):

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/menu...
ui/views/controls/menu/menu_controller.cc:546: } else if (event.type() ==
ui::ET_GESTURE_TAP) {
I think if something (e.g. a button) causes the menu to show up on TAP_DOWN,
then this menu may need to ignore the first TAP gesture (like it has logic for
ignoring first mouse-release). But since that is not the case currently, this
will work OK. This is just to point out that we may need to revisit this part of
the code later.

[sky, 2012-06-11 18:28:01]

Is there an expectation that menus show on a down event so that you can scroll
and release?

[flackr, 2012-06-11 18:36:03]

On 2012/06/11 18:28:01, sky wrote:
> Is there an expectation that menus show on a down event so that you can scroll
> and release?

This is something that has been suggested, but it would have to be handled
delicately because we also want to support scrolling long menus. This CL will
enable using menus with touch, I would propose deferring complex behaviour such
as that to a future CL. I'm not even sure that we will want to do this given
that most other touch device menus don't support this sort of behaviour.

[sky, 2012-06-11 19:15:56]

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/widget/widget.cc
File ui/views/widget/widget.cc (left):

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/widget/widget...
ui/views/widget/widget.cc:1114: ReleaseCapture();
This is needed for tab dragging.

[flackr, 2012-06-11 19:34:44]

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/butt...
File ui/views/controls/button/menu_button.cc (right):

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/controls/butt...
ui/views/controls/button/menu_button.cc:241: ui::GestureStatus
MenuButton::OnGestureEvent(const GestureEvent& event) {
On 2012/06/11 16:00:37, sadrul wrote:
> This should be before OnKeyPressed (order in cc file should match the other in
> h)

Done.

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/widget/widget.cc
File ui/views/widget/widget.cc (left):

https://chromiumcodereview.appspot.com/10534092/diff/1/ui/views/widget/widget...
ui/views/widget/widget.cc:1114: ReleaseCapture();
On 2012/06/11 19:15:56, sky wrote:
> This is needed for tab dragging.

Calling ReleaseCapture here causes the menu to close after any touch. Can we
move the ReleaseCapture to where we Capture for handling tab dragging?

[sky, 2012-06-11 20:14:20]

What if you only do it when ShouldReleaseCaptureOnMouseReleased() is true.

[flackr, 2012-06-11 20:31:54]

On 2012/06/11 20:14:20, sky wrote:
> What if you only do it when ShouldReleaseCaptureOnMouseReleased() is true.

This works, done. Thanks!

[flackr, 2012-06-12 20:14:09]

sky, ping

[sky, 2012-06-12 21:26:28]

LGTM

[commit-bot: I haz the power, 2012-06-12 22:24:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/10534092/2002

[commit-bot: I haz the power, 2012-06-13 00:08:08]

Change committed as 141796