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-
[PATCH] Step 1 with comments addressed. (49.84 KB, patch)
2009-10-26 13:10 PDT, Pavel Feldman
timothy: review+
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.