[webkit-reviews] review granted: [Bug 191174] Web Inspector: Network: remove unnecessary media event tracking : [Attachment 353652] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Nov 1 16:06:45 PDT 2018
Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 191174: Web Inspector: Network: remove unnecessary media event tracking
https://bugs.webkit.org/show_bug.cgi?id=191174
Attachment 353652: Patch
https://bugs.webkit.org/attachment.cgi?id=353652&action=review
--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 353652
--> https://bugs.webkit.org/attachment.cgi?id=353652
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=353652&action=review
r=me
> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:91
> +.network-table .data-container .cell.name .range {
> + font-style: italic;
> +}
I'm not sure I like the italics here. For tree elements we typically put a
subtitle in a lighter gray color and not italicize. I'm not sure what would be
best we can experiment.
> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1103
> + setTimeout(() => {
> + if (this._waterfallPopover)
> + this._waterfallPopover.resize();
> + });
Why in a setTimeout? Should this be a debounce?
> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1929
> + this._handleMousedownWaterfall(nodeEntry, popoverContentElement,
(cell) => {
> + let domEventElement =
nodeEntry.domEventElements.get(domEvents[0]);
It is weird to name this a `getTargetElementFunction` when it may also change
the contents of the popover. I see why its happening though. Maybe a more
generic `resizeAndPositionHandler`/`repositionAndResizeHandler` might be
better?
More information about the webkit-reviews
mailing list