[Webkit-unassigned] [Bug 36606] Web Inspector: Short Records filter sould be implemented in Timeline Panel

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36606


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51648|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2010-03-25 10:27:13 PST ---
(From update of attachment 51648)
Overall looks good. Please attach screenshot and fix comments!

> +    _toggleFilterButtonClicked: function()
> +    {
> +        this.toggleFilterButton.toggled = !this.toggleFilterButton.toggled;
> +        this._calculator._showShortEvents = this.toggleFilterButton.toggled;
> +        this._scheduleRefresh();

I think you want immediate refresh here.

>              }
> +            record = formattedRecord;
> +            while (record._isLongEvent && record.parent && !record.parent._isLongEvent) {
> +                record = record.parent;
> +                record._hasLongChildrenEvents = true;

Comment this?

> -        if (record.children.length) {
> +        if (record.visibleChildrenCount || (record.collapsed && record.children.length && (record._hasLongChildrenEvents || calculator._showShortEvents))) {

Comment would be nice. Something like: We show expand element for expanded
nodes or for nodes that are collapsed and have unfiltered children, or we are
not filtering.

>              this._expandElement.style.top = index * this._rowHeight + "px";
>              this._expandElement.style.left = barPosition.left + "px";
>              this._expandElement.style.width = Math.max(12, barPosition.width + 25) + "px";
> @@ -737,6 +758,12 @@ WebInspector.TimelinePanel.FormattedRecord = function(record, parentRecord, reco
>  }
>  
>  WebInspector.TimelinePanel.FormattedRecord.prototype = {
> +    get _isLongEvent()


This should be a function, not getter.

> +    {
> +        const shortEventLength = 0.015;
> +        return this._hasLongChildrenEvents || (this._lastChildEndTime - this.startTime) > shortEventLength;

No need for _hasLongChildrenEvents here.

> +    },
> +

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.