Bug 214958 – Web Inspector: Hide clipped timeline markers

• Component: Web Inspector
• Status: NEW

[Nikita Vasilyev]

Created attachment 405548 [details]
[Image] Clipped markers

When opening empty Timelines tab, the red round markers are clipped. We should simply hide them.

[Nikita Vasilyev]

Created attachment 405549 [details]
Patch

[Nikita Vasilyev]

Created attachment 405550 [details]
[Image] With patch applied

This patch also shows "0ms" label that was previously clipped.

[Nikita Vasilyev]

Created attachment 405551 [details]
[Video] With patch applied

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

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:93
> +.data-grid th:not(.sortable) > .timeline-ruler > .header > .divider > .label {
> +    background-color: var(--background-color);
> +}

This video shows better why I added this rule. Without it, the border would go directly through "1000.0ms".

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #2)
> Created attachment 405550 [details]
> [Image] With patch applied
> 
> This patch also shows "0ms" label that was previously clipped.
Personally, I don't think we should show "0ms".  It's kinda obvious, and doesn't add much.

I'm on the fence about allowing the header to clip, as I don't think it looks very good.  Not to mention, a clipped header can be inferred from the distance between the rest of the timeline markers.

Also, I'm seeing intermittent bugs with this where sometimes the current time marker (red circle and line) appears waaay outside of the graph area.  It usually happens after selecting a range shortly after the start and then double-clicking to remove that selection.  We definitely don't want that :(

[Devin Rousso]

Comment on attachment 405549 [details]
Patch

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

r- due to the issue above

> Source/WebInspectorUI/UserInterface/Views/OverviewTimelineView.css:30
> +    overflow: initial;

Does this actually do anything?

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:135
> +.timeline-ruler > .markers > .marker.before-selection {
> +    display: none;
> +}

Why is this needed?

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:639
> +            element.classList.toggle("before-selection", newPositionAprox <= 0);

Shouldn't it only be `< 0`?  I think we probably should show the marker if it's exactly at `0`.

[Nikita Vasilyev]

It looks like we disagree on a few things. Let's find common ground. Do you agree that the clipped red round markers look bad (when opening Timelines without any recordings, for example)?

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #6)
> It looks like we disagree on a few things. Let's find common ground. Do you agree that the clipped red round markers look bad (when opening Timelines without any recordings, for example)?

I think the clipped markers only look bad if there's no border (i.e. <https://webkit.org/b/214563>).  I have no problem with them when there is a border, as that border is a clear indicator of "the content ends here".

[Nikita Vasilyev]

I see. I'll add a border in <https://webkit.org/b/214563> then. I don't feel strongly about it.

I think clipped markers look bad, period. I don't think we should show half of the red circle (a quarter of the red circle in the overview). I think this is a poor visual design. I think, when the marker line isn't visible, the marker "head" (e.g. red circle) should be hidden completely.

[Nikita Vasilyev]

Comment on attachment 405549 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:639
>> +            element.classList.toggle("before-selection", newPositionAprox <= 0);
> 
> Shouldn't it only be `< 0`?  I think we probably should show the marker if it's exactly at `0`.

An empty timeline has a red marker at 0. The main point of this patch was to hide it.

[Radar WebKit Bug Importer]

<rdar://problem/66610846>

[Ahmad Saleem]

Created attachment 469296 [details]
Reference Screenshot - Safari 17.2.1

It seems to be still reproducible.