[webkit-reviews] review granted: [Bug 191037] Web Inspector: Styles: implement copying and deletion of multiple properties : [Attachment 353451] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 191037: Web Inspector: Styles: implement copying and deletion of multiple
properties
https://bugs.webkit.org/show_bug.cgi?id=191037

Attachment 353451: Patch

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




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

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

r=me. This is very clean, good work!

>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.
css:56
> +.multiple-properties-selection .spreadsheet-style-declaration-editor
:matches(.name, .value):not(.editing) {

Cool.

>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.
js:322
> +	       console.assert(false, `Nothing to select. anchorIndex
(${anchorIndex}) and focusIndex (${focusIndex}) must be numbers.`);

console.error?

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:124
> +    remove(replacement = "")

If this argument not going to be used as a string when it's the default value,
then I think the default value should be `null` so that this is unambiguous.