Bug 30834 - Web Inspector: Initial revision of the Timeline grid.
Summary: Web Inspector: Initial revision of the Timeline grid.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-27 14:01 PDT by Pavel Feldman
Modified: 2009-10-28 03:00 PDT (History)
1 user (show)

See Also:

Attachments
[IMAGE] Present timeline looks (124.91 KB, image/png)
2009-10-27 14:02 PDT, Pavel Feldman 		no flags Details
[PATCH] The patch (27.68 KB, patch)
2009-10-27 14:03 PDT, Pavel Feldman 		timothy: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-10-27 14:01:48 PDT 
Initial revision with imagery.
Comment 1 Pavel Feldman 2009-10-27 14:02:49 PDT 
Created attachment 41980 [details]
[IMAGE] Present timeline looks
Comment 2 Pavel Feldman 2009-10-27 14:03:43 PDT 
Created attachment 41981 [details]
[PATCH] The patch
Comment 3 Timothy Hatcher 2009-10-27 14:24:27 PDT 
Comment on attachment 41981 [details]
[PATCH] The patch

> +    _toggleTimelineButtonClick: function()

Better named _toggleTimelineButtonClicked.


> +        iconElement.style.setProperty("background-position-y", this._record.iconNumber * 12 + "px");

You should make this based on a classname.
Comment 4 Timothy Hatcher 2009-10-27 14:31:57 PDT 
(In reply to comment #3)
> (From update of attachment 41981 [details])
> > +    _toggleTimelineButtonClick: function()
> 
> Better named _toggleTimelineButtonClicked.
> 
> 
> > +        iconElement.style.setProperty("background-position-y", this._record.iconNumber * 12 + "px");
> 
> You should make this based on a classname.

The classnames you make for the colored dots should just be loading, scripting, rendering, other. The category names.
Comment 5 Pavel Feldman 2009-10-28 02:59:57 PDT 
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 41981 [details] [details])
> > > +    _toggleTimelineButtonClick: function()
> > 
> > Better named _toggleTimelineButtonClicked.
> > 

Done.

> > 
> > > +        iconElement.style.setProperty("background-position-y", this._record.iconNumber * 12 + "px");
> > 
> > You should make this based on a classname.
> 
> The classnames you make for the colored dots should just be loading, scripting,
> rendering, other. The category names.

Done.
Comment 6 Pavel Feldman 2009-10-28 03:00:15 PDT 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/WebCore.gypi
	M	WebCore/inspector/front-end/AbstractTimelinePanel.js
	A	WebCore/inspector/front-end/Images/timelineBarBlue.png
	A	WebCore/inspector/front-end/Images/timelineBarGray.png
	A	WebCore/inspector/front-end/Images/timelineBarGreen.png
	A	WebCore/inspector/front-end/Images/timelineBarOrange.png
	A	WebCore/inspector/front-end/Images/timelineBarPurple.png
	A	WebCore/inspector/front-end/Images/timelineBarRed.png
	A	WebCore/inspector/front-end/Images/timelineBarYellow.png
	A	WebCore/inspector/front-end/Images/timelineCheckmarks.png
	A	WebCore/inspector/front-end/Images/timelineDots.png
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/TimelineAgent.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/inspector.css
Committed r50203