[webkit-reviews] review granted: [Bug 129913] Web Inspector: show a debugging-oriented dashboard when scripts pause : [Attachment 226274] the patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has granted Brian Burg <bburg at apple.com>'s
request for review:
Bug 129913: Web Inspector: show a debugging-oriented dashboard when scripts
pause
https://bugs.webkit.org/show_bug.cgi?id=129913

Attachment 226274: the patch
https://bugs.webkit.org/attachment.cgi?id=226274&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226274&action=review


> Source/WebInspectorUI/UserInterface/Models/DebuggerDashboard.js:35
> +    __proto__: WebInspector.Object.prototype,
> +
> +};

Nit: stray newline.

> Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css:63
> +    background-color: #999;

Nit: rgb(153, 153, 153)

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:73
> +    from { opacity: 0.7; -webkit-transform: scale(1); }
> +    to { opacity: 1; -webkit-transform: scale(1.1); }

Not sure I would do scale. But I would have to see it.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:58
> +WebInspector.DebuggerDashboardView.FunctionIconStyleClassName =
WebInspector.CallFrameTreeElement.FunctionIconStyleClassName;
> +WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName =
WebInspector.CallFrameTreeElement.EventListenerIconStyleClassName;

There are other places in the code where we cross reference directly. I like
this approach better though.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:85
> +	   // This is more than likely an event listener function with an "on"
prefix and it is
> +	   // as long or longer than the shortest event listener name --
"oncut".
> +	   if (callFrame.functionName &&
callFrame.functionName.startsWith("on") && callFrame.functionName.length >= 5)
> +	       iconClassName =
WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName;

We should probably have a helper for this logic. I think this is the third copy
of it.