[webkit-reviews] review granted: [Bug 210837] Web Inspector: Storage: unable to filter cookies : [Attachment 397203] 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 210837: Web Inspector: Storage: unable to filter cookies
https://bugs.webkit.org/show_bug.cgi?id=210837

Attachment 397203: Patch

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




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

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

r=me

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.css:-2
> - * Copyright (C) 2013 Apple Inc. All rights reserved.

Nit: 2013-2020 ?

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.css:26
> +.content-view.cookie-storage > .message-text-view {

Nice. I love how all these variables already exist!

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:439
> +	       this._cookies =
this._getCookiesForHost(payload.cookies.map(WI.Cookie.fromPayload),
this.representedObject.host);

v nice

> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:472
> +	   if (this._filteredCookies.length || !filterText) {

Nit: I'd put this DOM manipulation into a separate function since everything up
until now is maintaining this._filteredCookies.
this._updatePlaceholderForSearchResults or something?

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:-1623
> -	   this._emptyFilterResultsMessageElement.style.width = width + "px";

Weird.