[Webkit-unassigned] [Bug 174176] Web Inspector: [META] Add 3D layer visualization

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=174176

--- Comment #11 from Ross Kirsling <ross.kirsling at sony.com> ---
(In reply to Devin Rousso from comment #10)

> > Source/WebInspectorUI/UserInterface/Views/Layers3DContentView.js:60
> > +    shown()
> 
> This should be under Protected.

I can't tell whether this is true or not... 🤔 ContentView itself doesn't specify anything as Protected and it looks like shown/hidden are only called from other classes. Admittedly, there are a few derived classes calling it Protected though. I'm happy to do whichever.

> > Source/WebInspectorUI/UserInterface/Views/LayersTabContentView.js:26
> > +WebInspector.LayersTabContentView = class LayersTabContentView extends WebInspector.ContentBrowserTabContentView
> 
> Extending ContentBrowserTabContentView means that you get a bunch of extra
> UI (sidebars, back/forward, navigation bar, etc.) that it doesn't look like
> you need here.  I think you should just extend from TabContentView (see
> NewTabContentView for an example).

To be sure, this is only used for the this.contentBrowser.showContentView call at the moment, but I figure we'll need the extra functionality soon, if we intend to have the visualization end up as one part of a split view? I suppose we could remove it until it becomes necessary though.

> > Source/WebInspectorUI/UserInterface/Views/LayersTabContentView.js:28
> > +    constructor(identifier)
> 
> This is an old style of constructing.  You can drop the `identifier`.

Everything that extends from ContentBrowserTabContentView or TabContentView seems to write those lines in the same way, so I followed suit...and yet, these are all being instantiated without arguments, so you must be right!

> > Source/WebInspectorUI/UserInterface/Views/LayersTabContentView.js:52
> > +    static shouldSaveTab()
> > +    {
> > +        return false;
> > +    }
> 
> Is there a reason that we don't want to save this tab?  In that case, do we
> also want to hide it in the New Tab view (isEphemeral)?

I did this to achieve the "closed by default" behavior that Nikita suggested, but given that this tab is feature-flagged and the idea for it has changed a little, I think this can probably go away.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170727/b40a82d6/attachment.html>