Bug 169696 – Web Inspector: WebSockets: Update Arrow Up icon to fit in with the rest of our iconography

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

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

Also, use half pixel values to make to more non-retina friendly.

[Radar WebKit Bug Importer]

<rdar://problem/31073748>

[Nikita Vasilyev]

Created attachment 304548 [details]
Patch

[Nikita Vasilyev]

Via https://bugs.webkit.org/show_bug.cgi?id=169011#c6

> Based on the screenshot this icon doesn't really fit in with the rest of our iconography. Compare to other arrows (download arrow, step arrows, gc button arrows, network tab icon arrows). We should file a follow-up to improve on this icon.

[Matt Baker]

Comment on attachment 304548 [details]
Patch

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

r=me, looks nice and clean!

Why is it necessary to specify the stroke width? Is it possible to do so in CSS?

> Source/WebInspectorUI/UserInterface/Images/ArrowUp.svg:5
> +    <path stroke="currentColor" stroke-width="1px" fill="none" d="M7.5,13V4"/>

Style: most (all?) of the Inspector SVGs use space-delimited path data.

[Nikita Vasilyev]

(In reply to comment #4)
> Comment on attachment 304548 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304548&action=review
> 
> r=me, looks nice and clean!
> 
> Why is it necessary to specify the stroke width? Is it possible to do so in
> CSS?

Yes, it is possible to specify `stroke-width` in CSS, as well as `stroke` and `fill`, e.g.:

    <style>
    path {
        stroke: currentColor;
        stroke-width: 2px;
        fill: none;
    }
    </style>

Is this what you want me to do?

> 
> > Source/WebInspectorUI/UserInterface/Images/ArrowUp.svg:5
> > +    <path stroke="currentColor" stroke-width="1px" fill="none" d="M7.5,13V4"/>
> 
> Style: most (all?) of the Inspector SVGs use space-delimited path data.

This is a Photoshop output. I need to write a code style script for this :/

[Matt Baker]

(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 304548 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=304548&action=review
> > 
> > r=me, looks nice and clean!
> > 
> > Why is it necessary to specify the stroke width? Is it possible to do so in
> > CSS?
> 
> Yes, it is possible to specify `stroke-width` in CSS, as well as `stroke`
> and `fill`, e.g.:
> 
>     <style>
>     path {
>         stroke: currentColor;
>         stroke-width: 2px;
>         fill: none;
>     }
>     </style>
> 
> Is this what you want me to do?

I think it makes sense for `fill: none` to be baked directly into the SVG file, since the arrowhead isn't meant to be solid.

You can style the other properties in WebSocketContentView.css, since the SVG is created with useSVGSymbol. However, I think you can drop the stroke-width and default color. Why is it 2px for GTK?

> > > Source/WebInspectorUI/UserInterface/Images/ArrowUp.svg:5
> > > +    <path stroke="currentColor" stroke-width="1px" fill="none" d="M7.5,13V4"/>
> > 
> > Style: most (all?) of the Inspector SVGs use space-delimited path data.
> 
> This is a Photoshop output. I need to write a code style script for this :/

I recall using a WebKit script for cleaning up SVG files, but can't find it. We should only need:

<?xml version="1.0"?>
<!-- Copyright © 2017 Apple Inc. All rights reserved. -->
<svg xmlns="http://www.w3.org/2000/svg" id="root" version="1.1" viewBox="0 0 16 16">
    <path stroke="currentColor" fill="none" d="M 3.5 7.5 L 7.5 3.5 l 4 4"/>
    <path stroke="currentColor" fill="none" d="M 7.5 13 V 4"/>
</svg>

[Nikita Vasilyev]

Created attachment 304585 [details]
Patch

[Nikita Vasilyev]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Comment on attachment 304548 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=304548&action=review
> > > 
> > > r=me, looks nice and clean!
> > > 
> > > Why is it necessary to specify the stroke width? Is it possible to do so in
> > > CSS?
> > 
> > Yes, it is possible to specify `stroke-width` in CSS, as well as `stroke`
> > and `fill`, e.g.:
> > 
> >     <style>
> >     path {
> >         stroke: currentColor;
> >         stroke-width: 2px;
> >         fill: none;
> >     }
> >     </style>
> > 
> > Is this what you want me to do?
> 
> I think it makes sense for `fill: none` to be baked directly into the SVG
> file, since the arrowhead isn't meant to be solid.
> 
> You can style the other properties in WebSocketContentView.css, since the
> SVG is created with useSVGSymbol.

I don't think I can. What CSS selector would I use?

> Why is it 2px for GTK?

To match other GTK icons. They tend to be thicker.

[WebKit Commit Bot]

Comment on attachment 304585 [details]
Patch

Clearing flags on attachment: 304585

Committed r214021: <http://trac.webkit.org/changeset/214021>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 304585 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Images/ArrowUp.svg:5
> +    <path stroke="currentColor" fill="none" d="M3.5 7.5 L7.5 3.5 l4 4"/>
> +    <path stroke="currentColor" fill="none" d="M7.5 13 V4"/>

Normally we have spaces after the letters. So M, L, and V here.