[webkit-reviews] review granted: [Bug 135870] Web Inspector: Timeline Close buttons can use polish for new and legacy styles : [Attachment 236489] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 135870: Web Inspector: Timeline Close buttons can use polish for new and
legacy styles
https://bugs.webkit.org/show_bug.cgi?id=135870

Attachment 236489: [PATCH] Proposed Fix
https://bugs.webkit.org/attachment.cgi?id=236489&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
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.