Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(23)

Issue 515853002: "ctrl + double click" acts on the highlighted/selected bookmark list items irrespective of on which… (Closed)

Created:
6 years, 3 months ago by b.rout
Modified:
6 years, 3 months ago
CC:
arv+watch_chromium.org, chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Handles selection on dblclick on any bookmark list item "ctrl + double click" acts on the highlighted/selected bookmark list items irrespective of on which list item the action is performed. To extend this to clicked, unhighlighted list item, invoking mouseup event handling functionality under doubleclick event handling before command execution. mouseup event takes care of highlighting and unhighlighting list items. BUG=243221 Committed: https://crrev.com/1d406f488b1f66f6779816459254a007bb652382 Cr-Commit-Position: refs/heads/master@{#295021}

Patch Set 1 #

Total comments: 1

Patch Set 2 : implemented review comment #

Total comments: 7

Patch Set 3 : put the implementation in dblclick event handler in list.js #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -2 lines) Patch
M AUTHORS View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M ui/webui/resources/js/cr/ui/list.js View 1 2 1 chunk +10 lines, -2 lines 0 comments Download

Messages

Total messages: 18 (4 generated)
b.rout
b.rout@samsung.com changed reviewers: + pam@chromium.org, yosin@chromium.org
6 years, 3 months ago (2014-08-28 08:50:36 UTC) #1
yosin_UTC9
LGTM +arv@ for OWNERS review https://codereview.chromium.org/515853002/diff/1/chrome/browser/resources/bookmark_manager/js/main.js File chrome/browser/resources/bookmark_manager/js/main.js (right): https://codereview.chromium.org/515853002/diff/1/chrome/browser/resources/bookmark_manager/js/main.js#newcode244 chrome/browser/resources/bookmark_manager/js/main.js:244: var index = -1; ...
6 years, 3 months ago (2014-09-01 01:04:55 UTC) #3
b.rout
On 2014/09/01 01:04:55, Yosi_UTC9 wrote: > LGTM > > +arv@ for OWNERS review > > ...
6 years, 3 months ago (2014-09-01 06:40:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/b.rout@samsung.com/515853002/40001
6 years, 3 months ago (2014-09-02 05:51:48 UTC) #6
b.rout
My bad. I checked the commit bit without OWNERS review. @pam and @arv, Could you ...
6 years, 3 months ago (2014-09-02 06:31:23 UTC) #8
arv (Not doing code reviews)
Thanks for looking into this. Can you clean up the CL description a bit. The ...
6 years, 3 months ago (2014-09-02 17:17:33 UTC) #9
b.rout
https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js File chrome/browser/resources/bookmark_manager/js/main.js (right): https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js#newcode248 chrome/browser/resources/bookmark_manager/js/main.js:248: if (!indexSelected) On 2014/09/02 17:17:32, arv wrote: > index ...
6 years, 3 months ago (2014-09-03 09:01:13 UTC) #10
arv (Not doing code reviews)
https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js File chrome/browser/resources/bookmark_manager/js/main.js (right): https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js#newcode248 chrome/browser/resources/bookmark_manager/js/main.js:248: if (!indexSelected) On 2014/09/03 09:01:13, b.rout wrote: > On ...
6 years, 3 months ago (2014-09-04 15:19:21 UTC) #11
b.rout
https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js File chrome/browser/resources/bookmark_manager/js/main.js (right): https://codereview.chromium.org/515853002/diff/40001/chrome/browser/resources/bookmark_manager/js/main.js#newcode249 chrome/browser/resources/bookmark_manager/js/main.js:249: list.handlePointerDownUp_(e); On 2014/09/04 15:19:20, arv wrote: > On 2014/09/03 ...
6 years, 3 months ago (2014-09-05 09:16:54 UTC) #12
b.rout
@arv, I have incorporated the review comment and uploaded the 3rd patch. Please have a ...
6 years, 3 months ago (2014-09-05 09:19:04 UTC) #13
arv (Not doing code reviews)
LGTM
6 years, 3 months ago (2014-09-15 21:35:37 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/515853002/60001
6 years, 3 months ago (2014-09-16 05:30:00 UTC) #16
commit-bot: I haz the power
Committed patchset #3 (id:60001) as 338adc69ba365a1806eece935277d7fec483f5fc
6 years, 3 months ago (2014-09-16 06:34:08 UTC) #17
commit-bot: I haz the power
6 years, 3 months ago (2014-09-16 06:35:29 UTC) #18
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/1d406f488b1f66f6779816459254a007bb652382
Cr-Commit-Position: refs/heads/master@{#295021}

Powered by Google App Engine
This is Rietveld 408576698