Bug 143108 - Web Inspector: Convert sidebar classes to ES6
Summary: Web Inspector: Convert sidebar classes to ES6
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks: 142891
  Show dependency treegraph
 
Reported: 2015-03-26 11:44 PDT by Timothy Hatcher
Modified: 2015-04-21 20:31 PDT (History)
7 users (show)

See Also:


Attachments
Patch (220.73 KB, patch)
2015-03-26 11:47 PDT, Timothy Hatcher
joepeck: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff
Patch (Ignore Space) (136.82 KB, patch)
2015-03-26 11:48 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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