[webkit-reviews] review granted: [Bug 175343] Web Inspector: Styles Redesign: hook up real data to spreadsheet style editor : [Attachment 319559] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Sep 4 00:08:40 PDT 2017
Devin Rousso <webkit at devinrousso.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 175343: Web Inspector: Styles Redesign: hook up real data to spreadsheet
style editor
https://bugs.webkit.org/show_bug.cgi?id=175343
Attachment 319559: Patch
https://bugs.webkit.org/attachment.cgi?id=319559&action=review
--- Comment #18 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 319559
--> https://bugs.webkit.org/attachment.cgi?id=319559
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=319559&action=review
r=me
This looks really nice! I know I'm in the minority here, but I miss the
selector icons :(
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.
js:33
> +
this.element.classList.add(WI.SpreadsheetCSSStyleDeclarationEditor.StyleClassNa
me);
> + this._style = style;
Style: add newline
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.
js:63
> + this.layout();
NIT: should be `needsLayout(WI.View.LayoutReason.Dirty)`
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection
.js:38
> + this._delegate = delegate || null;
> +
> + this._style = style;
Style: unnecessary newline
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection
.js:165
> + let originString = "";
> + switch (this._style.ownerRule.type) {
Style: I'd personally add a newline for readability, but it's up to you
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection
.js:201
> + this._selectorElement.append(WI.UIString("Style Attribute"));
NIT: this could just be `textContent`
>
Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:
92
> + let section =
style[WI.SpreadsheetRulesStyleDetailsPanel.RuleSection];
> +
> + if (!section) {
Style: unnecessary newline
More information about the webkit-reviews
mailing list