[webkit-reviews] review granted: [Bug 178323] Web Inspector: Network Tab - Metrics Detail View : [Attachment 324254] [PATCH] Proposed Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 19 14:29:49 PDT 2017


Devin Rousso <webkit at devinrousso.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 178323: Web Inspector: Network Tab - Metrics Detail View
https://bugs.webkit.org/show_bug.cgi?id=178323

Attachment 324254: [PATCH] Proposed Fix

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




--- Comment #14 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 324254
  --> https://bugs.webkit.org/attachment.cgi?id=324254
[PATCH] Proposed Fix

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

r=me.  One general comment; you have a lot of anonymous arrow functions that
have the same body.  I mentioned this in the review of the previous patch, but
would it be worth it to save these values to a `_bound*` variable so as to
reduce duplication?  This might be useful if the go-to arrows are recreated
multiple times (after refreshes), but I'm not sure how common that would be.

> Source/WebInspectorUI/ChangeLog:1
> +2017-10-15  Joseph Pecoraro	<pecoraro at apple.com>

Update date.

> Source/WebInspectorUI/UserInterface/Views/ResourceMetricsContentView.js:288
> +	       let p =
this._timingSection.appendChild(document.createElement("p"));

Style: this could use a better name, even `paragraphElement`.

> Source/WebInspectorUI/UserInterface/Views/ResourceMetricsContentView.js:313
> +	       block.style[property] = startOffset + "px";

NIT: use `setProperty`.

    block.style.setProperty(property, startOffset + "px");

> Source/WebInspectorUI/UserInterface/Views/ResourceMetricsContentView.js:325
> +	       timeLabel.style[property] = positionOffset + "px";

Ditto (313).


More information about the webkit-reviews mailing list