WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
30776
Web Inspector: Extract AbstractTimelinePanel that will be a base class for ResourcesPanel and TimelinePanel.
https://bugs.webkit.org/show_bug.cgi?id=30776
Summary
Web Inspector: Extract AbstractTimelinePanel that will be a base class for Re...
Pavel Feldman
Reported
2009-10-26 08:23:20 PDT
These panels will look very similar. This is going to be a multi-patch commit.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Pavel Feldman
Comment 1
2009-10-26 08:58:59 PDT
Created
attachment 41868
[details]
[PATCH] Step 1 that might need follow up.
Timothy Hatcher
Comment 2
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.
Pavel Feldman
Comment 3
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.
Pavel Feldman
Comment 4
2009-10-26 13:10:56 PDT
Created
attachment 41885
[details]
[PATCH] Step 1 with comments addressed.
Timothy Hatcher
Comment 5
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.
Pavel Feldman
Comment 6
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
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug