[webkit-reviews] review granted: [Bug 191770] Web Inspector: Remove parameters from TreeOutline SelectionDidChange event : [Attachment 355131] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 20 21:23:42 PST 2018


Devin Rousso <drousso at apple.com> has granted Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 191770: Web Inspector: Remove parameters from TreeOutline
SelectionDidChange event
https://bugs.webkit.org/show_bug.cgi?id=191770

Attachment 355131: Patch

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




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 355131
  --> https://bugs.webkit.org/attachment.cgi?id=355131
Patch

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

r=me, nice cleanup :)

Rather than use `event.target.selectedTreeElement`, it seems like most of these
cases could refer to the actual member `WI.TreeOutline` object (e.g.
CallFrameTreeController.js could use `this._treeOutline.selectedTreeElement`
instead, or most of the `WI.NavigationSidebarPanel` subclasses could use
`this.contentTreeOutline` instead)

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:845
> +	   let selectedTreeElement = event.target.selectedTreeElement;

NIT: I'd move this to be right above the `if` that checks for it's existence
(down one line)


More information about the webkit-reviews mailing list