Bug 135870

Summary: Web Inspector: Timeline Close buttons can use polish for new and legacy styles
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
timothy: review+, joepeck: commit-queue-
[PATCH] For Landing none

Description Joseph Pecoraro 2014-08-12 17:56:51 PDT
* SUMMARY
Timeline Close buttons can use polish for new and legacy styles.

  - new styles the X looks too thin on timeline overviews
  - legacy styles the X for timeline records has a white border but black filled content

* STEPS TO REPRODUCE
1. Click around the timelines view with records
Comment 1 Radar WebKit Bug Importer 2014-08-12 17:56:59 PDT
<rdar://problem/17999596>
Comment 2 Joseph Pecoraro 2014-08-12 18:00:12 PDT
Created attachment 236489 [details]
[PATCH] Proposed Fix
Comment 3 Timothy Hatcher 2014-08-12 22:49:09 PDT
Comment on attachment 236489 [details]
[PATCH] Proposed Fix

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

Screenshot looks good.

> Source/WebInspectorUI/ChangeLog:20
> +        Use the Close.svg in new styles, and CloseLarge.svg in legacy, styled to 12x12.

I am not sure why CloseLarge.svg is used in Legacy and Close.svg is used in modern. Ideally they would be the same size and only differ by stroke-width 2 vs 1.

> Source/WebInspectorUI/UserInterface/Images/Legacy/Close.svg:4
> +    <path class="stroked filled" d="M 12.949219 10.535156 L 11.535156 11.949219 L 8 8.414062 L 4.464844 11.949219 L 3.050781 10.535156 L 6.585938 7 L 3.050781 3.464844 L 4.464844 2.050781 L 8 5.585938 L 11.535156 2.050781 L 12.949219 3.464844 L 9.414062 7 Z"/>

You can remove "stroked", "filled" is all that matters since there is no stroke-width.

> Source/WebInspectorUI/UserInterface/Images/Legacy/CloseLarge.svg:4
> +    <path class="stroked filled" fill="none" stroke="black" stroke-width="2" stroke-linecap="square" d="M 1.5 2.5 L 12.5 13.5 M 1.5 13.5 L 12.5 2.5"/>

This should only be "stroked". Filled would not apply here since fill is "none" and the path is not a closed path.

> Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.css:250
> + /* Close button on timeline records */

I don't think this is needed.
Comment 4 Joseph Pecoraro 2014-08-13 11:18:25 PDT
(In reply to comment #3)
> (From update of attachment 236489 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236489&action=review
> 
> Screenshot looks good.
> 
> > Source/WebInspectorUI/ChangeLog:20
> > +        Use the Close.svg in new styles, and CloseLarge.svg in legacy, styled to 12x12.
> 
> I am not sure why CloseLarge.svg is used in Legacy and Close.svg is used in modern. Ideally they would be the same size and only differ by stroke-width 2 vs 1.

Yep, we figured out what was wrong (I added stroked which added thickness) and this let us revert all the .js changes.

> 
> > Source/WebInspectorUI/UserInterface/Images/Legacy/Close.svg:4
> > +    <path class="stroked filled" d="M 12.949219 10.535156 L 11.535156 11.949219 L 8 8.414062 L 4.464844 11.949219 L 3.050781 10.535156 L 6.585938 7 L 3.050781 3.464844 L 4.464844 2.050781 L 8 5.585938 L 11.535156 2.050781 L 12.949219 3.464844 L 9.414062 7 Z"/>
> 
> You can remove "stroked", "filled" is all that matters since there is no stroke-width.

Ahh yes, and this was causing problems.
Comment 5 Joseph Pecoraro 2014-08-13 11:41:21 PDT
Created attachment 236539 [details]
[PATCH] For Landing
Comment 6 WebKit Commit Bot 2014-08-13 12:01:09 PDT
Comment on attachment 236539 [details]
[PATCH] For Landing

Clearing flags on attachment: 236539

Committed r172527: <http://trac.webkit.org/changeset/172527>
Comment 7 WebKit Commit Bot 2014-08-13 12:01:13 PDT
All reviewed patches have been landed.  Closing bug.