Bug 152954 – Web Inspector: console.table background stripes are misaligned

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

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.

[Nikita Vasilyev]

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.

[Nikita Vasilyev]

Created attachment 268640 [details]
Patch

[Nikita Vasilyev]

Comment on attachment 268640 [details]
Patch

This broke Timelines DataGrid.

[Nikita Vasilyev]

(In reply to comment #0)
> Created attachment 268638 [details]
> [Image] Bug

Should the text be clipped on a single line?
Any opinions?

[Matt Baker]

(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.

[Matt Baker]

(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.

[Timothy Hatcher]

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.

[Nikita Vasilyev]

(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.

[Matt Baker]

(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;
}

[Matt Baker]

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;
}

[Radar WebKit Bug Importer]

<rdar://problem/24197735>

[Nikita Vasilyev]

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.

[Nikita Vasilyev]

Created attachment 270723 [details]
[Image] With the patch applied

[Timothy Hatcher]

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.

[Nikita Vasilyev]

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."

[Timothy Hatcher]

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.

[WebKit Commit Bot]

Comment on attachment 270722 [details]
Patch

Clearing flags on attachment: 270722

Committed r196166: <http://trac.webkit.org/changeset/196166>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.