Web Inspector: Data grid has a double left border when the first column is hidden

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 285701 [details]
[Image] Bug

Steps:
1. Open Network tab.
2. Right click on the table header and click on "Domain" to hide the first column.

Actual:
The next column ("Type" by default) has a border to its left.

Expected:
The next column ("Type" by default) has no border to its left.

[Radar WebKit Bug Importer]

<rdar://problem/27778081>

[Nikita Vasilyev]

Created attachment 285703 [details]
Patch

cq- because I need more time to make sure it didn't break anything.

[Timothy Hatcher]

Comment on attachment 285703 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DataGrid.css:93
> -    border-right: 0.5px solid var(--border-color);
> +    background-image: linear-gradient(to right, transparent calc(100% - 0.5px), var(--border-color) 0.5px);

Is there a class we can use to set the border with to 0 when the column is hidden?

We would need to be careful with using background since this could conflict with other potential styling in the future. I also worry about performance aspects of using gradients like this — that is a lot of individual gradients.

[Nikita Vasilyev]

(In reply to comment #3)
> Comment on attachment 285703 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285703&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DataGrid.css:93
> > -    border-right: 0.5px solid var(--border-color);
> > +    background-image: linear-gradient(to right, transparent calc(100% - 0.5px), var(--border-color) 0.5px);
> 
> Is there a class we can use to set the border with to 0 when the column is
> hidden?

We would have to add this class to every <td> in the column.

Currently, we change the width of a <col> element. We don't modify <td>'s directly.

<table class="data">
  <colgroup>
    <col style="width: 0%;">
    ...
  </colgroup>
  <tbody>
    <tr>
      <td>...</td>
    </tr>
    ...
  </tbody>
</table>
  

> 
> We would need to be careful with using background since this could conflict
> with other potential styling in the future. I also worry about performance
> aspects of using gradients like this — that is a lot of individual gradients.

Agreed.

[Timothy Hatcher]

Comment on attachment 285703 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/DataGrid.css:93
>>> +    background-image: linear-gradient(to right, transparent calc(100% - 0.5px), var(--border-color) 0.5px);
>> 
>> Is there a class we can use to set the border with to 0 when the column is hidden?
>> 
>> We would need to be careful with using background since this could conflict with other potential styling in the future. I also worry about performance aspects of using gradients like this — that is a lot of individual gradients.
> 
> We would have to add this class to every <td> in the column.
> 
> Currently, we change the width of a <col> element. We don't modify <td>'s directly.
> 
> <table class="data">
>   <colgroup>
>     <col style="width: 0%;">
>     ...
>   </colgroup>
>   <tbody>
>     <tr>
>       <td>...</td>
>     </tr>
>     ...
>   </tbody>
> </table>

We might want to just remove the hidden table cells and columns from the DOM. We can try this fix assuming it does not have any regressions found in testing.

[Matt Baker]

It's too bad col { visibility: collapse; } isn't supported yet.

[Nikita Vasilyev]

Created attachment 285797 [details]
WIP

This WIP patch removes hidden columns from the DOM and adds them back when they are turned on. In general, it's good for performance to minimize number of unnecessary nodes in the DOM. However, this patch is way more complex than the previous one.

[Nikita Vasilyev]

(In reply to comment #6)
> It's too bad col { visibility: collapse; } isn't supported yet.

Alas.

> [1] Chrome and Safari treat visibility: collapse like hidden, leaving a white gap; they support it only on <tr>, <thead>, <tbody>, and <tfoot>, but not on <col> and <colgroup> elements.

https://developer.mozilla.org/en-US/docs/Web/CSS/visibility

[Nikita Vasilyev]

(In reply to comment #7)
> Created attachment 285797 [details]
> WIP
> 
> This WIP patch removes hidden columns from the DOM and adds them back when
> they are turned on. In general, it's good for performance to minimize number
> of unnecessary nodes in the DOM. However, this patch is way more complex
> than the previous one.

Another way of solving this bug would be by adding borders inside the resizers!

Resizers are optional, some data grids don't have them. For these cases I would still need to add them to the DOM and make them click-through.

I'm not going to go with borders inside the resizers because:
1. I'm almost done with this patch.
2. It's good for performance to minimize number of unnecessary nodes in the DOM.

[Nikita Vasilyev]

Created attachment 286092 [details]
Patch

Another CSS hack.

Tim and I agreed that removing <td>'s from DOM is a more complicated than a pure CSS solution and it doesn't worth spending my time working on this right now.

[Nikita Vasilyev]

Created attachment 286190 [details]
Patch

Rebaselined.

[WebKit Commit Bot]

Comment on attachment 286190 [details]
Patch

Clearing flags on attachment: 286190

Committed r204518: <http://trac.webkit.org/changeset/204518>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.