[webkit-reviews] review denied: [Bug 182353] Web Inspector: TabBar redesign: remove top-level search field and pin the Search tab : [Attachment 332921] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <webkit at devinrousso.com> has denied Matt Baker
<mattbaker at apple.com>'s request for review:
Bug 182353: Web Inspector: TabBar redesign: remove top-level search field and
pin the Search tab
https://bugs.webkit.org/show_bug.cgi?id=182353

Attachment 332921: Patch

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




--- Comment #13 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 332921
  --> https://bugs.webkit.org/attachment.cgi?id=332921
Patch

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

r-.  I'm not sure where this happens, but with the new setting enabled, I am
unable to select any search results.  Whenever I try, it always jumps to
Elements/Resources, as if I had double-clicked.  It works as expected with the
experimental setting turned off.  My guess is that making the Search tab pinned
means that it gets removed from the list of open tabs (`_recentTabContentViews`
in TabBrowser.js), and therefore when something tries to get shown via
WI.showRepresentedObject, it doesn't even consider showing it in the Search
tab.

Other than this issue, it looks great =D

> Source/WebInspectorUI/UserInterface/Views/SearchTabContentView.js:30
> +	   let tabBarItem;

NIT: I think our preferred style is to always assign some value to variables,
even if it's just `null`.

    let tabBarItem = null;