Bug 192578 – Web Inspector: Computed: make UI more usable when the panel is narrow

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 357030 [details]
[Animated GIF] Bug

Perhaps we should ellipsize CSS selectors depending on the width of the panel.

[Radar WebKit Bug Importer]

<rdar://problem/46615753>

[Nikita Vasilyev]

Created attachment 357657 [details]
Patch

[Nikita Vasilyev]

Created attachment 357658 [details]
[Image] With patch applied

[Devin Rousso]

Comment on attachment 357657 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357657&action=review

r=me, I think we might actually benefit from having a little padding/offset between the actual computed value and its override chain.  When looking at semi-complex properties (e.g. gradients) it can sometimes be confusing where one value ends and another value begins.  Adding an inset (like the default list styling) or some vertical space before each item in the override chain might help distinguish values from each other.

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:55
> +    vertical-align: top;

Is this needed?  When I tried removing it, it looked like it worked the same.

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:84
> +    vertical-align: top;

This is already set on the non-`.expanded` rule.

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:94
> +    content: "â¢";

That's unfortunate :(  You should use the escaped unicode sequence ("\2022") so it's nice and pretty inside diffs.

Also, adding this causes the text after it to become ever-so-slightly indented when compared to the actual computed value.  You might want to offset the text slightly to ensure that it aligns horizontally.

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:96
> +    width: 0.8em;

Ditto (>55).

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:97
> +    margin-left: -0.8em;

Is there a way to get the actual width of the disclosure triangle and divide it by two?  ComputedStyleSection.css has `calc(var(--disclosure-button-size) + 6px)`, so you could rework that into a variable and use `calc(var(--disclosure-area-width) / 2)` as your value instead of something hardcoded.

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:151
>  .computed-style-section .property-trace-item .property .name + span {

Maybe we should give a class to these <span>s, like "brace"?

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:152
> +    display: none;

Merge this rule with the following one so there's no repetition.

[Nikita Vasilyev]

Comment on attachment 357657 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357657&action=review

>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:55
>> +    vertical-align: top;
> 
> Is this needed?  When I tried removing it, it looked like it worked the same.

You're right. This isn't longer needed since `.computed-property-item .property` is `inline` now.

>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:97
>> +    margin-left: -0.8em;
> 
> Is there a way to get the actual width of the disclosure triangle and divide it by two?  ComputedStyleSection.css has `calc(var(--disclosure-button-size) + 6px)`, so you could rework that into a variable and use `calc(var(--disclosure-area-width) / 2)` as your value instead of something hardcoded.

I've tried:

    width: var(--disclosure-button-size);
    margin-left: calc(var(--disclosure-button-size) * -1);
    text-align: center;

Unfortunately, it was a few pixels off :(

[Devin Rousso]

Comment on attachment 357657 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357657&action=review

>>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:97
>>> +    margin-left: -0.8em;
>> 
>> Is there a way to get the actual width of the disclosure triangle and divide it by two?  ComputedStyleSection.css has `calc(var(--disclosure-button-size) + 6px)`, so you could rework that into a variable and use `calc(var(--disclosure-area-width) / 2)` as your value instead of something hardcoded.
> 
> I've tried:
> 
>     width: var(--disclosure-button-size);
>     margin-left: calc(var(--disclosure-button-size) * -1);
>     text-align: center;
> 
> Unfortunately, it was a few pixels off :(

You'd need to take into account the extra 6px added in `calc(var(--disclosure-button-size) + 6px)`, or are you saying it was still off even with that?

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #6)
> Comment on attachment 357657 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=357657&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.css:97
> >>> +    margin-left: -0.8em;
> >> 
> >> Is there a way to get the actual width of the disclosure triangle and divide it by two?  ComputedStyleSection.css has `calc(var(--disclosure-button-size) + 6px)`, so you could rework that into a variable and use `calc(var(--disclosure-area-width) / 2)` as your value instead of something hardcoded.
> > 
> > I've tried:
> > 
> >     width: var(--disclosure-button-size);
> >     margin-left: calc(var(--disclosure-button-size) * -1);
> >     text-align: center;
> > 
> > Unfortunately, it was a few pixels off :(
> 
> You'd need to take into account the extra 6px added in
> `calc(var(--disclosure-button-size) + 6px)`, or are you saying it was still
> off even with that?

These 6px are indentation *before* the disclosure button. You wouldn't want to use them to align the list bullet with the disclosure button.

And yes, it was still off.

[Nikita Vasilyev]

Created attachment 357728 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 357728 [details]
Patch

Clearing flags on attachment: 357728

Committed r239399: <https://trac.webkit.org/changeset/239399>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.