.
<rdar://problem/45100694>
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 :/
Created attachment 358476 [details] [Image] After Patch is applied
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?
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.
(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.
(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); }
Created attachment 358521 [details] Patch CSS Magic 😬
Created attachment 358522 [details] [Image] After Patch is applied
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?
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.
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.
Created attachment 360157 [details] Patch
Comment on attachment 360157 [details] Patch Clearing flags on attachment: 360157 Committed r240506: <https://trac.webkit.org/changeset/240506>
All reviewed patches have been landed. Closing bug.