[Webkit-unassigned] [Bug 20629] Inspector Resources / Graphs should support filtering

[bugzilla-daemon at webkit.org]

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


Timothy Hatcher <timothy at hatcher.name> changed:

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




--- Comment #7 from Timothy Hatcher <timothy at hatcher.name>  2009-08-10 13:21:50 PDT ---
(From update of attachment 34502)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog

> +    for (var i = 0; i < this.categoryOrder.length; i++) {
> +        createFilterElement.bind(this)(this.categoryOrder[i]);
> +    }

No need for the curly braces here. Also no need for bind, you can just do:

createFilterElement.call(this, this.categoryOrder[i]);

> +    get filterItem()
> +    {
> +        return this._filterItem;
> +    },
> +
> +    set filterItem(x)
> +    {
> +        if (!x || this._filterItem === x)
> +            return;
> +    
> +        if (this._filterItem) {
> +            this._filterItem.removeStyleClass("selected");
> +            var oldClass = "filter-" + this._filterItem.firstChild.data.toLowerCase();
> +            this.resourcesTreeElement.childrenListElement.removeStyleClass(oldClass);
> +            this.resourcesGraphsElement.removeStyleClass(oldClass);
> +        }
> +        this._filterItem = x;
> +        this._filterItem.addStyleClass("selected");
> +        var newClass = "filter-" + this._filterItem.firstChild.data.toLowerCase();
> +        this.resourcesTreeElement.childrenListElement.addStyleClass(newClass);
> +        this.resourcesGraphsElement.addStyleClass(newClass);
> +    },

It seems odd to need a getter and a setter for filterItem that takes a DOM
element, and you only ever set it. A function called "filter" would be more
targeted. The function could take a category identifier string and update the
selected DOM node. (You can use getElementsByClassName to find them.) You would
want to store the current filter category string, so you can unselect it, and
maybe prevent doing more work if the same one is filtered the next time.

It is a little fragile as-is since it uses .firstChild.data. You could use the
string passed into the function for making the class name. And you can store
the category name on the DOM element when you make the scope buttons. So
_updateFilterItem would look like:

_updateFilterItem: function (e) {
    this.filter(e.target.category);
}

Then you have an API that can be called by anyone, not just someone that has a
DOM element.

> +        console.log(this.categoryOrder);

Remove the console.log.

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