Devtools UX: Add elements toolbar with breadcrumbs (issue 1158133004 by samli@chromium.org)

[sa...@chromium.org]

Reviewers: pfeldman,

Message:
PTAL. Screenshot: http://imgur.com/SNlm5KP

Description:
Devtools UX: Add elements toolbar with breadcrumbs

This change adds an elements toolbar to the material design UX in Devtools.
The bar contains the breadcrumbs and a global settings button.

BUG=494836

Please review this at https://codereview.chromium.org/1158133004/

Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Affected files (+78, -5 lines):
M Source/devtools/front_end/elements/ElementsBreadcrumbs.js
M Source/devtools/front_end/elements/ElementsPanel.js
M Source/devtools/front_end/elements/breadcrumbs.css
M Source/devtools/front_end/elements/elementsPanel.css
M Source/devtools/front_end/settings/module.json
M Source/devtools/front_end/ui/toolbar.css


Index: Source/devtools/front_end/elements/ElementsBreadcrumbs.js
diff --git a/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
b/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
index
0b1c4e4d737abc1beec7efaaf86a0025af4d49b3..28a95eb402f055b77d74009ba36d6d03bd272263
100644
--- a/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
+++ b/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
@@ -223,9 +223,10 @@ WebInspector.ElementsBreadcrumbs.prototype = {
// Layout 1: Measure total and normal crumb sizes
var contentElementWidth = this.contentElement.offsetWidth;
var normalSizes = [];
+ const leftMargin =
Runtime.experiments.isEnabled("materialDesign") ? 5 : 0;
for (var i = 0; i < crumbs.childNodes.length; ++i) {
var crumb = crumbs.childNodes[i];
- normalSizes[i] = crumb.offsetWidth;
+ normalSizes[i] = crumb.offsetWidth + leftMargin;
}

// Layout 2: Measure collapsed crumb sizes
@@ -236,7 +237,7 @@ WebInspector.ElementsBreadcrumbs.prototype = {
}
for (var i = 0; i < crumbs.childNodes.length; ++i) {
var crumb = crumbs.childNodes[i];
- compactSizes[i] = crumb.offsetWidth;
+ compactSizes[i] = crumb.offsetWidth + leftMargin;
}

// Layout 3: Measure collapsed crumb size
Index: Source/devtools/front_end/elements/ElementsPanel.js
diff --git a/Source/devtools/front_end/elements/ElementsPanel.js
b/Source/devtools/front_end/elements/ElementsPanel.js
index
cb51eaa752a8ba0e8b30a9cadc2079d7d3dce0a8..66fb43b15d50e88b7088aeec376920e264d64e4e
100644
--- a/Source/devtools/front_end/elements/ElementsPanel.js
+++ b/Source/devtools/front_end/elements/ElementsPanel.js
@@ -48,19 +48,30 @@ WebInspector.ElementsPanel = function()
this._searchableView.setPlaceholder(WebInspector.UIString("Find by
string, selector, or XPath"));
var stackElement = this._searchableView.element;

+ this._contentElement = createElement("div");
+ var crumbsContainer = createElement("div");
+ if (Runtime.experiments.isEnabled("materialDesign")) {
+ this._toolbar = this._createElementsToolbar();
+ var toolbar = stackElement.createChild("div", "elements-topbar
hbox");
+ toolbar.appendChild(crumbsContainer);
+ toolbar.appendChild(this._toolbar.element);
+ stackElement.appendChild(this._contentElement);
+ } else {
+ stackElement.appendChild(this._contentElement);
+ stackElement.appendChild(crumbsContainer);
+ }
+
this._elementsPanelTreeOutilneSplit = new
WebInspector.SplitWidget(false, true, "treeOutlineAnimationTimelineWidget",
300, 300);
this._elementsPanelTreeOutilneSplit.hideSidebar();

this._elementsPanelTreeOutilneSplit.setMainWidget(this._searchableView);
this._splitWidget.setMainWidget(this._elementsPanelTreeOutilneSplit);

- this._contentElement = stackElement.createChild("div");
this._contentElement.id = "elements-content";
// FIXME: crbug.com/425984
if (WebInspector.moduleSetting("domWordWrap").get())
this._contentElement.classList.add("elements-wrap");

WebInspector.moduleSetting("domWordWrap").addChangeListener(this._domWordWrapSettingChanged.bind(this));

- var crumbsContainer = stackElement.createChild("div");
crumbsContainer.id = "elements-crumbs";
this._breadcrumbs = new WebInspector.ElementsBreadcrumbs();
this._breadcrumbs.show(crumbsContainer);
@@ -109,6 +120,17 @@ WebInspector.ElementsPanel = function()
WebInspector.ElementsPanel._elementsSidebarViewTitleSymbol =
Symbol("title");

WebInspector.ElementsPanel.prototype = {
+ /**
+ * @return {!WebInspector.Toolbar}
+ */
+ _createElementsToolbar: function()
+ {
+ var toolbar = new
WebInspector.ExtensibleToolbar("elements-toolbar");
+ toolbar.element.classList.add("elements-toolbar");
+ toolbar.appendToolbarItem(new WebInspector.ToolbarSeparator());
+ return toolbar;
+ },
+
_loadSidebarViews: function()
{
var extensions = self.runtime.extensions("@WebInspector.Widget");
Index: Source/devtools/front_end/elements/breadcrumbs.css
diff --git a/Source/devtools/front_end/elements/breadcrumbs.css
b/Source/devtools/front_end/elements/breadcrumbs.css
index
ad0f668fb6e31da956f8b70bd09ee827fd28cfa4..80376d6d7ead108a17c37dcf2f6e4af4214311c1
100644
--- a/Source/devtools/front_end/elements/breadcrumbs.css
+++ b/Source/devtools/front_end/elements/breadcrumbs.css
@@ -19,6 +19,25 @@
white-space: nowrap;
}

+:host-context(.material) .crumbs .crumb {
+ padding: 0 5px;
+ cursor: pointer;
+ border-radius: 2px;
+ line-height: 18px;
+ margin: 5px 0px 5px 5px;
+ position: relative;
+ color: hsl(0, 0%, 39%);
+}
+
+:host-context(.material) .crumbs .crumb:not(.collapsed):not(:first-child):before
{
+ content: '⟩';
+ position: absolute;
+ left: -5px;
+ color: hsl(0, 0%, 39%);
+ width: 5px;
+ text-align: center;
+}
+
.crumbs .crumb.collapsed > * {
display: none;
}
@@ -38,6 +57,14 @@
text-shadow: rgba(255, 255, 255, 0.5) 0 0 0;
}

+:host-context(.material) .crumbs > .crumb.selected {
+ background-color: hsl(216, 68%, 53%) !important;
+}
+
.crumbs .crumb:hover {
background-color: rgb(216, 216, 216);
}
+
+:host-context(.material) .crumbs > .crumb {
+ background-color: hsla(216, 68%, 53%, 0.2);
+}
Index: Source/devtools/front_end/elements/elementsPanel.css
diff --git a/Source/devtools/front_end/elements/elementsPanel.css
b/Source/devtools/front_end/elements/elementsPanel.css
index
5de02d1166923f3de206fc95a7f40369b4714abd..0bc406b3c7a70c3a6c3c9c6b7a7d7d30ea1b3c8f
100644
--- a/Source/devtools/front_end/elements/elementsPanel.css
+++ b/Source/devtools/front_end/elements/elementsPanel.css
@@ -43,6 +43,11 @@
overflow-x: hidden;
}

+.elements-topbar {
+ border-bottom: 1px solid hsla(0, 0%, 0%, 0.1);
+ flex-shrink: 0;
+}
+
#elements-crumbs {
flex: 0 0 19px;
background-color: white;
@@ -52,6 +57,12 @@
width: 100%;
}

+.material #elements-crumbs {
+ flex: 1 0 28px;
+ border: none;
+ height: 100%;
+}
+
.metrics {
padding: 8px;
font-size: 10px;
Index: Source/devtools/front_end/settings/module.json
diff --git a/Source/devtools/front_end/settings/module.json
b/Source/devtools/front_end/settings/module.json
index
6632676bc9b10022f6dd0ba49653c99ef8d41526..f31047dc97268da021e23a02351c2447f56d3506
100644
--- a/Source/devtools/front_end/settings/module.json
+++ b/Source/devtools/front_end/settings/module.json
@@ -19,6 +19,14 @@
"elementClass": "settings-toolbar-item"
},
{
+ "type": "@WebInspector.ToolbarItem.Provider",
+ "actionId": "settings.show",
+ "location": "elements-toolbar",
+ "title": "Settings",
+ "order": 1,
+ "elementClass": "settings-toolbar-item"
+ },
+ {
"type": "@WebInspector.SettingUI",
"category": "Sources",
"className": "WebInspector.SettingsScreen.SkipStackFramePatternSettingUI"
Index: Source/devtools/front_end/ui/toolbar.css
diff --git a/Source/devtools/front_end/ui/toolbar.css
b/Source/devtools/front_end/ui/toolbar.css
index
cf1e1d897614ba5e20a42560feb9b905fde4bbee..3b15f039447ed202db6ac143a21f5c87c37d3193
100644
--- a/Source/devtools/front_end/ui/toolbar.css
+++ b/Source/devtools/front_end/ui/toolbar.css
@@ -257,7 +257,11 @@ select.toolbar-item {

.toolbar-divider {
margin-left: 7px;
- border-right: 1px solid #CCC;
+ border-right: 1px solid hsla(0, 0%, 0%, 0.1);
+}
+
+:host-context(.material) .toolbar-divider {
+ height: 18px;
}

.toolbar-group-label {

[sa...@chromium.org]

On 2015/06/01 at 09:56:40, samli wrote:
> PTAL. Screenshot: http://imgur.com/SNlm5KP

Updated screenshot, which increases the arrow margin by decreasing the text
padding so width remains unchanged: http://i.imgur.com/AIZqpLi.png

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
File Source/devtools/front_end/elements/ElementsBreadcrumbs.js (right):

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js#newcode226
Source/devtools/front_end/elements/ElementsBreadcrumbs.js:226: const
leftMargin = Runtime.experiments.isEnabled("materialDesign") ? 7 : 0;
Can we do this using css?

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

Design-wise, breadcrumbs' presence in the UI is now too strong. It is a
secondary feature that should not stand out with the huge blue selection
area.
Is there a way we could fix it? Underline selection? Subtle selection color?

https://codereview.chromium.org/1158133004/

[sa...@chromium.org]

Re: design

I'll let maxwalker@ take this one.

My 2c: The screenshots I took were of crazy large selectors without the
context
of the page. The current breadcrumbs implementation uses the same blue
selection
area and that area is exactly the same as before (in fact 1px smaller on the
sides). So the only difference is that the breadcrumbs are at the top and
there's more white-space.

Perhaps we should land this and re-discuss as polish when we graduate from
experiments.

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
File Source/devtools/front_end/elements/ElementsBreadcrumbs.js (right):

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js#newcode226
Source/devtools/front_end/elements/ElementsBreadcrumbs.js:226: const
leftMargin = Runtime.experiments.isEnabled("materialDesign") ? 7 : 0;

On 2015/06/01 16:20:33, pfeldman wrote:
> Can we do this using css?

No. I don't want to impact the actual width of the element due to
background changes on selection/hover. I can getComputedStyle and get
the marginLeft, but that seems redundant.

https://codereview.chromium.org/1158133004/

[maxw...@chromium.org]

> My 2c: The screenshots I took were of crazy large selectors without the
context
> of the page. The current breadcrumbs implementation uses the same blue
selection
> area and that area is exactly the same as before (in fact 1px smaller on
> the
> sides). So the only difference is that the breadcrumbs are at the top and
> there's more white-space.

> Perhaps we should land this and re-discuss as polish when we graduate from
> experiments.

SGTM! I have tried alternative selection styles with varying levels of
obtrusiveness (see attachment). I think the current style (blue background)
works well because of the visual connection to the DOM (same selection
style for
nodes) and because it is very clear and bold. Now living at the top, the
selected breadcrumb can also be seen as a kind of heading which would also
speak
for a bold representation. If this design turns out to be too prominent, I
could
alternatively imagine something subtle like the bottommost version. My
impression was that everything in between adds noise to the toolbar without
being very clear.

http://f.cl.ly/items/3A1K2T3O2I2B0l3Z2931/Breadcrumbs.png

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

Well, my problem with it is exactly its being clear and bold.

From the new mock I like new variations 2, 3, and 5. Number 4 would work if
the
underline was right beneath the text baseline.

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
File Source/devtools/front_end/elements/ElementsBreadcrumbs.js (right):

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js#newcode226
Source/devtools/front_end/elements/ElementsBreadcrumbs.js:226: const
leftMargin = Runtime.experiments.isEnabled("materialDesign") ? 7 : 0;

Please don't use getComputedStyle and I don't understand why you can't
achieve this via changes to CSS / DOM.

https://codereview.chromium.org/1158133004/

[paul...@google.com]

> http://f.cl.ly/items/3A1K2T3O2I2B0l3Z2931/Breadcrumbs.png


Agree that #1 is too bold and overpowers the rest of the toolbar/tabstrip
when
its infrequently important info.

I like #2, #3, and the last (subtle colored chevron).


https://codereview.chromium.org/1158133004/

[sa...@chromium.org]

Went with #3.

Screenshot (blue is selected, underline is hover):
http://i.imgur.com/iKbKIjD.png


https://codereview.chromium.org/1158133004/

[sa...@chromium.org]

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js
File Source/devtools/front_end/elements/ElementsBreadcrumbs.js (right):

https://codereview.chromium.org/1158133004/diff/40001/Source/devtools/front_end/elements/ElementsBreadcrumbs.js#newcode226
Source/devtools/front_end/elements/ElementsBreadcrumbs.js:226: const
leftMargin = Runtime.experiments.isEnabled("materialDesign") ? 7 : 0;

On 2015/06/02 at 10:54:52, pfeldman wrote:
> Please don't use getComputedStyle and I don't understand why you can't
achieve this via changes to CSS / DOM.

Done.

https://codereview.chromium.org/1158133004/

[maxw...@chromium.org]

> Number 4 would work if the underline was right beneath the text baseline.

> I like #2, #3, and the last (subtle colored chevron).

> Went with #3. Screenshot (blue is selected, underline is hover).

A link-style underline is not the right choice IMO, because our breadcrumbs
aren't links. They do not take you to another place like a link does (e.g.
in
Styles).
I also like the last version (subtle colored chevron). It's visible, but
doesn't
dominate the whole toolbar.

Additionally I just noted two things when playing around with the prototype:
1. The hover effect should be very very subtle, because it can be annoying
to
see it every time you move the cursor to the navigation bar.
2. There already is visual feedback on hover: the element is highlighted on
the
page.

So after that I would suggest to use very subtle effects for both states:
Selected: colored chevron
Hover: darken font-color ( hsl(0, 0%, 35%) -> hsl(0, 0%, 20%) )

Thanks for bringing this up, Pavel!

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

> Thanks for bringing this up, Pavel!

Thank you for following up and making sure the tool is excellent!

https://codereview.chromium.org/1158133004/

[pfel...@chromium.org]

lgtm



https://codereview.chromium.org/1158133004/

['commit-bot: I haz the power' via codereview.chromium.org]

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


https://codereview.chromium.org/1158133004/

['commit-bot: I haz the power' via codereview.chromium.org]

Try jobs failed on following builders:
mac_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/builds/46434)

https://codereview.chromium.org/1158133004/

['commit-bot: I haz the power' via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/patch-status/1158133004/100001


https://codereview.chromium.org/1158133004/

['commit-bot: I haz the power' via codereview.chromium.org]

Try jobs failed on following builders:

linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/64907)

https://codereview.chromium.org/1158133004/

['commit-bot: I haz the power' via codereview.chromium.org]

['commit-bot: I haz the power' via codereview.chromium.org]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196466

https://codereview.chromium.org/1158133004/