|
|
Created:
5 years, 11 months ago by lushnikov Modified:
5 years, 11 months ago CC:
aandrey+blink_chromium.org, apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, devtools-reviews_chromium.org, eustas+blink_chromium.org, loislo+blink_chromium.org, lushnikov+blink_chromium.org, malch+blink_chromium.org, paulirish+reviews_chromium.org, pfeldman+blink_chromium.org, sergeyv+blink_chromium.org, vsevik+blink_chromium.org, yurys+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionDevTools: Implement "close tabs to the right" for Sources panel tabs
The patch implements "close tabs to the right" functionality for the
sources panel tabs.
BUG=424298
R=vsevik
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188628
Patch Set 1 #
Total comments: 8
Patch Set 2 : address comments #
Total comments: 2
Patch Set 3 : address nit #Messages
Total messages: 18 (7 generated)
PTAL
alph@chromium.org changed reviewers: + alph@chromium.org
https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... File Source/devtools/front_end/ui/TabbedPane.js (right): https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:320: tabsToTheRight: function(id) _tabsToTheRight https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:322: var tabs = this._tabs.slice(); why do you need slice? https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:332: var result = []; nit: return tabs.slice(index + 1).map(function(tab) { return tab.id; }); https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:1107: contextMenu.appendItem(WebInspector.UIString.capitalize("Close ^tabs ^to ^the ^right"), closeToTheRight.bind(this)); Prepositions and articles should not be capitalized.
https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... File Source/devtools/front_end/ui/TabbedPane.js (right): https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:320: tabsToTheRight: function(id) On 2015/01/15 17:35:17, alph wrote: > _tabsToTheRight Done. https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:322: var tabs = this._tabs.slice(); On 2015/01/15 17:35:17, alph wrote: > why do you need slice? It came due to copy-pasting of other methods; removed (here and there) https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:332: var result = []; On 2015/01/15 17:35:17, alph wrote: > nit: > return tabs.slice(index + 1).map(function(tab) { return tab.id; }); Done. https://codereview.chromium.org/849073006/diff/1/Source/devtools/front_end/ui... Source/devtools/front_end/ui/TabbedPane.js:1107: contextMenu.appendItem(WebInspector.UIString.capitalize("Close ^tabs ^to ^the ^right"), closeToTheRight.bind(this)); On 2015/01/15 17:35:17, alph wrote: > Prepositions and articles should not be capitalized. Done.
lgtm https://codereview.chromium.org/849073006/diff/20001/Source/devtools/front_en... File Source/devtools/front_end/ui/TabbedPane.js (right): https://codereview.chromium.org/849073006/diff/20001/Source/devtools/front_en... Source/devtools/front_end/ui/TabbedPane.js:295: for (var i = 0; i < this._tabs.length; ++i) nit: return this._tabs.map(function(tab) { return tab.id; });
https://codereview.chromium.org/849073006/diff/20001/Source/devtools/front_en... File Source/devtools/front_end/ui/TabbedPane.js (right): https://codereview.chromium.org/849073006/diff/20001/Source/devtools/front_en... Source/devtools/front_end/ui/TabbedPane.js:295: for (var i = 0; i < this._tabs.length; ++i) On 2015/01/19 07:23:38, alph wrote: > nit: > return this._tabs.map(function(tab) { return tab.id; }); Done.
The CQ bit was checked by lushnikov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/849073006/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)
The CQ bit was checked by lushnikov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/849073006/40001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to commit the patch. svn: E155010: Commit failed (details follow): svn: E155010: '/b/infra_internal/commit_queue/workdir/blink/LayoutTests/virtual/threaded/fast' is scheduled for addition, but is missing
The CQ bit was unchecked by commit-bot@chromium.org
The CQ bit was checked by lushnikov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/849073006/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://src.chromium.org/viewvc/blink?view=rev&revision=188628 |