[webkit-reviews] review canceled: [Bug 90871] Web Inspector: Display Named Flows in the "CSS Named Flows" drawer : [Attachment 164069] Patch

[bugzilla-daemon at webkit.org]

Alexander Pavlov (apavlov) <apavlov at chromium.org> has canceled Andrei Poenaru
<poenaru at adobe.com>'s request for review:
Bug 90871: Web Inspector: Display Named Flows in the "CSS Named Flows" drawer
https://bugs.webkit.org/show_bug.cgi?id=90871

Attachment 164069: Patch
https://bugs.webkit.org/attachment.cgi?id=164069&action=review

------- Additional Comments from Alexander Pavlov (apavlov)
<apavlov at chromium.org>
Much better overall, a few more nits left. Feel free to ask questions online.

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


> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:132
> +	       var overflowIcon = document.createElement("img");

Sorry for overlooking this earlier. We decorate tree elements simply by setting
appropriate classes on them and using images in .class:before. I can help you
write the related CSS if you wish.

So, in this case you will just do
flowTreeElement.title.addStyleClass("named-flow-overflow");
and have the respective CSS place the desired image in the :before
pseudo-element for you.

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:197
> +		   var overflowIcon = document.createElement("img");

Ditto

> Source/WebCore/inspector/front-end/CSSNamedFlowCollectionsView.js:223
> +	      
this._showNamedFlowForNode(WebInspector.panels["elements"].treeOutline.selected
DOMNode());

This is not the correct way to address panels now, since panels are loaded
lazily. Use
WebInspector.panel("elements").treeOutline....

> Source/WebCore/inspector/front-end/ElementsPanel.js:356
> +	  
WebInspector.showViewInDrawer(WebInspector.cssNamedFlowCollectionsView._statusE
lement, WebInspector.cssNamedFlowCollectionsView);

Since CSSNamedFlowCollectionsView is defined in a different file, you cannot
access its private field (_statusElement). Instead, you should have something
like
showInDrawer: function()
{
    WebInspector.showViewInDrawer(this._statusElement, this);
}
as a separate method on CSSNamedFlowCollectionsView.prototype, so that instead
of this line you can call
WebInspector.cssNamedFlowCollectionsView.showInDrawer();


_showNamedFlowCollections() is only necessary to create the view instance as a
field on the ElementsPanel lazily.

As an alternative, you can define showNamedFlowCollections() on
WebInspector.CSSNamedFlowCollectionsView (as a "static" field), and create the
view singleton as a field on WebInspector.CSSNamedFlowCollectionsView (e.g.
WebInspector.CSSNamedFlowCollectionsView._instance). This way, you will not
need the said showInDrawer() method at all.