Bug 144461 - [GTK] Web Inspector: icons for console.info() messages
Summary: [GTK] Web Inspector: icons for console.info() messages
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gomez Garcia
URL:
Keywords: InRadar
Depends on: 18530
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-30 11:35 PDT by Andres Gomez Garcia
Modified: 2015-05-05 06:45 PDT (History)
12 users (show)

See Also:


Attachments
Patch (5.96 KB, patch)
2015-04-30 11:43 PDT, Andres Gomez Garcia
no flags Details | Formatted Diff | Diff
Patch for landing (5.88 KB, patch)
2015-05-05 06:31 PDT, Andres Gomez Garcia
no flags Details | Formatted Diff | Diff
Patch for landing (5.88 KB, patch)
2015-05-05 06:34 PDT, Andres Gomez Garcia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gomez Garcia 2015-04-30 11:35:25 PDT
GTK icons for bug 18530.
Comment 1 Radar WebKit Bug Importer 2015-04-30 11:35:38 PDT
<rdar://problem/20766903>
Comment 2 Andres Gomez Garcia 2015-04-30 11:36:09 PDT
Taking.
Comment 3 Andres Gomez Garcia 2015-04-30 11:43:02 PDT
Created attachment 252075 [details]
Patch
Comment 4 Joseph Pecoraro 2015-04-30 12:29:50 PDT
Comment on attachment 252075 [details]
Patch

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

rs=me, it would be funny if this lands before the other console.info change lands =)

> Source/WebInspectorUI/UserInterface/Images/gtk/Debug.svg:1
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>

I commented on another patch that I don't think the standalone="no" lines are needed. You should be able to remove them and simplify things a bit more.

> Source/WebInspectorUI/UserInterface/Images/gtk/Debug.svg:5
> +  <path style="color:#000000" d="m10.025 3.125 4.875 4.875l-4.875 4.875h-7.3412c-0.4311-0.018-0.784-0.4-0.784-0.813v-8.125c0-0.4308 0.2808-0.812 0.8197-0.812z" fill="#729fcf"/>

One thing we have been doing with our SVGs is adding a class name denoting a path that is filled versus stroked.

    <path d="..." class="filled" fill="..."/>
    <path d="..." class="stroked" stroke="..."/>

The class names serve no purpose right now, but in the past we have used CSS to customize the stroke / fill color of SVG images. You may want to adopt this pattern too. There are no plans to use it, we have just been proactively adding it.
Comment 5 Andres Gomez Garcia 2015-05-05 06:20:31 PDT
(In reply to comment #4)
> > Source/WebInspectorUI/UserInterface/Images/gtk/Debug.svg:1
> > +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> 
> I commented on another patch that I don't think the standalone="no" lines
> are needed. You should be able to remove them and simplify things a bit more.

Yep.

> > Source/WebInspectorUI/UserInterface/Images/gtk/Debug.svg:5
> > +  <path style="color:#000000" d="m10.025 3.125 4.875 4.875l-4.875 4.875h-7.3412c-0.4311-0.018-0.784-0.4-0.784-0.813v-8.125c0-0.4308 0.2808-0.812 0.8197-0.812z" fill="#729fcf"/>
> 
> One thing we have been doing with our SVGs is adding a class name denoting a
> path that is filled versus stroked.
> 
>     <path d="..." class="filled" fill="..."/>
>     <path d="..." class="stroked" stroke="..."/>
> 
> The class names serve no purpose right now, but in the past we have used CSS
> to customize the stroke / fill color of SVG images. You may want to adopt
> this pattern too. There are no plans to use it, we have just been
> proactively adding it.

I'm adding this now.

Thanks a lot for your review.
Comment 6 Andres Gomez Garcia 2015-05-05 06:31:19 PDT
Created attachment 252379 [details]
Patch for landing
Comment 7 Andres Gomez Garcia 2015-05-05 06:34:46 PDT
Created attachment 252380 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2015-05-05 06:45:12 PDT
Comment on attachment 252380 [details]
Patch for landing

Clearing flags on attachment: 252380

Committed r183804: <http://trac.webkit.org/changeset/183804>
Comment 9 WebKit Commit Bot 2015-05-05 06:45:17 PDT
All reviewed patches have been landed.  Closing bug.