Context MenuItem Touch Feedback

Context MenuItem Touch Feedback

Modify Menu.jr to also respond to touch events. Provide visual touch feedback based on mouse hover.

TEST=Manual testing of Bookmark Manager
BUG=398404
Committed: https://crrev.com/84e610205c01dfebe7aa419f160e6986687e1289
Cr-Commit-Position: refs/heads/master@{#295779}

[jonross, 2014-09-18 20:42:09]

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

[jonross, 2014-09-18 20:42:10]

Hi Rob,

Here I'm adding touch feedback to system menus. This is based on the design for
Bookmark Manager. I spoke with stevet@ and he wants us to apply it system wide
to context menus.

[flackr, 2014-09-18 21:16:33]

I think this should be done using the :active pseudoclass. Since I believe we
rely on gestures (i.e. a synthetic click) to activate menu items, they do not
necessarily have the same target as the touchstart and a tap gesture is still
allowed even if there is some finger movement (looks like 8 dips as per
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/gesture_...).

The :active state should track exactly when a release will trigger a tap,
turning into a mouse click on the menu item.

[jonross, 2014-09-19 14:50:42]

I've switched to setting the background color on :active

[flackr, 2014-09-19 14:54:36]

https://codereview.chromium.org/584553005/diff/20001/ui/webui/resources/css/m...
File ui/webui/resources/css/menu.css (right):

https://codereview.chromium.org/584553005/diff/20001/ui/webui/resources/css/m...
ui/webui/resources/css/menu.css:70: menu > :not(hr):active {
Seems if the above rule matches this will match. Do we need the above rule?

[jonross, 2014-09-19 16:59:44]

https://codereview.chromium.org/584553005/diff/20001/ui/webui/resources/css/m...
File ui/webui/resources/css/menu.css (right):

https://codereview.chromium.org/584553005/diff/20001/ui/webui/resources/css/m...
ui/webui/resources/css/menu.css:70: menu > :not(hr):active {
On 2014/09/19 14:54:35, flackr wrote:
> Seems if the above rule matches this will match. Do we need the above rule? 

Nope.

[flackr, 2014-09-19 17:07:57]

lgtm

[jonross, 2014-09-19 17:16:12]

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

[jonross, 2014-09-19 17:16:13]

dbeam@chromium.org: Please review changes in 

ui/webui/resources/css/menu.css

I am currently adding visual feedback for touch events to a series of controls
across ui/ and ash/

[Dan Beam, 2014-09-19 18:44:00]

https://codereview.chromium.org/584553005/diff/40001/ui/webui/resources/css/m...
File ui/webui/resources/css/menu.css (right):

https://codereview.chromium.org/584553005/diff/40001/ui/webui/resources/css/m...
ui/webui/resources/css/menu.css:62: menu > :not(hr)[selected] {
menu > :not(hr):-webkit-any([selected], :active) {
  background-color: rgba(0, 0, 0, .06);
}

[jonross, 2014-09-19 19:07:03]

https://codereview.chromium.org/584553005/diff/40001/ui/webui/resources/css/m...
File ui/webui/resources/css/menu.css (right):

https://codereview.chromium.org/584553005/diff/40001/ui/webui/resources/css/m...
ui/webui/resources/css/menu.css:62: menu > :not(hr)[selected] {
On 2014/09/19 18:43:59, Dan Beam wrote:
> menu > :not(hr):-webkit-any([selected], :active) {
>   background-color: rgba(0, 0, 0, .06);
> }

Done.

[Dan Beam, 2014-09-19 19:32:46]

lgtm

[jonross, 2014-09-19 19:33:46]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-09-19 19:34:33]

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

[commit-bot: I haz the power, 2014-09-19 21:18:50]

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

[commit-bot: I haz the power, 2014-09-19 21:18:51]

Failed to apply the patch.

[commit-bot: I haz the power, 2014-09-19 21:18:59]

Committed patchset #4 (id:60001) as a1f9c67bf63aa800f71d73d7162ecb1f591916a2

[commit-bot: I haz the power, 2014-09-19 21:19:32]

Patchset 4 (id:??) landed as
https://crrev.com/84e610205c01dfebe7aa419f160e6986687e1289
Cr-Commit-Position: refs/heads/master@{#295779}