Bug 150612

Summary: Web Inspector: Show Timeline Markers again (DOMContentLoaded, Load, console.timeStamp)
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
timothy: review+, bburg: commit-queue-
[IMAGE] Markers
none
[PATCH] For Landing none

Description Joseph Pecoraro 2015-10-27 16:33:59 PDT
* SUMMARY
Show Timeline Markers again (DOMContentLoaded, Load, console.timeStamp).

The markers were being received on the frontend, but nothing was happening with them. Show them again and update their styles.
Comment 1 Radar WebKit Bug Importer 2015-10-27 16:34:51 PDT
<rdar://problem/23286592>
Comment 2 Joseph Pecoraro 2015-10-27 16:38:43 PDT
Created attachment 264174 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2015-10-27 16:39:01 PDT
Created attachment 264175 [details]
[IMAGE] Markers
Comment 4 Matt Baker 2015-10-27 17:22:17 PDT
Comment on attachment 264174 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:147
> +    opacity: 0.5;

I like the solid markers, but they visually compete with the record bars somewhat. Maybe an opacity of 0.2 would be better.

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:152
> +    opacity: 0.5;

See above.

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:157
> +    opacity: 0.5;

See above.
Comment 5 Timothy Hatcher 2015-10-27 17:34:51 PDT
Comment on attachment 264174 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:147
>> +    opacity: 0.5;
> 
> I like the solid markers, but they visually compete with the record bars somewhat. Maybe an opacity of 0.2 would be better.

I agree, try 0.25.
Comment 6 Matt Baker 2015-10-27 17:38:23 PDT
Comment on attachment 264174 [details]
[PATCH] Proposed Fix

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

>>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:147
>>> +    opacity: 0.5;
>> 
>> I like the solid markers, but they visually compete with the record bars somewhat. Maybe an opacity of 0.2 would be better.
> 
> I agree, try 0.25.

Just applied the patch and 0.2 was too faint. 0.3 looked good. I also noticed the markers are above the timeline overview graph record bars in the z-order, but they are positioned behind record bars in the content view below. We want the current time marker to always be on top, but if we could resolve the z-index issue that would help visually.
Comment 7 Matt Baker 2015-10-27 17:39:24 PDT
(In reply to comment #6)
> Comment on attachment 264174 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264174&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:147
> >>> +    opacity: 0.5;
> >> 
> >> I like the solid markers, but they visually compete with the record bars somewhat. Maybe an opacity of 0.2 would be better.
> > 
> > I agree, try 0.25.
> 
> Just applied the patch and 0.2 was too faint. 0.3 looked good. I also
> noticed the markers are above the timeline overview graph record bars in the
> z-order, but they are positioned behind record bars in the content view
> below. We want the current time marker to always be on top, but if we could
> resolve the z-index issue that would help visually.

If that wasn't clear, I mean that the markers are less distracting in the bottom view, where they are behind the record bars.
Comment 8 Timothy Hatcher 2015-10-27 17:57:34 PDT
Yeah I think behind the record bars would be good too.
Comment 9 BJ Burg 2015-10-28 10:07:33 PDT
Comment on attachment 264174 [details]
[PATCH] Proposed Fix

Clearing cq? since it needs some tweaks per discussion above.
Comment 10 Joseph Pecoraro 2015-10-28 16:20:41 PDT
(In reply to comment #8)
> Yeah I think behind the record bars would be good too.

I was able to do this on the bottom, but not the top due to how timeline-ruler selection works. We should revisit this when we start making timeline bubbles selectable.
Comment 11 Joseph Pecoraro 2015-10-28 16:21:12 PDT
Created attachment 264264 [details]
[PATCH] For Landing
Comment 12 Joseph Pecoraro 2015-10-28 16:26:39 PDT
http://trac.webkit.org/changeset/191703