Update the remaining icon artwork to support different color schemes particularly for light, dark, and override icons.
Created attachment 390430 [details] Patch
Created attachment 390434 [details] Sources tab in dark mode
Created attachment 390435 [details] Sources tab light mode
Created attachment 390436 [details] Timeline tab in dark mode
Created attachment 390437 [details] Timeline tab in light mode
Created attachment 390438 [details] Network tab in dark mode
Created attachment 390439 [details] Network tab in light mode
Created attachment 390442 [details] Sources tab in dark mode
Created attachment 390443 [details] Sources tab in light mode
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!
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
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.
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.
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.
(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.
Created attachment 390769 [details] Patch
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.
Created attachment 390949 [details] Patch
Created attachment 390951 [details] Sources tab in dark mode
Created attachment 390952 [details] Sources tab in light mode
Created attachment 390955 [details] Patch
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!
Created attachment 390965 [details] Patch
Created attachment 390972 [details] Sources tab in dark mode
Created attachment 390973 [details] Sources tab in light mode
Created attachment 390974 [details] Network tab in dark mode
Created attachment 390975 [details] Network tab in light mode
Created attachment 390976 [details] Timeline tab in dark mode
Created attachment 390977 [details] Timeline tab in light mode
Comment on attachment 390965 [details] Patch Clearing flags on attachment: 390965 Committed r256774: <https://trac.webkit.org/changeset/256774>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59526753>