Created attachment 268638 [details] [Image] Bug Image attached. Steps: console.table([ {x: 1, msg: "A long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-long-string"}, {x: 2, msg: "One line"} ]) Expected: The first table raw has white background. Actual: It has striped background.
The striped background is made by CSS gradient: .data-grid table.data { background-image: linear-gradient(to bottom, white, white 50%, hsl(214, 41%, 96%) 50%, hsl(214, 41%, 96%)); } https://github.com/WebKit/webkit/blob/master/Source/WebInspectorUI/UserInterface/Views/DataGrid.css#L127 It should really use tr:nth-child(even) CSS pseudo selector.
Created attachment 268640 [details] Patch
Comment on attachment 268640 [details] Patch This broke Timelines DataGrid.
(In reply to comment #0) > Created attachment 268638 [details] > [Image] Bug Should the text be clipped on a single line? Any opinions?
(In reply to comment #4) > (In reply to comment #0) > > Created attachment 268638 [details] > > [Image] Bug > > Should the text be clipped on a single line? > Any opinions? Have you tried adding a background-size to the style based on (twice) the row height? Something like this might fix the misaligned strips (assuming row height of 21px: .data-grid table.data { background-image: linear-gradient(to bottom, white, white 50%, hsl(214, 41%, 96%) 50%, hsl(214, 41%, 96%)); background-size: 100% 42px; } If that doesn't work, you could clip it and provide a popover or tooltip with the complete text.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #0) > > > Created attachment 268638 [details] > > > [Image] Bug > > > > Should the text be clipped on a single line? > > Any opinions? > > Have you tried adding a background-size to the style based on (twice) the > row height? Something like this might fix the misaligned strips (assuming > row height of 21px: > > > .data-grid table.data { > background-image: linear-gradient(to bottom, white, white 50%, hsl(214, > 41%, 96%) 50%, hsl(214, 41%, 96%)); > background-size: 100% 42px; > } Never mind, that would only align the stripes with each line of text.
How did Timelines break? I assume it broke because it no longer had the stripes all the way down when there were not enough rows. You might just need to add the gradient background back to the tr.filler row.
(In reply to comment #7) > How did Timelines break? > > I assume it broke because it no longer had the stripes all the way down when > there were not enough rows. Exactly. > > You might just need to add the gradient background back to the tr.filler row. I would also need to take into account if the row before the tr.filler was odd or even, otherwise these two rows would have the same background. I think it's simpler to keep the gradient background on the table for the timelines and use tr:nth-child(even) for the console.
(In reply to comment #8) > (In reply to comment #7) > > How did Timelines break? > > > > I assume it broke because it no longer had the stripes all the way down when > > there were not enough rows. > > Exactly. > > > > > You might just need to add the gradient background back to the tr.filler row. > > I would also need to take into account if the row > before the tr.filler was odd or even, otherwise these two rows would have > the same background. > > I think it's simpler to keep the gradient background on the table for the > timelines > and use tr:nth-child(even) for the console. AFAIK, the only use case for the gradient background is for tables that fill the entire view, to prevent having a blank background for empty or almost empty tables. Maybe we could add a grid property to encapsulate the choice between these two styles: class DataGrid { set alwaysShowRowBackground(x) { this._dataTableElement.classList.toggle("always-show-row-background", x); } }; .data-grid table.data tr:nth-child(even) { background-color: hsl(214, 41%, 96%); } .data-grid table.data.always-show-row-background { background-image: linear-gradient(to bottom, white, white 50%, hsl(214, 41%, 96%) 50%, hsl(214, 41%, 96%)); background-size: 100% 42px; } .data-grid table.data.always-show-row-background tr { background-color: transparent; }
Cleaner: .data-grid table.data:not(.always-show-row-background) tr:nth-child(even) { background-color: hsl(214, 41%, 96%); } .data-grid table.data.always-show-row-background { background-image: linear-gradient(to bottom, white, white 50%, hsl(214, 41%, 96%) 50%, hsl(214, 41%, 96%)); background-size: 100% 42px; }
<rdar://problem/24197735>
Created attachment 270722 [details] Patch (In reply to comment #10) > Cleaner: > > .data-grid table.data:not(.always-show-row-background) tr:nth-child(even) { > background-color: hsl(214, 41%, 96%); > } > > .data-grid table.data.always-show-row-background { > background-image: linear-gradient(to bottom, white, white 50%, hsl(214, > 41%, 96%) 50%, hsl(214, 41%, 96%)); > background-size: 100% 42px; > } .data-grid is used in 21 CSS files. I'll just fix the console for now, keeping the rest unaffected.
Created attachment 270723 [details] [Image] With the patch applied
Comment on attachment 270722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270722&action=review > Source/WebInspectorUI/ChangeLog:13 > + Replace CSS gradient that produces fixed height stripes with > + a rule that sets background only on even table rows. Comment not correct anymore.
Comment on attachment 270722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270722&action=review >> Source/WebInspectorUI/ChangeLog:13 >> + a rule that sets background only on even table rows. > > Comment not correct anymore. Is it? Would the following be better? "Overwrite background-image to undo fixed height stripes and set background only on even table rows."
Comment on attachment 270722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270722&action=review >>> Source/WebInspectorUI/ChangeLog:13 >>> + a rule that sets background only on even table rows. >> >> Comment not correct anymore. > > Is it? Would the following be better? > > "Overwrite background-image to undo fixed height stripes and > set background only on even table rows." I see, replace means override, not delete.
Comment on attachment 270722 [details] Patch Clearing flags on attachment: 270722 Committed r196166: <http://trac.webkit.org/changeset/196166>
All reviewed patches have been landed. Closing bug.