Summary: | Web Inspector: add the basics for the new TimelineSidebarPanel and TimelineContentView | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||
Component: | Web Inspector | Assignee: | Timothy Hatcher <timothy> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | graouts, joepeck, timothy, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Timothy Hatcher
2013-11-07 11:15:02 PST
Created attachment 216319 [details]
Patch
Created attachment 216324 [details]
Screenshot
Comment on attachment 216319 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=216319&action=review r=me > Source/WebInspectorUI/UserInterface/FrameContentView.js:-70 > - get allowedNavigationSidebarPanels() > - { > - return ["resource", "debugger"]; > - }, > - Why is this getting removed? > Source/WebInspectorUI/UserInterface/NavigationSidebarPanel.js:176 > + // Implemneted by subclasses if needed to show a content view when no existing tree element is selected. Typo: "Implemneted" => "Implemented" > Source/WebInspectorUI/UserInterface/ResourceClusterContentView.js:-69 > - get allowedNavigationSidebarPanels() > - { > - return ["resource", "debugger"]; > - }, > - Why is this getting removed? > Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.css:66 > + height: 23px; > + border-top: 1px solid rgb(179, 179, 179); Why a height of 23px and not the normal 22? Because of the 1px border border and box-sizing? > Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.css:67 > + top: 152px; I assume the top will eventually not be hardcoded? > Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.js:35 > + this.contentTreeOutlineLabel = ""; This is cryptic! The method defaults to "Timeline Events". I have no better ideas, other then just being explicit. > Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.js:42 > + this._timelinesTreeOutline.element.classList.add(WebInspector.NavigationSidebarPanel.HideDisclosureButtonsStyleClassName); Seems like this might make more sense as a little API on NavigationSidebarPanel, or a param to createContentTreeOutline. But fine as is! > Source/WebInspectorUI/UserInterface/TimelineSidebarPanel.js:92 > +WebInspector.TimelineSidebarPanel.TitleBarStyleClass = "title-bar"; > +WebInspector.TimelineSidebarPanel.TimelinesTitleBarStyleClass = "timelines"; > +WebInspector.TimelineSidebarPanel.TimelineEventsTitleBarStyleClass = "timeline-events"; > +WebInspector.TimelineSidebarPanel.TimelinesContentContainerStyleClass = "timelines-content"; > +WebInspector.TimelineSidebarPanel.CloseButtonStyleClass = "close-button"; > +WebInspector.TimelineSidebarPanel.LargeIconStyleClass = "large"; > +WebInspector.TimelineSidebarPanel.StopwatchIconStyleClass = "stopwatch-icon"; > +WebInspector.TimelineSidebarPanel.NetworkIconStyleClass = "network-icon"; > +WebInspector.TimelineSidebarPanel.ColorsIconStyleClass = "colors-icon"; > +WebInspector.TimelineSidebarPanel.ScriptIconStyleClass = "script-icon"; Yeesh. I think sometimes I would prefer use once style class names being inline instead of having variables. But this is the convention for now. |