[DevTools] Hide empty toolbars (issue 2648253006 by eostroukhov@chromium.org)

0 views
Skip to first unread message

eostr...@chromium.org

unread,
Jan 24, 2017, 12:21:55 PM1/24/17
to dgo...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Reviewers: dgozman
CL: https://codereview.chromium.org/2648253006/

Message:
Please take a look.

Description:
[DevTools] Hide empty toolbars

BUG=684608

Affected files (+10, -2 lines):
M third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
M third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js


Index: third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
diff --git a/third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js b/third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
index 58a3e642cdab996bb9782eb1741aa93a34e3d2c1..ccbef798afc87718b90c55e21aa1e8b21e909824 100644
--- a/third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
+++ b/third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
@@ -639,9 +639,10 @@ Resources.ResourcesPanel = class extends UI.PanelWithSidebar {
this.visibleView = view;

this._storageViewToolbar.removeToolbarItems();
- var toolbarItems = view instanceof UI.SimpleView ? view.syncToolbarItems() : null;
- for (var i = 0; toolbarItems && i < toolbarItems.length; ++i)
+ var toolbarItems = (view instanceof UI.SimpleView && view.syncToolbarItems()) || [];
+ for (var i = 0; i < toolbarItems.length; ++i)
this._storageViewToolbar.appendToolbarItem(toolbarItems[i]);
+ this._storageViewToolbar.setVisible(toolbarItems.length > 0);
}

closeVisibleView() {
Index: third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
diff --git a/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js b/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
index 8e6f552001bba2f712cf2fb3412318eddf459255..b3c75cb232d02aaab47434cdfa67fe096d721529 100644
--- a/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
+++ b/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
@@ -355,6 +355,13 @@ UI.Toolbar = class {
}
}
}
+
+ /**
+ * @param {boolean} visible
+ */
+ setVisible(visible) {
+ this.element.classList.toggle('hidden', !visible);
+ }
};

/**


dgo...@chromium.org

unread,
Jan 24, 2017, 12:32:15 PM1/24/17
to eostr...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/2648253006/diff/1/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
File third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js (right):

https://codereview.chromium.org/2648253006/diff/1/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js#newcode362
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:362:
setVisible(visible) {
Don't introduce a method for this, since toolbar doesn't do anything
special. Instead, toggle the class directly in the client.

https://codereview.chromium.org/2648253006/

eostr...@chromium.org

unread,
Jan 24, 2017, 12:40:29 PM1/24/17
to dgo...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Thanks for the review. New version uploaded, please take another look.



https://codereview.chromium.org/2648253006/diff/1/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js
File third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js (right):

https://codereview.chromium.org/2648253006/diff/1/third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js#newcode362
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:362:
setVisible(visible) {
On 2017/01/24 17:32:14, dgozman wrote:
> Don't introduce a method for this, since toolbar doesn't do anything
special.
> Instead, toggle the class directly in the client.

dgo...@chromium.org

unread,
Jan 24, 2017, 1:14:27 PM1/24/17
to eostr...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Jan 24, 2017, 1:21:33 PM1/24/17
to eostr...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Jan 24, 2017, 2:36:57 PM1/24/17
to eostr...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Try jobs failed on following builders:
win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/369894)

https://codereview.chromium.org/2648253006/

commit-bot@chromium.org via codereview.chromium.org

unread,
Jan 24, 2017, 2:42:36 PM1/24/17
to eostr...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Jan 24, 2017, 4:22:49 PM1/24/17
to eostr...@chromium.org, dgo...@chromium.org, commi...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Reply all
Reply to author
Forward
0 new messages