Bug 207587 – Web Inspector: Update resource, type, and instrument icons for light, dark, and override colors

• Component: Web Inspector
• Status: RESOLVED FIXED

[Jon Davis]

Update the remaining icon artwork to support different color schemes particularly for light, dark, and override icons.

[Jon Davis]

Created attachment 390430 [details]
Patch

[Jon Davis]

Created attachment 390434 [details]
Sources tab in dark mode

[Jon Davis]

Created attachment 390435 [details]
Sources tab light mode

[Jon Davis]

Created attachment 390436 [details]
Timeline tab in dark mode

[Jon Davis]

Created attachment 390437 [details]
Timeline tab in light mode

[Jon Davis]

Created attachment 390438 [details]
Network tab in dark mode

[Jon Davis]

Created attachment 390439 [details]
Network tab in light mode

[Jon Davis]

Created attachment 390442 [details]
Sources tab in dark mode

[Jon Davis]

Created attachment 390443 [details]
Sources tab in light mode

[Blaze Burg]

Comment on attachment 390430 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ResourceIcons.css:18
> + * CONSEQUENTIAL DAMAGES (INCLUDING, RBUT NOT LIMITED TO, PROCUREMENT OF

Nit: Don't RBUT about it to me!

[Blaze Burg]

Comment on attachment 390430 [details]
Patch

I only found one little error. This is a neat way to do a sprite sheet!

My only suggestion is that -dark be renamed to -light-theme and -light be renamed to -dark-theme. If I were to inspect the Inspector, it would be confusing to use Dark mode (eg. dark theme) and see icons labelled as -light. I prefer the label to be of the corresponding theme (dark or light) rather than the luma values of the image.

r=me

[Timothy Hatcher]

Comment on attachment 390430 [details]
Patch

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

Very cool! I agree with Brian that the dark mode ids should use dark, and light-mode should use light.

> Source/WebInspectorUI/UserInterface/Images/ApplicationCacheManifest.svg:10
> +        <style>
> +          g { visibility: hidden; }
> +          g:target { visibility: visible; }
> +          #dark { color: black; }
> +          #light { color: white; }
> +        </style>

Should be 4 spaces indent on the last level.

Why visibility? I think g:not(:target) { display: none; } will work best, it will not make render objects for the hidden group like visibility: hidden will. (Ditto for this in the other images.)

> Source/WebInspectorUI/UserInterface/Images/Cookie.svg:8
> +    :root {
> +      --caramel: hsl(36, 65%, 40%);
> +      --shading: black;
> +    }

Should indent one more level, and the last level should be 4 spaces.

> Source/WebInspectorUI/UserInterface/Images/TypeIcons.svg:20
> +        .dark {
> +            --outline: hsla(0, 0%, 100%, 0.5);
> +        }
> +        .light {
> +            --outline: hsla(0, 0%, 0%, 0.5);
> +            --shadow: hsla(0, 0%, 0%, 0.5);
> +        }
> +
> +        .dark.grey {

The extra lines between the rules are nice, and I would do them everywhere. But this image has some grouped and not others. Consistency is better.

> Source/WebInspectorUI/UserInterface/Images/TypeIcons.svg:263
> +    <g id="Assertion-dark" class="dark blue"><use href="#box"/><use href="#A"/></g>
> +    <g id="Assertion-light" class="light blue"><use href="#box"/><use href="#A"/></g>
> +    <g id="AuditTestCase-dark" class="dark blue"><use href="#box"/><use href="#t"/></g>

This is hot!

> Source/WebInspectorUI/UserInterface/Views/ResourceIcons.css:110
> +    .resource-icon .icon {
> +        content: url(../Images/DocumentIcons.svg#generic-light);
> +    }
> +    .resource-icon.override .icon {
> +        content: url(../Images/DocumentIcons.svg#generic-light-override);
> +    }

Add the lines between the rules here, and else where in this block.

[Timothy Hatcher]

Comment on attachment 390430 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Images/TypeIcons.svg:11
> +        g {
> +            visibility: hidden;
> +        }
> +        g:target {
> +            visibility: visible;
> +        }

Using display: none is critical here, since the groups not targeted are being rendered invisibly and all those paths will have a large impact on WebKit, especially if there are a lot of type icons on screen multiples by all the hidden ones.

[Timothy Hatcher]

The close icon in the Network tab content view corner is hard to see in dark mode. That seems to, since it isn't that low contrast on my machine.

[Timothy Hatcher]

(In reply to Timothy Hatcher from comment #14)
> The close icon in the Network tab content view corner is hard to see in dark
> mode. That seems to, since it isn't that low contrast on my machine.

Oh, I think your screenshot was a background window so it was dim like the other buttons.

[Jon Davis]

Created attachment 390769 [details]
Patch

[Timothy Hatcher]

Comment on attachment 390769 [details]
Patch

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

Looking great! Just some minor style issues and one case of visibility.

> Source/WebInspectorUI/UserInterface/Images/Database.svg:7
> +            g { visibility: hidden; }
> +            g:target { visibility: visible; }

Old approach still in this file.

> Source/WebInspectorUI/UserInterface/Images/Database.svg:13
> +            }
> +            #dark {

Also needs line breaks.

> Source/WebInspectorUI/UserInterface/Images/DocumentIcons.svg:6
> +        <style>
> +        g:not(:target) {

Style rules not indented like the rest of the images.

> Source/WebInspectorUI/UserInterface/Images/FolderGeneric.svg:6
> +        <style>
> +          g:not(:target) {

Wrong indentation here.

> Source/WebInspectorUI/UserInterface/Images/InstrumentIcons.svg:4
> +<svg xmlns="http://www.w3.org/2000/svg" version="1.1">
> +<defs>

Not indented at a second level like the rest.

> Source/WebInspectorUI/UserInterface/Images/InstrumentIcons.svg:9
> +        }
> +        .dark {

Add line break.

> Source/WebInspectorUI/UserInterface/Images/Stopwatch.svg:9
> +            }
> +            #light {

Line break.

> Source/WebInspectorUI/UserInterface/Images/Stopwatch.svg:14
> +            }
> +            #dark {

Ditto.

> Source/WebInspectorUI/UserInterface/Images/TypeIcons.svg:6
> +        <style>
> +        g:not(:target) {

Indentation.

[Jon Davis]

Created attachment 390949 [details]
Patch

[Jon Davis]

Created attachment 390951 [details]
Sources tab in dark mode

[Jon Davis]

Created attachment 390952 [details]
Sources tab in light mode

[Jon Davis]

Created attachment 390955 [details]
Patch

[Timothy Hatcher]

Comment on attachment 390955 [details]
Patch

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

I like the full color resource icons for overrides! I don’t think the dog ear needs to be orange then. Just match the file color and shade it instead? Or have it be light (dark) as if ony the front of the paper is colored.

> Source/WebInspectorUI/UserInterface/Images/InstrumentIcons.svg:9
> +            }
> +            .dark {

Found one!

[Jon Davis]

Created attachment 390965 [details]
Patch

[Jon Davis]

Created attachment 390972 [details]
Sources tab in dark mode

[Jon Davis]

Created attachment 390973 [details]
Sources tab in light mode

[Jon Davis]

Created attachment 390974 [details]
Network tab in dark mode

[Jon Davis]

Created attachment 390975 [details]
Network tab in light mode

[Jon Davis]

Created attachment 390976 [details]
Timeline tab in dark mode

[Jon Davis]

Created attachment 390977 [details]
Timeline tab in light mode

[WebKit Commit Bot]

Comment on attachment 390965 [details]
Patch

Clearing flags on attachment: 390965

Committed r256774: <https://trac.webkit.org/changeset/256774>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59526753>