Summary: | Web Inspector: Reimplement TimelinePanel to make it scale | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||
Component: | Web Inspector (Deprecated) | Assignee: | Pavel Feldman <pfeldman> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bweinstein, joepeck, rik, timothy | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Pavel Feldman
2009-11-22 04:32:37 PST
Created attachment 43675 [details]
[PATCH] the patch.
Comment on attachment 43675 [details] [PATCH] the patch. > + var visibleTop = this._containerElement.scrollTop; > + var visibleBottom = visibleTop + this._containerElement.clientHeight; > + var rowHeight = 18; > + var startIndex = Math.max(0, Math.floor(visibleTop / rowHeight) - 1); > + var endIndex = Math.min(recordsInWindow.length, Math.ceil(visibleBottom / rowHeight)); Use const instead of var for these. > + var top = (startIndex * rowHeight) + "px"; Use const. > .sidebar { > position: absolute; > top: 0; > + min-height: 100%; > left: 0; > - bottom: 0; Why was this needed? > .sidebar-resizer-vertical { > position: absolute; > top: 0; > - bottom: 0; > + min-height: 100%; Ditto. > +.timeline-tree-section { Is this class uses in your patch? What is this for? > + color: rgb(92, 110, 129); > + font-family: Helvetica, Arial, sans-serif; Why is this Helvetica? We don't use Helvetica except in a few rare places. Otherwise the font is inherited from the body. > + position: relative; Why relative, so it have positioned children? > + text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px; Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision anyway. > + font-family: Helvetica, Arial, sans-serif; Why specify the font? > Use const instead of var for these. > Done. > > Use const. > Done. > > > .sidebar { > > position: absolute; > > top: 0; > > + min-height: 100%; > > left: 0; > > - bottom: 0; > > Why was this needed? > I shared an image on IRC. > > > +.timeline-tree-section { > > Is this class uses in your patch? What is this for? > Should not be here - from a previous version of patch. Removed. > Why is this Helvetica? We don't use Helvetica except in a few rare places. > Otherwise the font is inherited from the body. > Why relative, so it have positioned children? I was experimenting and forgot to delete. from a previous version of patch. Removed. > > + text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px; > > Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision > anyway. > That was a copypaste from inspector. Fixed. Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/inspector/timeline-test.js M WebCore/ChangeLog M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/inspector/front-end/AbstractTimelinePanel.js A WebCore/inspector/front-end/TimelineGrid.js A WebCore/inspector/front-end/TimelineOverviewPane.js M WebCore/inspector/front-end/TimelinePanel.js M WebCore/inspector/front-end/WebKit.qrc M WebCore/inspector/front-end/inspector.css M WebCore/inspector/front-end/inspector.html Committed r51296 |