[webkit-reviews] review granted: [Bug 177896] Web Inspector: Network Tab - Headers Detail View : [Attachment 323030] [PATCH] Proposed Fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 6 15:10:29 PDT 2017
Devin Rousso <webkit at devinrousso.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 177896: Web Inspector: Network Tab - Headers Detail View
https://bugs.webkit.org/show_bug.cgi?id=177896
Attachment 323030: [PATCH] Proposed Fix
https://bugs.webkit.org/attachment.cgi?id=323030&action=review
--- Comment #29 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 323030
--> https://bugs.webkit.org/attachment.cgi?id=323030
[PATCH] Proposed Fix
View in context: https://bugs.webkit.org/attachment.cgi?id=323030&action=review
r=me
> Source/WebInspectorUI/ChangeLog:32
> + New event whenever metrics change. This is the first event that will
allow
> + a client to react to a Protocol change.
What does this mean? I think this can be reworded a bit for clarity.
> Source/WebInspectorUI/UserInterface/Views/NetworkResourceDetailView.css:82
> + padding: 0 20px 20px;
I tried messing with this a bit, and I think you could decrease the horizontal
padding here to 10px and the vertical padding of `.resource-details > section`
(ResourceDetailsSection.css), and you'll get a bit more space to work with.
Just a thought :)
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.css:26
> +body[dir] .resource-headers > section > .details {
I'm confused as to what the purpose of `body[dir]` is? Is it necessary?
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.css:53
> + /* Don't include indents in RTL */
I tried this locally, and it does seem really funky. I wonder why it does this
:/
> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:37
> + this._resource.addEventListener(WI.Resource.Event.MetricsDidChange,
this._resourceMetricsDidChange, this);
> +
this._resource.addEventListener(WI.Resource.Event.RequestHeadersDidChange,
this._resourceRequestHeadersDidChange, this);
> + this._resource.addEventListener(WI.Resource.Event.ResponseReceived,
this._resourceResponseReceived, this);
Can these event listeners be moved to initialLayout()? It seems they all call
`needsLayout()` anyway.
More information about the webkit-reviews
mailing list