Bug 190388 – Web Inspector: indent all network entries when "Group by Node" is checked

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

It looks weird to have only some of the network entry rows be indented.  It should be more uniform.

[Devin Rousso]

Created attachment 351846 [details]
[Image] Screenshot of Issue

[Devin Rousso]

Created attachment 351850 [details]
Patch

[Devin Rousso]

Created attachment 351851 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 351850 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:13
> +        Hide the "Group by Node" checkbox until there is a node that can be grouped.

Hiding a setting some of the time seems like a bad idea. This is a persistent setting, right?

> Source/WebInspectorUI/ChangeLog:22
> +        Apply a padding to all nodes when the `WI.Table` is grouped.

I do like the idea of only adding padding when grouped. I think the screenshot makes the grouping look much clearer. I wish there was something we could do about the wasted space where most of the disclosure triangles would be. What if we just don't have a disclosure triangle (and don't allow collapsing). Does that look or feel weird?

[EWS Watchlist]

Comment on attachment 351850 [details]
Patch

Attachment 351850 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9502203

New failing tests:
js/dom/custom-constructors.html

[EWS Watchlist]

Created attachment 351874 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Devin Rousso]

Comment on attachment 351850 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:13
>> +        Hide the "Group by Node" checkbox until there is a node that can be grouped.
> 
> Hiding a setting some of the time seems like a bad idea. This is a persistent setting, right?

It isn't visible until we are able to group by node for the first time.  From that point on (even if you navigate the page), it'll stay visible.  My thinking on this is that it would be weird to show a checkbox that doesn't appear to have any effect (e.g. for a page with no media elements), but I suppose we do this elsewhere too (still showing the pretty print icon for things that are already nicely formatted).  I can remove.

>> Source/WebInspectorUI/ChangeLog:22
>> +        Apply a padding to all nodes when the `WI.Table` is grouped.
> 
> I do like the idea of only adding padding when grouped. I think the screenshot makes the grouping look much clearer. I wish there was something we could do about the wasted space where most of the disclosure triangles would be. What if we just don't have a disclosure triangle (and don't allow collapsing). Does that look or feel weird?

I feel like the disclosure triangle is the "indicator" of the fact that this item is a parent.  It is aggravating to "waste" all this space, especially since most network entries won't be grouped, but we do this all the time (e.g. timelines).  I will, however, do a mockup and see how it looks without the triangle.

[Timothy Hatcher]

This looks better! I do think the indentation of the children needs to be more — the icon should align with the text. Right now it seems like a half indent.

See other places in the Inspector to see what I mean.

[Devin Rousso]

Created attachment 351922 [details]
[Image] Screenshot of Resources

(In reply to Timothy Hatcher from comment #8)
> This looks better! I do think the indentation of the children needs to be
> more — the icon should align with the text. Right now it seems like a half
> indent.
> 
> See other places in the Inspector to see what I mean.
Per our conversation in person, I tried to match this as close as I could with our other `WI.TreeOutline`s.

[Devin Rousso]

Created attachment 351947 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 351947 [details]
Patch

Clearing flags on attachment: 351947

Committed r237006: <https://trac.webkit.org/changeset/237006>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45165926>