Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(25)

Issue 1358893003: Ignore mouseup shortly after showing cr.ui.Menu (Closed)

Created:
3 years, 11 months ago by Dan Beam
Modified:
3 years, 11 months ago
Reviewers:
mustaq
CC:
chromium-reviews, oshima+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Ignore mouseup shortly after showing cr.ui.Menu This works simlarly to views::MenuController[1] by ignoring mouseups: - less than 200ms after showing some UI - with minimal mouse movement (4px length difference) R=mustaq@chromium.org BUG=531939 [1] https://goo.gl/iZht5k Committed: https://crrev.com/70aa692d68ee86d365928edd160c3575fda2b453 Cr-Commit-Position: refs/heads/master@{#350323}

Patch Set 1 : . #

Total comments: 4

Patch Set 2 : nit #

Patch Set 3 : fix context menus #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -9 lines) Patch
M chrome/test/data/webui/menu_test.html View 1 2 2 chunks +47 lines, -2 lines 0 comments Download
M ui/webui/resources/js/cr/ui/context_menu_handler.js View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M ui/webui/resources/js/cr/ui/menu.js View 1 2 3 chunks +43 lines, -0 lines 0 comments Download
M ui/webui/resources/js/cr/ui/menu_button.js View 5 chunks +7 lines, -5 lines 0 comments Download

Messages

Total messages: 23 (13 generated)
Dan Beam
3 years, 11 months ago (2015-09-22 03:43:55 UTC) #4
mustaq
Thanks Dan, this is clearly a better solution than mine. LGTM with nits. https://codereview.chromium.org/1358893003/diff/60001/ui/webui/resources/js/cr/ui/menu.js File ...
3 years, 11 months ago (2015-09-22 14:20:37 UTC) #5
Dan Beam
https://codereview.chromium.org/1358893003/diff/60001/ui/webui/resources/js/cr/ui/menu.js File ui/webui/resources/js/cr/ui/menu.js (right): https://codereview.chromium.org/1358893003/diff/60001/ui/webui/resources/js/cr/ui/menu.js#newcode118 ui/webui/resources/js/cr/ui/menu.js:118: * @param {Event} e On 2015/09/22 14:20:36, mustaq wrote: ...
3 years, 11 months ago (2015-09-22 15:51:31 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1358893003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1358893003/100001
3 years, 11 months ago (2015-09-22 15:55:07 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/106199)
3 years, 11 months ago (2015-09-22 17:02:03 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1358893003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1358893003/100001
3 years, 11 months ago (2015-09-22 17:05:38 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/106231)
3 years, 11 months ago (2015-09-22 18:22:10 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1358893003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1358893003/160001
3 years, 11 months ago (2015-09-23 05:49:54 UTC) #21
commit-bot: I haz the power
Committed patchset #3 (id:160001)
3 years, 11 months ago (2015-09-23 07:04:55 UTC) #22
commit-bot: I haz the power
3 years, 11 months ago (2015-09-23 07:05:31 UTC) #23
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/70aa692d68ee86d365928edd160c3575fda2b453
Cr-Commit-Position: refs/heads/master@{#350323}

Powered by Google App Engine
This is Rietveld 408576698