Created attachment 405548 [details] [Image] Clipped markers When opening empty Timelines tab, the red round markers are clipped. We should simply hide them.
Created attachment 405549 [details] Patch
Created attachment 405550 [details] [Image] With patch applied This patch also shows "0ms" label that was previously clipped.
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".
(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 :(
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`.
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)?
(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".
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.
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.
<rdar://problem/66610846>
Created attachment 469296 [details] Reference Screenshot - Safari 17.2.1 It seems to be still reproducible.