Bug 127530 - Web Inspector: Refactor bar combining logic into a TimelineRecordBar helper
Summary: Web Inspector: Refactor bar combining logic into a TimelineRecordBar helper
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-01-23 18:10 PST by Timothy Hatcher
Modified: 2014-01-23 19:37 PST (History)
4 users (show)

See Also:


Attachments
Patch (38.34 KB, patch)
2014-01-23 18:22 PST, Timothy Hatcher
joepeck: review+
timothy: commit-queue-
Details | Formatted Diff | Diff
Screenshot (404.91 KB, image/png)
2014-01-23 18:23 PST, Timothy Hatcher
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2014-01-23 18:10:19 PST
This will let NetworkTimelineOverviewGraph use TimelineRecordBar.
Comment 1 Radar WebKit Bug Importer 2014-01-23 18:11:07 PST
<rdar://problem/15898755>
Comment 2 Timothy Hatcher 2014-01-23 18:22:22 PST
Created attachment 222063 [details]
Patch
Comment 3 Timothy Hatcher 2014-01-23 18:23:36 PST
Created attachment 222065 [details]
Screenshot
Comment 4 Joseph Pecoraro 2014-01-23 19:09:39 PST
Comment on attachment 222063 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222063&action=review

Clean! r=me

> Source/WebInspectorUI/ChangeLog:49
> +        Lazy create DOM elements. Support rendering one or both segments. Doing this lets

Typo: Lazy => Lazily

> Source/WebInspectorUI/ChangeLog:50
> +        combined inactive segments to sit behind multiple active segments.

Typo: "combined inactive segments to sit" => "combined inactive segments sit"

> Source/WebInspectorUI/UserInterface/NetworkTimelineOverviewGraph.css:27
> +    padding-top: 3px

Nit: Missing semicolon.

> Source/WebInspectorUI/UserInterface/TimelineRecordBar.js:238
> +        var barEndTime = this._records.reduce(function(previousValue, currentValue) { return Math.max(previousValue, currentValue.endTime); }, 0);

The list is no longer sorted?

Nit: Up above, out of context, there is a "return;" that should be "return false;"
Comment 5 Timothy Hatcher 2014-01-23 19:37:32 PST
https://trac.webkit.org/r162681