[webkit-reviews] review granted: [Bug 227098] Web Inspector: Styles: should autocomplete `var()` and `attr()` values : [Attachment 432696] Patch 1.3

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Razvan Caliman
<rcaliman at apple.com>'s request for review:
Bug 227098: Web Inspector: Styles: should autocomplete `var()` and `attr()`
values
https://bugs.webkit.org/show_bug.cgi?id=227098

Attachment 432696: Patch 1.3

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




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

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

r=me, nice work :)

> Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js:114
> +	   completions.addValues(contextualValueCompletions);

We should either not call this if `contextualValueCompletions` is empty or have
a check inside `addValues` that bails if the provided array is empty so that we
don't `sort()` unnecessarily.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:347
> +	       return
Array.from(this._property.ownerStyle.nodeStyles.allCSSVariables);
> +	   case "attr":

Style: missing newline between