[webkit-reviews] review granted: [Bug 200683] Web Inspector: Sources: Give Origins their own icon in the Sources sidebar : [Attachment 376213] [PATCH] Proposed Fix (Color)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 20 00:54:29 PDT 2019

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 200683: Web Inspector: Sources: Give Origins their own icon in the Sources

Attachment 376213: [PATCH] Proposed Fix (Color)


--- Comment #12 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 376213
  --> https://bugs.webkit.org/attachment.cgi?id=376213
[PATCH] Proposed Fix (Color)

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

r=me, nice!

> Source/WebInspectorUI/ChangeLog:15
> +	   (.origin-icon.main .icon):

This doesn't appear to exist in this patch.

> Source/WebInspectorUI/UserInterface/Images/Origin.svg:5
> +	   <radialGradient id="gradient" cx="58.60%" cy="27.74%" fx="58.60%"
fy="27.74%" r="57.50%" gradientTransform="translate(0.59, 0.28), rotate(98.61),
scale(1, 0.98), translate(-0.59, -0.28)">

NIT: you could remove the trailing `0` from `58.6%`.

> Source/WebInspectorUI/UserInterface/Images/Origin.svg:7
> +	       <stop stop-color="#A8ABB6" offset="0%"/>
> +	       <stop stop-color="#35374C" offset="100%"/>

We normally use `rgb()` values for our *.svg images.

> Source/WebInspectorUI/UserInterface/Images/Origin.svg:12
> +    <circle fill="none" stroke="#585A63" stroke-width="3" cx="31.9" cy="32"

Ditto (6).

> Source/WebInspectorUI/UserInterface/Main.html:532
> +    <script src="Views/OriginTreeElement.js"></script>

Does this need to be this far "up" in the list, or can it be put in the more
general grouping further below?

> +	       this._mainFrameTreeElement = new
WI.OriginTreeElement(mainFrame.securityOrigin, mainFrame, {hasChildren: true});

This will need to be rebased.

> +		       originTreeElement = new WI.OriginTreeElement(origin,
origin === resource.parentFrame.securityOrigin ? resource.parentFrame : null,
{hasChildren: true});

Ditto (648).

More information about the webkit-reviews mailing list