Bug 193186 – Web Inspector: Timelines: DOMContentLoaded and load event lines need to be more obvious

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

.

[Devin Rousso]

<rdar://problem/45100694>

[Devin Rousso]

Created attachment 358475 [details]
Patch

One thing I'm worried about is that the "load" event marker and the current time marker are both red, which may get confusing.  I couldn't find a better color though :/

[Devin Rousso]

Created attachment 358476 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 358475 [details]
Patch

Why put them behind record bars?
If we keep them in front can we just tweak the opacity? Instead of 0.25 maybe 0.75?

[Nikita Vasilyev]

Created attachment 358513 [details]
[Image] Proposed design

I think we should add markers at the top to make lines more obvious.

It's hard to discover what these lines are for. We show a title when hovering a line, but the line is only 1px wide! The markers should have a title text as well.

Besides this, I agree with Joe.

[Devin Rousso]

(In reply to Joseph Pecoraro from comment #4)
> Why put them behind record bars?
> If we keep them in front can we just tweak the opacity? Instead of 0.25 maybe 0.75?
Frankly, I find that the more important data is the record bars themselves, not the vertical marker lines.  By moving them "back", we can increase their color opacity and make them "pop" out more, while also not having to worry about them obstructing or "overshadowing" the actual record bars.

(In reply to Nikita Vasilyev from comment #5)
> I think we should add markers at the top to make lines more obvious.
I like it!  I'll add this as well.

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #5)
> It's hard to discover what these lines are for. We show a title when hovering a line, but the line is only 1px wide! The markers should have a title text as well.
One thing to note (this doesn't change anything; I'm just putting it out here for the record) is that the markers are actually 3px wide due to some CSS "trickery" :P

    .timeline-ruler > .markers > .marker::before {
        width: 3px;
        content: "";
        position: absolute;
        top: 0;
        bottom: 0;

        --timeline-ruler-marker-before-offset-start: -3px;
    }

    body[dir=ltr] .timeline-ruler > .markers > .marker::before {
        left: var(--timeline-ruler-marker-before-offset-start);
    }

    body[dir=rtl] .timeline-ruler > .markers > .marker::before {
        right: var(--timeline-ruler-marker-before-offset-start);
    }

[Devin Rousso]

Created attachment 358521 [details]
Patch

CSS Magic 😬

[Devin Rousso]

Created attachment 358522 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 358521 [details]
Patch

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

Not sure how I feel about the triangle. I do think it makes it easier to spot, but it think it looks almost like a handle that can be moved even though it can't.

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:143
> +    --timeline-ruler-marker-before-size: 5px;

On @2x it seems this should be 6px to balance out (2px on each side of the line). On @1x 5px seems to balance out.

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:163
> +    border-right: calc(var(--timeline-ruler-marker-after-size) / 2) solid transparent;
> +    border-left: calc(var(--timeline-ruler-marker-after-size) / 2) solid transparent;
> +    border-top: calc(var(--timeline-ruler-marker-after-size) / 2) solid currentColor;
> +
> +    --timeline-ruler-marker-after-size: 9px;

On @1x the triangle always ends with a 2px point so the 1px line doesn't line up never lines up with the triangle nicely.

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:198
>  .timeline-ruler > .markers > .marker.load-event {
> -    border-color: hsla(0, 100%, 50%, 0.25);
> +    color: hsl(0, 100%, 50%);
>  }
>  
>  .timeline-ruler > .markers > .marker.dom-content-event {
> -    border-color: hsla(240, 100%, 50%, 0.25);
> +    color: hsl(240, 100%, 50%);
>  }
>  
>  .timeline-ruler > .markers > .marker.timestamp {
> -    border-color: hsla(119, 100%, 21%, 0.25);
> +    color: hsl(119, 100%, 21%);
>  }

Do these colors need to be adjusted in dark mode?

[Devin Rousso]

Comment on attachment 358521 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:143
>> +    --timeline-ruler-marker-before-size: 5px;
> 
> On @2x it seems this should be 6px to balance out (2px on each side of the line). On @1x 5px seems to balance out.

Is this an existing issue before this change, or did some of my CSS variable "trickery" cause this to start happening?

>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:163
>> +    --timeline-ruler-marker-after-size: 9px;
> 
> On @1x the triangle always ends with a 2px point so the 1px line doesn't line up never lines up with the triangle nicely.

Ditto (>143).

>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:198
>>  }
> 
> Do these colors need to be adjusted in dark mode?

The old colors were almost invisible in Dark Mode, so this is definitely a step in the right direction.  The `.dom-content-event` and `.timestamp` colors could use some tweaking, but they're not awful.

[Joseph Pecoraro]

Comment on attachment 358521 [details]
Patch

r=me, but my earlier comments might warrant some small tweak.

Also, in the CPU and Memory timelines there is a legend in the top right showing a maximum value. Right now the markers show over that legend... maybe they should show behind it? What do you think.

[Devin Rousso]

Created attachment 360157 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 360157 [details]
Patch

Clearing flags on attachment: 360157

Committed r240506: <https://trac.webkit.org/changeset/240506>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.