Bug 124003 - Web Inspector: add the basics for the new TimelineSidebarPanel and TimelineContentView
Summary: Web Inspector: add the basics for the new TimelineSidebarPanel and TimelineCo...
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:
 
Reported: 2013-11-07 11:15 PST by Timothy Hatcher
Modified: 2014-01-20 19:01 PST (History)
4 users (show)

See Also:


Attachments
Patch (64.06 KB, patch)
2013-11-07 11:23 PST, Timothy Hatcher
joepeck: review+
timothy: commit-queue-
Details | Formatted Diff | Diff
Screenshot (143.58 KB, image/png)
2013-11-07 12:05 PST, Timothy Hatcher
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2013-11-07 11:15:02 PST
Start fleshing out the TimelineSidebarPanel and TimelineContentView.
Comment 1 Radar WebKit Bug Importer 2013-11-07 11:15:23 PST
<rdar://problem/15416202>
Comment 2 Timothy Hatcher 2013-11-07 11:23:06 PST
Created attachment 216319 [details]
Patch
Comment 3 Timothy Hatcher 2013-11-07 12:05:56 PST
Created attachment 216324 [details]
Screenshot
Comment 4 Joseph Pecoraro 2013-11-07 16:08:41 PST
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.
Comment 5 Timothy Hatcher 2014-01-20 19:01:10 PST
https://trac.webkit.org/changeset/162403