Bug 143108

Summary: Web Inspector: Convert sidebar classes to ES6
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 142891    
Attachments:
Description Flags
Patch
joepeck: review+, joepeck: commit-queue-
Patch (Ignore Space) none

Description Timothy Hatcher 2015-03-26 11:44:40 PDT
The sidebar classes are easy to convert and working fine.
Comment 1 Radar WebKit Bug Importer 2015-03-26 11:45:09 PDT
<rdar://problem/20312676>
Comment 2 Timothy Hatcher 2015-03-26 11:47:15 PDT
Created attachment 249503 [details]
Patch
Comment 3 Timothy Hatcher 2015-03-26 11:48:19 PDT
Created attachment 249504 [details]
Patch (Ignore Space)
Comment 4 Joseph Pecoraro 2015-03-26 12:08:42 PDT
Comment on attachment 249504 [details]
Patch (Ignore Space)

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

r=me

> Source/WebInspectorUI/UserInterface/Views/ApplicationCacheDetailsSidebarPanel.js:140
> +WebInspector.ApplicationCacheDetailsSidebarPanel.StyleClassName = "application-cache";

For single use StyleClassName's I've inlining them. Donno if you feel the same, but this feels really verbose for little benefit.

> Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js:36
> -    this.element.classList.add(WebInspector.DOMNodeDetailsSidebarPanel.StyleClassName);
> +        this.element.classList.add("dom-node");

I see sometimes you have been doing this as well.

> Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js:589
> +            console.log(attribute.name, attribute.value);
> +
>              var node = new WebInspector.DataGridNode({name: attribute.name, value: attribute.value || ""}, false);
> +            console.log(node);

Looks like debug logging we should remove.

> Source/WebInspectorUI/UserInterface/Views/Sidebar.js:274
> +    CollapsedStateDidChange: "sidebar-sidebar-collapsed-state-did-change",

Compared to the others, this one has an extra "sidebar-".
Comment 5 Timothy Hatcher 2015-03-26 16:39:28 PDT
Comment on attachment 249503 [details]
Patch

https://trac.webkit.org/r182041