Devtools UI: Update inspect element node description label UI

Devtools UI: Update inspect element node description label UI

This change updates the node description label to match updated tooltip
UI. These labels have the following characteristics:
- Default to above the anchor element
- Centered above the anchor element
- Clamped to the visible viewport (no change)
- Non monospace font to improve text readibility
- Removal of "px" in dimensions

BUG=513066
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199732

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199956

[samli, 2015-07-23 06:34:11]

samli@chromium.org changed reviewers:
+ dgozman@chromium.org, maxwalker@chromium.org, sergeyv@chromium.org

[samli, 2015-07-23 06:34:11]

PTAL. Confirmed it looks fine on Mac/Linux since this doesn't use canvas.

Screenshot: http://i.imgur.com/7YytX88.png

[pfeldman, 2015-07-23 20:49:54]

On 2015/07/23 06:34:11, samli wrote:
> PTAL. Confirmed it looks fine on Mac/Linux since this doesn't use canvas.
> 
> Screenshot: http://i.imgur.com/7YytX88.png

I find arrow helpful when the elements is off screen, it used to point to where
the actual element was.

[pbakaus, 2015-07-24 00:37:39]

On 2015/07/23 at 20:49:54, pfeldman wrote:
> On 2015/07/23 06:34:11, samli wrote:
> > PTAL. Confirmed it looks fine on Mac/Linux since this doesn't use canvas.
> > 
> > Screenshot: http://i.imgur.com/7YytX88.png
> 
> I find arrow helpful when the elements is off screen, it used to point to
where the actual element was.

+1. Would like the arrow back. Arrows are nice.

[samli, 2015-07-24 02:03:31]

On 2015/07/24 at 00:37:39, pbakaus wrote:
> On 2015/07/23 at 20:49:54, pfeldman wrote:
> > On 2015/07/23 06:34:11, samli wrote:
> > > PTAL. Confirmed it looks fine on Mac/Linux since this doesn't use canvas.
> > > 
> > > Screenshot: http://i.imgur.com/7YytX88.png
> > 
> > I find arrow helpful when the elements is off screen, it used to point to
where the actual element was.
> 
> +1. Would like the arrow back. Arrows are nice.

Added arrows back in. PTAL.

Screenshot: http://i.imgur.com/A5LP5op.png

[maxwalker, 2015-07-24 07:47:18]

Yup, arrows are helpful for the inspect-element label.
LGTM

[dgozman, 2015-07-24 13:52:50]

Played with this:
- tooltip is now centered relative to margin box; what's the reason? It
conflicts with layout editor by Sergey - we should do something;
- dark tooltip dominates the visual view, while I'd like to focus on box
highlight instead;
- looks very nice;
- hard to notice it on dark-themed site (e.g.
http://dabblet.com/gist/d8bf427c2c96b2248518) - we should add something light
there (in border?);
- let's set cursor to default for tooltip.

[dgozman, 2015-07-24 13:57:11]

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorOverlayPage.html (right):

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:419:
document.getElementById("element-tooltip-arrow").style.visibility = "hidden";
Migrate to "hidden" class?

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:431: elementTitle.style.top =
"0px";
We usually use "0" without "px".

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:448: const pageMargin = 2;
var

[samli, 2015-07-27 01:19:11]

On 2015/07/24 at 13:52:50, dgozman wrote:
> Played with this:
> - tooltip is now centered relative to margin box; what's the reason? It
conflicts with layout editor by Sergey - we should do something;
Easier to find/clearer. Not sure how it plays with layout editor.
> - dark tooltip dominates the visual view, while I'd like to focus on box
highlight instead;
> - looks very nice;
> - hard to notice it on dark-themed site (e.g.
http://dabblet.com/gist/d8bf427c2c96b2248518) - we should add something light
there (in border?);
Deferring to Max for updated spec.
> - let's set cursor to default for tooltip.
?

[samli, 2015-07-27 01:19:20]

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorOverlayPage.html (right):

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:419:
document.getElementById("element-tooltip-arrow").style.visibility = "hidden";
On 2015/07/24 at 13:57:10, dgozman wrote:
> Migrate to "hidden" class?

Done.

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:431: elementTitle.style.top =
"0px";
On 2015/07/24 at 13:57:10, dgozman wrote:
> We usually use "0" without "px".

Done.

https://codereview.chromium.org/1255573004/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:448: const pageMargin = 2;
On 2015/07/24 at 13:57:10, dgozman wrote:
> var

Done.

[maxwalker, 2015-07-27 13:10:52]

> Hard to notice it on dark-themed site - we should add something light there
(in border?
> Deferring to Max for updated spec.

Added a border and slightly changed to background color to make the label stand
more out on dark backgrounds, examples on a variety of backgrounds:
https://goo.gl/6OBasZ.
The border is 50% transparent white, so it's clearly visible on darker
background while not blurring the label outline on white/light backgrounds.
Sam: possible approach to make the border appear outside the element:
"background-clip: padding-box".
Updated spec: https://goo.gl/mksjXK.

[pfeldman, 2015-07-27 17:03:42]

Do we need to add this much code to achieve this?

[pfeldman, 2015-07-27 20:45:14]

Applied it again, posted screenshot along with the comments in the bug.

[paulirish, 2015-07-28 23:20:52]

On 2015/07/27 at 17:03:42, pfeldman wrote:
> Do we need to add this much code to achieve this?

Looks like it's actually a reduction in JS. But more CSS.

[pfeldman, 2015-07-28 23:38:07]

On 2015/07/28 23:20:52, paulirish wrote:
> On 2015/07/27 at 17:03:42, pfeldman wrote:
> > Do we need to add this much code to achieve this?
> 
> Looks like it's actually a reduction in JS. But more CSS.

My diff tells me -74 lines of code that have been working for years, +115 lines
of new code I need to review and then probably follow up with fixes to. One
should have a good reason to change any code.

[samli, 2015-07-28 23:53:46]

On 2015/07/28 at 23:38:07, pfeldman wrote:
> On 2015/07/28 23:20:52, paulirish wrote:
> > On 2015/07/27 at 17:03:42, pfeldman wrote:
> > > Do we need to add this much code to achieve this?
> > 
> > Looks like it's actually a reduction in JS. But more CSS.
> 
> My diff tells me -74 lines of code that have been working for years, +115
lines of new code I need to review and then probably follow up with fixes to.
One should have a good reason to change any code.

I think this is reason enough.
http://i.imgur.com/Mk7AZlc.png

[pbakaus, 2015-07-29 00:02:12]

On 2015/07/28 at 23:38:07, pfeldman wrote:
> On 2015/07/28 23:20:52, paulirish wrote:
> > On 2015/07/27 at 17:03:42, pfeldman wrote:
> > > Do we need to add this much code to achieve this?
> > 
> > Looks like it's actually a reduction in JS. But more CSS.
> 
> My diff tells me -74 lines of code that have been working for years, +115
lines of new code I need to review and then probably follow up with fixes to.
One should have a good reason to change any code.

Actually, I think it's super reasonable. All in favor of more CSS, and less JS.
This actually simplifies logic while fixing it.

[pfeldman, 2015-07-29 00:54:49]

> I think this is reason enough.
> http://i.imgur.com/Mk7AZlc.png

Well, that can be fixed with a single line change, right?

[pfeldman, 2015-07-29 00:57:24]

> Actually, I think it's super reasonable. All in favor of more CSS, and less
JS.
> This actually simplifies logic while fixing it.

Sam, do you think your new code is border-friendly? I.e. it would be able to
render the border as on the Max's mocks?

[samli, 2015-07-29 05:08:28]

On 2015/07/29 at 00:57:24, pfeldman wrote:
> > Actually, I think it's super reasonable. All in favor of more CSS, and less
JS.
> > This actually simplifies logic while fixing it.
> 
> Sam, do you think your new code is border-friendly? I.e. it would be able to
render the border as on the Max's mocks?

No, not without changing the way the arrow is implemented. I just started again
from scratch, attempting to modify the existing one, but canvas doesn't support
multiple box shadows as used in Max's latest light mock (https://goo.gl/AjmAjR).

Also, this change introduced more than just style changes, it also changed the
positioning so heavy modification is required anyway.

I understand the concerns, but find the CSS styling much clearer.

[samli, 2015-07-29 05:32:26]

I've modified this to match Max's spec (https://goo.gl/AjmAjR)
Screenshot: http://i.imgur.com/cd0SxBE.png

[pfeldman, 2015-07-29 21:04:33]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2015-07-29 21:04:33]

Ok, if we are dropping the border, the code lgtm.

[paulirish, 2015-07-30 01:35:35]

new shadow doesn't match the spec. i guess drop-shadow functions a bit
differently.

http://jsbin.com/duzubilife/edit?html,css,output

[samli, 2015-07-30 03:12:30]

On 2015/07/30 at 01:35:35, paulirish wrote:
> new shadow doesn't match the spec. i guess drop-shadow functions a bit
differently.
> 
> http://jsbin.com/duzubilife/edit?html,css,output

The second has box-shadow and drop-shadow, so they are not actually that
different. Looks like the spread is different though and you can't set in
webkit. Max's mock favours the bottom shadow more, which doesn't work as well
for a top arrow, so I actually prefer it like this.

[paulirish, 2015-07-30 03:31:13]

> The second has box-shadow and drop-shadow, so they are not actually that
different. Looks like the spread is different though and you can't set in
webkit. Max's mock favours the bottom shadow more, which doesn't work as well
for a top arrow, so I actually prefer it like this.

My bad. Fixed up and added up-arrow variant to compare:
http://jsbin.com/laloxovuya/1/edit?css,output

Tried to get it to match as close as I could. Feels good.
Can you update to match?

[paulirish, 2015-07-30 03:32:30]

paulirish@chromium.org changed reviewers:
+ paulirish@chromium.org

[paulirish, 2015-07-30 03:32:31]

https://codereview.chromium.org/1255573004/diff/60001/Source/core/inspector/I...
File Source/core/inspector/InspectorOverlayPage.html (right):

https://codereview.chromium.org/1255573004/diff/60001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:179: margin-left: 4px;
I don't think we should add space in between as it appears like a descendant
selector. 
nuke the margin here and below.

[samli, 2015-07-30 03:38:29]

https://codereview.chromium.org/1255573004/diff/60001/Source/core/inspector/I...
File Source/core/inspector/InspectorOverlayPage.html (right):

https://codereview.chromium.org/1255573004/diff/60001/Source/core/inspector/I...
Source/core/inspector/InspectorOverlayPage.html:179: margin-left: 4px;
On 2015/07/30 at 03:32:30, paulirish wrote:
> I don't think we should add space in between as it appears like a descendant
selector. 
> nuke the margin here and below.

Good point. Done.

[maxwalker, 2015-07-30 10:09:29]

On 2015/07/30 at 03:32:30, paulirish wrote:
> I don't think we should add space in between as it appears like a descendant
selector. 
Hadn't thought about that, good point!

LGTM

[dgozman, 2015-07-30 11:54:34]

lgtm

[samli, 2015-07-30 12:02:39]

The CQ bit was checked by samli@chromium.org

[samli, 2015-07-30 12:02:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1255573004/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-07-30 12:02:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1255573004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1255573004/100001

[commit-bot: I haz the power, 2015-07-30 13:07:54]

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

[pfeldman, 2015-07-30 21:53:39]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1259413003/ by pfeldman@chromium.org.

The reason for reverting is: Tried it, regresses my experience, will post more
into the bug..

[samli, 2015-08-04 06:37:07]

Screenshot: http://i.imgur.com/55xTlS1.png

[samli, 2015-08-04 06:37:11]

The CQ bit was checked by samli@chromium.org

[samli, 2015-08-04 06:37:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, maxwalker@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1255573004/#ps120001
(title: " ")

[commit-bot: I haz the power, 2015-08-04 06:37:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1255573004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1255573004/120001

[commit-bot: I haz the power, 2015-08-04 07:32:44]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199956

[samli, 2015-08-06 01:35:55]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1271343004/ by samli@chromium.org.

The reason for reverting is: See crbug.com/517151.