Bug 143914 – Web Inspector: Pass multiple arguments to classList.add and classList.remove

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

To make code more brief, instead of:

    for (var i = 0; i < this._classNames.length; ++i)
        this._listItemNode.classList.add(this._classNames[i]);

We should use:

    this._listItemNode.classList.add(...this._classNames);

And instead of:

    document.body.classList.remove("docked");
    document.body.classList.remove("right");
    document.body.classList.remove("bottom");

We should use:

    document.body.classList.remove("docked", "right", "bottom");

[Nikita Vasilyev]

Created attachment 251083 [details]
Patch

[Timothy Hatcher]

Comment on attachment 251083 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:35
> +        this._element.classList.add(identifier, WebInspector.DetailsSection.StyleClassName);

You could inline and remove the StyleClassName constant. We have started doing that in new code.

> Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js:80
> +            this._listItemNode.classList.remove(...this._classNames);

Nice!

> Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js:35
> +    this.element.classList.add(styleClassName, WebInspector.ResourceContentView.StyleClassName);

Ditto about StyleClassName.

[Nikita Vasilyev]

(In reply to comment #2)
> Comment on attachment 251083 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251083&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:35
> > +        this._element.classList.add(identifier, WebInspector.DetailsSection.StyleClassName);
> 
> You could inline and remove the StyleClassName constant. We have started
> doing that in new code.
> 
> > Source/WebInspectorUI/UserInterface/Views/GeneralTreeElement.js:80
> > +            this._listItemNode.classList.remove(...this._classNames);
> 
> Nice!
> 
> > Source/WebInspectorUI/UserInterface/Views/ResourceContentView.js:35
> > +    this.element.classList.add(styleClassName, WebInspector.ResourceContentView.StyleClassName);
> 
> Ditto about StyleClassName.

Besides these two, there are dozens of constants that are used only for CSS classes. They should probably be inlined in a separate patch.

[Nikita Vasilyev]

Created attachment 251107 [details]
Patch

[Timothy Hatcher]

You don't/can't set r+ if you put the reviewers name in the ChangeLog. You can also set cq+ yourself since you are a commiter of you got a r+ on a previous version and there were only minor tweaks needed.

[WebKit Commit Bot]

Comment on attachment 251107 [details]
Patch

Clearing flags on attachment: 251107

Committed r182990: <http://trac.webkit.org/changeset/182990>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.