[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