[webkit-reviews] review granted: [Bug 31784] Web Inspector: Reimplement TimelinePanel to make it scale : [Attachment 43675] [PATCH] the patch.

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 31784: Web Inspector: Reimplement TimelinePanel to make it scale
https://bugs.webkit.org/show_bug.cgi?id=31784

Attachment 43675: [PATCH] the patch.
https://bugs.webkit.org/attachment.cgi?id=43675&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +	   var visibleTop = this._containerElement.scrollTop;
> +	   var visibleBottom = visibleTop +
this._containerElement.clientHeight;
> +	   var rowHeight = 18;
> +	   var startIndex = Math.max(0, Math.floor(visibleTop / rowHeight) -
1);
> +	   var endIndex = Math.min(recordsInWindow.length,
Math.ceil(visibleBottom / rowHeight));

Use const instead of var for these.


> +	   var top = (startIndex * rowHeight) + "px";

Use const.


>  .sidebar {
>      position: absolute;
>      top: 0;
> +    min-height: 100%;
>      left: 0;
> -    bottom: 0;

Why was this needed?


>  .sidebar-resizer-vertical {
>      position: absolute;
>      top: 0;
> -    bottom: 0;
> +    min-height: 100%;

Ditto.


> +.timeline-tree-section {

Is this class uses in your patch? What is this for?


> +    color: rgb(92, 110, 129);
> +    font-family: Helvetica, Arial, sans-serif;

Why is this Helvetica? We don't use Helvetica except in a few rare places.
Otherwise the font is inherited from the body.


> +    position: relative;

Why relative, so it have positioned children?


> +    text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px;

Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision
anyway.


> +    font-family: Helvetica, Arial, sans-serif;

Why specify the font?