Bug 30776 - Web Inspector: Extract AbstractTimelinePanel that will be a base class for ResourcesPanel and TimelinePanel.
Summary: Web Inspector: Extract AbstractTimelinePanel that will be a base class for Re...
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-26 08:23 PDT by Pavel Feldman
Modified: 2009-10-26 14:36 PDT (History)
1 user (show)

See Also:


Attachments
[PATCH] Step 1 that might need follow up. (48.78 KB, patch)
2009-10-26 08:58 PDT, Pavel Feldman
timothy: review-
Details | Formatted Diff | Diff
[PATCH] Step 1 with comments addressed. (49.84 KB, patch)
2009-10-26 13:10 PDT, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-10-26 08:23:20 PDT
These panels will look very similar. This is going to be a multi-patch commit.
Comment 1 Pavel Feldman 2009-10-26 08:58:59 PDT
Created attachment 41868 [details]
[PATCH] Step 1 that might need follow up.
Comment 2 Timothy Hatcher 2009-10-26 09:50:27 PDT
Comment on attachment 41868 [details]
[PATCH] Step 1 that might need follow up.

There are a few mentions of "resources" in AbstractTimelinePanel, mostly in classes and node ids.

I think createTimelinePanels would be better named "createInterface", createTimelinePanels confused me since I thought it was creating the panels.

Will the scope bar live in AbstractTimelinePanel but just not be used by TimelinePanel?

The scope bar code needes merged with SVN, there have been some changes to support Command-Clicking.

Any "create" function that only gets called internal to one class should have a underscore prefix. (Like createStatusbarButtons.)

r- since it needs merged with SVN first to get scope bar changes.
Comment 3 Pavel Feldman 2009-10-26 13:05:11 PDT
(In reply to comment #2)
> (From update of attachment 41868 [details])
> There are a few mentions of "resources" in AbstractTimelinePanel, mostly in
> classes and node ids.
> 

I plan on renaming those, but want to do it in smaller steps.

> I think createTimelinePanels would be better named "createInterface",
> createTimelinePanels confused me since I thought it was creating the panels.
> 

Done.

> Will the scope bar live in AbstractTimelinePanel but just not be used by
> TimelinePanel?
> 

Whatever is there, it will probably look similar between Resources and Timeline, so I decided extracting just in case.

> The scope bar code needes merged with SVN, there have been some changes to
> support Command-Clicking.
> 

Done.

> Any "create" function that only gets called internal to one class should have a
> underscore prefix. (Like createStatusbarButtons.)
> 

Done.

> r- since it needs merged with SVN first to get scope bar changes.
Comment 4 Pavel Feldman 2009-10-26 13:10:56 PDT
Created attachment 41885 [details]
[PATCH] Step 1 with comments addressed.
Comment 5 Timothy Hatcher 2009-10-26 14:29:04 PDT
Comment on attachment 41885 [details]
[PATCH] Step 1 with comments addressed.

isCategoryVisible should probially be in the abstract class too and just return false and be something that needs subclasses to implement. Along with show/hideCategory.
Comment 6 Pavel Feldman 2009-10-26 14:36:38 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	A	WebCore/inspector/front-end/AbstractTimelinePanel.js
	M	WebCore/inspector/front-end/ResourceCategory.js
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/SummaryBar.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/inspector.js
Committed r50095