[webkit-reviews] review granted: [Bug 190986] Web Inspector: refactor WI.ScopeBarItem for better extensibility : [Attachment 353242] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 190986: Web Inspector: refactor WI.ScopeBarItem for better extensibility
https://bugs.webkit.org/show_bug.cgi?id=190986

Attachment 353242: Patch

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




--- Comment #4 from Brian Burg <bburg at apple.com> ---
Comment on attachment 353242
  --> https://bugs.webkit.org/attachment.cgi?id=353242
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:8
> +	   * UserInterface/Views/ScopeBarItem.js:

You didn't say what the better extensibility is.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:82
> +	       new WI.ScopeBarItem(WI.LogContentView.Scopes.Debugs,
WI.UIString("Debugs"), {className: "debugs", hidden: true}),

Yaaassss

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:58
> +	       exclusive: true,

Nit: No need to wrap this line

> Source/WebInspectorUI/UserInterface/Views/ScopeBar.js:-155
> -	       var replacesCurrentSelection =
this._shouldGroupNonExclusiveItems || !event.data.withModifier;

Nit: let

> Source/WebInspectorUI/UserInterface/Views/ScopeBar.js:157
> +	       var replacesCurrentSelection =
this._shouldGroupNonExclusiveItems || !event.data.extendSelection;

You should mention this renaming in the changelog. I thought it was a mistake.