[webkit-reviews] review granted: [Bug 176988] Web Inspector: REGRESSION(r222057): recording state doesn't update when changing actions : [Attachment 320878] Patch

[bugzilla-daemon at webkit.org]

Matt Baker <mattbaker at apple.com> has granted Devin Rousso
<webkit at devinrousso.com>'s request for review:
Bug 176988: Web Inspector: REGRESSION(r222057): recording state doesn't update
when changing actions
https://bugs.webkit.org/show_bug.cgi?id=176988

Attachment 320878: Patch

https://bugs.webkit.org/attachment.cgi?id=320878&action=review




--- Comment #2 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 320878
  --> https://bugs.webkit.org/attachment.cgi?id=320878
Patch

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

r=me

>
Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:
33
> +	   this._action = NaN;

Should be null instead of NaN.

>
Source/WebInspectorUI/UserInterface/Views/RecordingStateDetailsSidebarPanel.js:
84
> +    _generateDetailsCanvas2D(action, context, options = {})

A more descriptive name would be nice. How about
`_populateCanvasStateDataGrid`? I think having the name be canvas-type agnostic
is fine.

>
Source/WebInspectorUI/UserInterface/Views/RecordingTraceDetailsSidebarPanel.js:
82
> +	       noTraceDataMessageElement.textContent = WI.UIString("No Trace
Data");

What about "Call Stack Unavailable" or "No Call Stack" instead? As an aside, I
think we should standardize on referring to traces/stack traces as call stacks
in user-facing text. Variables names for things can still be trace, etc.