Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vscode-data-grid with grid-template-columns is not respected #473

Open
worksofliam opened this issue Apr 20, 2023 · 12 comments · May be fixed by #517
Open

vscode-data-grid with grid-template-columns is not respected #473

worksofliam opened this issue Apr 20, 2023 · 12 comments · May be fixed by #517
Assignees
Labels
bug Something isn't working upstream issue A blocked issue that is caused by an upstream issue in FAST

Comments

@worksofliam
Copy link

Version defined in package.json:

Describe the bug

vscode-data-grid with grid-template-columns is not respected(?)

I am trying to build a data grid where it can fit more than 4 columns in one view.

The data set gets in fine, but there is just so much white space in columns and I assumed grid-template-columns might be able to solve this.

image

But as you can see, it's generating code with the same attribute, but full of 1fr.

image

To reproduce

<vscode-data-grid id="x" grid-template-columns="min-content"></vscode-data-grid>
x.columnDefinitions = [...]
x.rowsData = [...];
  • OS Version: Mac 12.6
  • Toolkit Version: ^1.0.0
@worksofliam worksofliam added the bug Something isn't working label Apr 20, 2023
@worksofliam
Copy link
Author

Updated to 1.2.2 and still getting the same issue.

image

@worksofliam
Copy link
Author

Figured out I need to apply a value for each column in that property. Silly me.

@worksofliam
Copy link
Author

Perhaps a different issue, but I will reuse this in case it's related.

I apply max-content to the grid like so:

let result = {
    html: `<vscode-data-grid id="${id}" grid-template-columns="${columns.map(c => `max-content`).join(` `)}"></vscode-data-grid>`
};

But now when I use this, my column don't align at all D:

image

I tested this in CodeSandbox provided in the readme and I change this example to use max-width and it turned out ok.

image

<vscode-data-grid
        id="custom-column-grid"
        grid-template-columns="max-content max-content max-content max-content"
        aria-label="With Custom Column Widths"
></vscode-data-grid>

I can't see what I am doing wrong, would love any ideas

@worksofliam
Copy link
Author

Further digging: I created my issue in a sandbox and it's led me to this finding.

  • grid-template-columns does not work at all with min-width: max-content;
  • I am using min-width: max-content; because I want my users to be able to see many (maybe like 20+) columns. This css prop allows them to do that
  • Not using grid-template-columns solves the problem of the out of line columns, but it still has all that whitespace in most columns.

@hawkticehurst
Copy link
Member

Hey @worksofliam! Just chiming in to say I'm a bit swamped for the next 2 weeks so it might take a second for me to circle back this, but I have definitely seen this and will get back to you as soon as I can :)

p.s. thanks for all the thorough notes, super helpful to follow!!

@hawkticehurst
Copy link
Member

hawkticehurst commented May 15, 2023

Hey! Finally found a touch of time to do some initial snooping and it looks like this may be an issue we have to bubble up to FAST. Specifically, I can at least speak to your last bullet point:

Not using grid-template-columns solves the problem of the out of line columns, but it still has all that whitespace in most columns.

It looks like the FAST foundation data grid component will automatically assign grid-template-columns for each row with a value of 1fr (repeated for each column) if the attribute is not manually defined.

This should also hopefully give insight into the original issue you were seeing –– when you remove grid-template-columns and saw all those 1fr values you were seeing intentional behavior of the FAST data grid component. If I recall correctly, this behavior might actually be my fault/suggestion (😅) because very very early on during the private preview of the toolkit there were a bunch of issues showing up where people didn't really understand CSS grid and would just see a misshapen grid as the default. Applying 1fr to each column/row is an easy way to make the data grid "look (mostly) good/correct".

With that said, I'm still trying to figure out why other valid grid-template-column values are not being respected/rendered (i.e. max-content), but thought I'd toss this initial finding here before I inevitably get pulled away to do some other work 😅

@sklinov
Copy link

sklinov commented May 24, 2023

@hawkticehurst I think I'm experiencing the same issue
I was trying to build a table with values of different width with column width matching max-content.
But it goes row by row instead of looking like a conventional table.
image

Here's the sandbox with reproduction
https://codesandbox.io/s/vscode-data-grid-column-width-khn4fj?file=/index.html

@sklinov
Copy link

sklinov commented Jun 8, 2023

BTW, a quick question, @hawkticehurst - is that intentional, that top-level <VSCodeDataGrid> has display: flex; flex-direction: column?
I think that may the cause why the grid doesn't want to look like a conventional table

@sklinov
Copy link

sklinov commented Jun 8, 2023

Overall, I ended up with grid divs removing VSCodeDataGridRow because it ruins the structure of grid.
IMHO, it looks redundant, and the same results could be achieved by setting required grid-template-columns to parent element and passing an array of children.
Here's an example https://codesandbox.io/s/zealous-murdock-c8jfm5?file=/src/App.tsx

@worksofliam
Copy link
Author

@hawkticehurst

If I recall correctly, this behavior might actually be my fault/suggestion (😅) because very very early on during the private preview of the toolkit there were a bunch of issues showing up where people didn't really understand CSS grid

Thanks for looking into this. I let your comment slip! So really what is the suggested fix? Do you think waiting until July for a new FAST version will solve this issue?

@joacoc
Copy link

joacoc commented Nov 5, 2023

Setting the vscode-data-grid display to grid solved my issue:

const table = document.createElement("vscode-data-grid");
...
table.style.display = "grid";

@hawkticehurst
Copy link
Member

hawkticehurst commented Nov 11, 2023

Hello!

Back again with more updates and I thinkkk I finally understand what's going on here. Sad news, I'm like 98% sure what you're hoping to achieve is impossible given how the FAST data grid has been architected and built 🥲. Less sad news, there seems to be a theoretical workaround but I'm not sure if it will in fact work and what the consequences of that are yet 😅

The gist of the problem is that the vscode-data-grid-row component really should not exist. In fact, @sklinov is spot on with this comment back in June:

Overall, I ended up ... removing VSCodeDataGridRow because it ruins the structure of [the] grid.

The problem

Because the data grid component is split into distinctly separate sub-components (i.e. vscode-data-grid, vscode-data-grid-row, and vscode-data-grid-cell) and those components are web components that use Shadow DOM it means each component can be thought of as having a strong boundary where parent component style changes are not easily passed down to child/grandchildren/etc web components –– aka the normal rules of using web components + shadow dom.

This becomes a problem when using vscode-data-grid-row components because they themselves are styled as CSS grid containers containing cell components –– the effect is vscode-data-grid is just a simple container wrapping a bunch of vscode-data-grid-rows that are all distinctly separate grid containers. As a result, there is no way (that I'm aware of) to align columns across multiple CSS grids stacked on top of each other.

This is also why using a simple display: flex and flex-direction: column inside of vscode-data-grid has (for the most part) worked up to this point –– it was just vertically stacking a bunch of CSS grid containers.

Image

Also, @joacoc I'm curious what your specific issue was because changing vscode-data-grid to use display: grid had no change in trying to resolve @worksofliam's and @sklinov's provided/described issues.

A solution (?)

With all that said, a casual workaround solution is, in fact, to change vscode-data-grid to use display: grid and then literally ignore/not use the vscode-data-grid-row component at all. Here's some theoretical example code:

<!-- 
  Note: Third and fourth columns are not included for brevity, 
  but they repeat the same pattern as columns one/two 
-->
<vscode-data-grid 
  generate-header="none" 
  style="grid-template-columns: repeat(4, max-content);"
>
  <vscode-data-grid-cell grid-column="1" cell-type="columnheader">
    A Custom Header Title
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="2" cell-type="columnheader">
    Another Custom Title
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="1">
    Cell Data Boop
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="2">
    Cell Data Yummy cool stuff
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="1">
    Cell Data Wee waa
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="2">
    Cell Data
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="1">
    Cell Data Deloop vee
  </vscode-data-grid-cell>
  <vscode-data-grid-cell grid-column="2">
    Cell Data
  </vscode-data-grid-cell>
</vscode-data-grid>

In doing this, you will actually be able to achieve a proper table-like data-grid component where columns are aligned, and applying grid-template-columns: repeat(4, max-content); will work*.

*You can try out this code now, but the only remaining problem seems to be the issue where FAST data grids will automatically assign grid-template-columns for each row with a value of 1fr that I mentioned back in May. This means style="grid-template-columns: repeat(4, max-content);" is not being correctly applied when a webview is initially rendered.

The only way I've been able to confirm that it works is by opening the developer tools and toggling the style declaration for grid-template-columns on and off.

Toggle on/off:

Image
Image

Before:

Image

After:

Image

The real solution

The real/proper solution is to bring this to FAST as an upstream issue or (in all honesty) think about just building a brand new vscode-data-grid component from scratch that caters to the specific needs of the community here. But since I know both teams are extremely resource-constrained and likely wouldn't be able to tackle this type of issue for a very long time I'm not actually sure where that leaves us...

I'm actually keen to have a discussion about this and see if any of you in the community have any thoughts/suggestions/preferences for how to move forward. I'm all ears to any thoughts :)

@hawkticehurst hawkticehurst added the upstream issue A blocked issue that is caused by an upstream issue in FAST label Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream issue A blocked issue that is caused by an upstream issue in FAST
Projects
None yet
4 participants