[webkit-reviews] review denied: [Bug 35971] Web Inspector: Implement a progress indicator in the Audits panel when checks are running : [Attachment 50607] [PATCH] Separated progress message, added spinner

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 35971: Web Inspector: Implement a progress indicator in the Audits panel
when checks are running
https://bugs.webkit.org/show_bug.cgi?id=35971

Attachment 50607: [PATCH] Separated progress message, added spinner
https://bugs.webkit.org/attachment.cgi?id=50607&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +	   this.totalResources = 1;

Why 1 not 0?

> +	   this.totalResources++;

Should be ++this.totalResources;

> +	   this.loadedResources++;

Ditto.


> +	       this._resourceProgressTextElement.textContent =
WebInspector.UIString("Loading, completed %d of %d items",
this.loadedResources, this.totalResources);

I think "Loading (%d of %d)" is just as clear and shorter.

> +    vertical-align: sub;

I would expect middle, I never heard of sub until now. What does sub do? Does
middle give the same result?

Looks good. Will r+ if you fix these things.