[BUG] Table visualisations sometimes show results in a small scrollable portion of the screen

[jgough]

Describe the bug

When viewing many results in a table visualisation, these are only shown in a small area of the screen

To Reproduce
Steps to reproduce the behavior:

1. Open any table visualisation split by rows with a default size of 5 -or- Go to this Sandbox Page

2. Inside the Split rows bucket, change the Size to 100

3. Click on Update. More rows should be shown but it should be paginated.

4. Click on Options in the side panel

5. Change Max rows per page to 100

6. Note that only 10 rows are shown, but the data table is actually scrollable with the mouse wheel to view 100 results within the small window:

Expected behavior
If you perform the steps above and copy the current URL and paste into a new tab you get the expected behaviour which is a table showing more than 10 rows:

OpenSearch Version
2.6.0

Dashboards Version
2.6.0

Additional context
Reproduced in Edge 111.0.1661.54 and Firefox 102.9.0esr (64-bit)

[joshuarrrr]

Interesting - I'm actually having a little difficulty reproducing that exact behavior, but there's definitely a bug here with the viz sizing and pagination. Thanks for reporting - we'll take a look.

[joshuarrrr]

@ananzh I tagged this as 2.7 - I'm not sure if that will be possible, but that's probably what we should aim for.

[jgough]

@joshuarrrr I have updated the repro instructions above, I think I had the steps the wrong way around, apologies.

[curq]

I have researched this bug, and was able to reproduce it.

The problem seems to lie within EuiDataGrid component, specifically EuiDataGridBody. It seems like for some reason ResizeObserver is not working/reacting on initial load. When the number of row changes the value remains the same as in the screenshot, 375 is a default height.

To test it I have opened the same page in the new tab and here the ResizeObserver does work, and height changes after every update of number of rows. Below is the screenshot from the new tab. I also noticed that the state that I assume responsible for the height is 0 in this case.

I'm currently not sure how to solve but as I am not familiar with the eui, I will try to look into this more.

[ananzh]

@curq @jgough thanks for opening the issue and provide some initial thoughts. Kudos to @curq's finding and yes euiDataGrid__virtualized class in the EuiDataGrid is used to create a virtualized data grid that can efficiently handle large amounts of data. By default, this euiDataGrid__virtualized class will adjust its height to fit the visible rows, but it won't take into account the total number of rows in the grid. Therefore, if we want the height of the table to adjust based on the total number of rows in the grid, we need to use CSS to adjust. So I think we only need to modify the scss file (file path is src/plugins/vis_type_table/public/components/table_vis_app.scss). Once we set some overflow property of the container element to enable scrollbars when the content overflows the container, then this will also fix #3756. We could also write some tests for it.

@curq @jgough do either one of you want to take this issue and #3756?

[curq]

@ananzh Could you please assign these issues to me? I'd like to try solving them.

[ananzh]

Sweet! @curq I have assigned both issues to you.

[curq]

@ananzh After further investigation, I have discovered that the root cause of the two issues is not the same. Regarding this issue, It looks like the state is being updated incorrectly or not being updated when needed. Additionally, I noticed that changing the order of rows in the options sidebar (asc/desc) does not work, and the height is sometimes not adjusted properly. I think we can also see that in the screenshot made by @jgough

There is a weird behavior that state that is supposed to store value of height is never updated and the dynamic height adjustment works only when it is zero.

If we change the value of state [26] to 0, it fixes the layout and dynamic height adjusting works.

When refreshing or opening the same page in a new tab it initializes this state as 0, so height changes correctly. I'm not sure, but it looks like this is the reason why bug appears only on initial load.

Do you have any ideas or suggestions on how to resolve this issue?

[ananzh]

Yes, you are right. They are different root causes.

#3756 is caused by the table container's layout and overflow behavior. As I explained earlier, this can be fixed by modifying the CSS for the table container.

This issue is related to the EuiDataGrid component's handling of large amounts of data and pagination. The root cause lies within the resize not working/reacting on the initial load, resulting in the table displaying only a small area of the screen.

I was thinking about adding below

.visTable {
  display: flex;
  flex-direction: column;
  flex: 1 0 0;
  overflow: auto;
}

Updating the CSS in this way also affects the way EuiDataGrid handles data within the table. With the modified CSS, the table container can now adjust its height based on the total number of rows in the grid (#3756). This indirectly solves the pagination issue by displaying more rows in the table within the available space and having the table appear correctly paginated.

But I think your PR #3797 solves the root cause of the pagination logic to adjust the number of rows displayed in the table. So if the user sets a high value for max rows per page, the pageSize will be limited to the total number of rows, ensuring that there's no excessive empty space.

@curq could you compare two approaches pros and cons? Maybe we should accept both, one for each issue. Then update your PR #3797 to fix both bugs. What do you think? Don't forget to update CHANGELOG.

[curq]

@ananzh I have reviewed both approaches and concluded that it is probably better to accept both. PR #3797 fixes the pagination logic, but it doesn't address the issue with the initial load. However, the approach you suggested does, and I have created a separate PR for it: #3816. By implementing both approaches, we can resolve the current issue and improve the pagination logic to prevent potential problems in the future.

For instance, without the fix for pagination logic, when there is only one resulting page, the pagination controls are not rendered. This may lead to confusion as users might not realize that all results fit on one page.

Here's a screenshot illustrating the issue:

With both fixes applied, it looks like this:

Therefore, I propose that we accept both approaches.

[ananzh]

@curq this is great. I approved both PRs and I will have another reviewer to take a look. Meanwhile, once the mentioned unit test PR, I will have you add/modify the unit test and submit a new PR for the test.