[BUG] TableVirtuoso EmptyPlaceholder is rendered inside the table without proper wrapper elements

[mihkeleidast]

Describe the bug When setting EmptyPlaceholder to a custom component, it is rendered inside the <table> element. This is wrong for two reasons:

1. Semantically, I believe this is incorrect. One could wrap the contents of the actual placeholder inside <tbody><tr><td>, but it should not be part of the table. For example, when a screen reader user would go to read the table, they would be told that the table has a row of data, while actually it does not.
2. If you believe the above behavior is correct, i.e. that EmptyPlaceholder should render in Having to wrap the EmptyPlaceholder with multiple additional components seems overly complex to use. I would much rather have the placeholder be just another div below the table. Rendering with a div currently creates invalid HTML, which React tells to developers.

Reproduction https://codesandbox.io/s/sandpack-project-forked-yw9tr6

To Reproduce Steps to reproduce the behavior:

1. Go to the CodeSandbox
2. Open the internal browser console.
3. See DOM nesting validation error

Expected behavior

I would expect either:

1. That the EmptyPlaceholder would be rendered outside the <table> HTML element, removing the need to wrap it with multiple HTML elements, and making the DOM validation error disappear. This could however be considered sort of a breaking change, i.e. if someone is currently adding the additional table elements in EmptyPlaceholder, this would cause their HTML structure to break.
2. That TableVirtuoso would wrap EmptyPlaceholder in <tbody><tr><td> itself. This might be hard because I think TableVirtuoso does not know currently how many columns there are, so can't set colSpan correctly, but would otherwise make using the component easier.

Desktop (please complete the following information):

• OS: Windows 11.
• Browser: Edge 103

Additional context I can probably make some time to fix this, if we settle on a solution.

[petyosi]

Certainly food for thought here. If the table is empty, I would probably expect that its columns are still visible, and there's an "no data" message rendered within the table. That was my assumption when doing the placeholder the way it is now. Rendering a placeholder outside of the table will leave the table collapsed, which I feel to be suboptimal if it has some sort of a border. And yes, pre-wrapping the placeholder in tbody/tr/td is not possible because of unknown td counts.

[mihkeleidast]

What do you mean by "will leave the table collapsed"? Do you mean that the column widths are not correct, or something else? I think the column headers would be rendered anyway, even if there ain't any content.

[petyosi]

By collapsed, I mean a row with columns only. It feels broken - like content has not loaded or an error has happened.

[mihkeleidast]

That's at least partially because the table does not have fixed layout and the columns don't have explicit widths either, right? I think those are mostly mandatory anyway, to keep the table layout from shifting around when rows with different content lengths are added/removed by Virtuoso.

I update the sandbox to include those styles, this way the thead will keep the same layout not depending on data count: https://codesandbox.io/s/sandpack-project-forked-yw9tr6?file=/App.js

But just to reiterate, my main issue with the current solution is still that the placeholder renders a row in the body, which gives false info to assistive tech. Given that table structures are quite strict, I don't think there's any special role that row could be given to work around this.

[petyosi]

Assistive tech is a concern indeed. I do believe that the empty placeholder in its current implementation is not going to work for it. At the same time, I'm hesitant of changing it (due to breaking things). Is there an additional element or configuration we can introduce that works well, like table caption?

[mihkeleidast]

Yeah, maybe a caption would work, but that would be rendered above the Table, not below the thead. Another option would be to just connect the placeholder element to the table with aria-describedby for example. This could probably be done on the consumer side as well, no need to add new options.

I understand the reluctance to change the existing solution, but this would mean that the EmptyPlaceholder component should not be used going forward, right? I guess I am wondering what the benefit of keeping the existing approach would be besides not breaking things, if it is not the best solution considering accessibility and whatnot.

[mihkeleidast]

Hi again, some time has passed. Have there been any more thoughts on this - can we fix it in react-virtuoso, or should we go with a workaround where we don't use the EmptyPlaceholder of TableVirtuoso?

[petyosi]

With all your concerns being correct, I would not call the EmptyPlaceholder an evil so huge that it has to be removed as a config option. Accessibility aside, it can be useful from a visual perspective and is purely optional.

I would happily accept a doc for the site (or even a sandbox which I can adapt) as the correct way to display an empty table message so that the users of the library can benefit from it. If adding such markup is too hard, we can make that a prop.

[leminhthanh142]

i just get my hand dirty with react-virtuoso recently, this is my workaround to achieve custom EmptyPlaceHolder. My idea is to use a separate columns for columns control and use colspan to make it cover the whole row

// define columns
const columns = [
    {
      name: "Name",
      style: {
        width: 150,
        background: "yellow",
      },
      // other props
    },
    {
      name: "Description",
      style: {
        background: "yellow",
      },
      // other props
    },
  ];

// render custom empty placeholder
const renderEmptyPlaceholder = (props) => {
    console.log(props);
    return (
      <tbody>
        <tr>
          <td colSpan={columns.length} style={{ textAlign: "center" }}>
            Empty Placeholder
          </td>
        </tr>
      </tbody>
    );
  };

// pass it to table components
components={{
   EmptyPlaceholder: renderEmptyPlaceholder,
}}

https://codesandbox.io/p/sandbox/sandpack-project-forked-4vxh33?file=%2FApp.js%3A51%2C39