[Data Explorer][Discover 2.0] Dynamic adjustable row heights based on field data

[ananzh]

Describe the bug
Show empty spaces when _source has less fields. This is because we currently hard code rowHeightsOptions to check _source and set to 3.

const rowHeightsOptions = useMemo(
    () => ({
      defaultHeight: {
        lineCount: adjustedColumns.includes('_source') ? 3 : 1,
      },
    }),
    [adjustedColumns]
  );

[joshuarrrr]

relabeling for 2.12, as it's unassigned with no PR open.

[JohnathonBowers]

I'd be interested in taking this one, Josh.

[joshuarrrr]

@ananzh Can you provide @JohnathonBowers some pointers on how to load some test data to reproduce the screenshot?

[Abilashinamdar]

@ananzh , Is it something we need to check the _source data length and then set the line count?

[JohnathonBowers]

@joshuarrrr @ananzh @ashwin-pc I'm trying to replicate this bug, and I'm having some trouble. I've added sample data to my cluster using the OpenSearch API:

However, I'm not sure where to go from here. I've tried to create a new VisBuilder Visualization to display the documents in my cluster. But I'm not able to replicate what Anan's screenshot is showing. Here is what I have right now:

Any suggestions for how to best replicate the bug?

[JohnathonBowers]

Update: I have been able to replicate the bug. And, yes, it seems that what is causing the issue is the fact that lineCount is hard-coded to 3 when _source is in the adjustedColumns array, as shown in Anan's screenshot at the top of this issue.

The options for fixing this bug are fairly limited at this point, largely because of the built-in limitations of the OuiDataGridRowHeightOption type in oui/src/components/datagrid/data_grid_types.ts, lines 344-346. See the screenshot below:

As the documentaiton website for "Data grid row height options" explains, the rowHeightsOptions prop only allows for numbers to be passed as values for the defaultHeight and rowHeight keys. So, row heights can only be set in pixels or line count.

I would appreciate any advice about the best way to move forward. It seems we have several options:

1. Hard code the default row height to 1 for all types of columns. This would get rid of the empty space that shows when a data set only has a few fields. The downside is that, for larger data sets with many fields, hard-coding the row height to 1 cuts off much of the content.

Here is the adjusted useMemo() function and its effects in Discover when a large vs. a small data set is used:

const rowHeightsOptions = useMemo(
    () => ({
        defaultHeight: 1,
    }),
    []
);

Screen.Recording.2023-11-10.at.12.28.22.PM.mov

2. Leave the code as is and consider other alternatives. (See point 3 below.)
3. Consider refactoring the rowHeightsOptions prop to allow for more fine-tuning (e.g., auto-sizing the row to fit the content or setting a max height for the row).

Perhaps we could take option 1 as a temporary solution and then open a separate issue if we wanted to pursue option 3? Any suggestions @ananzh @joshuarrrr @BSFishy ?

[ashwin-pc]

@JohnathonBowers There are two solutions here. As you correctly pointed out the long term solution here is to update rowHeightsOptions to allow a prop like auto to automatically size each row. Ive opened an issue in OUI (opensearch-project/oui#1172) to track that. In the mean time the best solution here is to allow the user to control the row height in the datatable using some form of control. @shanilpa What do you think of this solution?

[kgcreative]

My preference would be to set the to height to auto, with a configurable max height for when it truncates. I believe advanced settings has a table row max height property we can leverage. Ideally data grid can use that setting

In the fullness of time, we should be able to toggle row height between set number of lines, auto, or single line

[kgcreative]

Related: #5227

[shanilpa]

@ashwin-pc I do wonder if we are able to set the row height options in the header of data grid (next to density). I know not an existing item but maybe an enhancement we can include. The options can be auto, truncate, and custom height?

Having this option in advanced settings means users need to navigate away from the table to update the setting - Im guessing updating in advanced settings also applies globally to all data grid elements. I could see this be frustrating - whereas if the control is per table the user can configure it differently where they need it and they don't need to navigate away every time.

Ignore me looks like you already thought about this in the linked issues.

[ashwin-pc]

Yeah id stay away from the advanced setting as a whole. A datagrid and table while similar in look operate differently and asking a user to define a pixel max height is not the best control. What we need here is 2 things:

1. The ability for the user to select the row height in the table header: This issue
2. Support for auto row height in the data grid component: [OuiDataTable] Add auto rowHeight support to the datatable layout props oui#1172

With both of these in place dealing with data per row should be easier.

[ashwin-pc]

To clarify for anyone coming to this thread. The solution proposed to the issue of row heights not adapting to the field contents and the ability for the user to set the heigh at which truncation begins is being discussed here. Dynamically adjusting the row heights is being tracked by opensearch-project/oui#1172 while the interim solution is to allow the user to manually specify the number of lines they want to display per row. In future both options will be simultaneously available to the user.

[aramvcf]

@ashwin-pc Will the changes work with log field data?
And please confirm that the changes will arrive on the new release version 2.12.
Regards.

[ashwin-pc]

It should work with all forms of data since its simply allowing the user to set how many lines each row should display before truncation. And yes this is slated for 2.12

[ananzh]

This is different from #5227 and #5404. We are looking for an update in OuiDataGrid to auto adjust row height based on the context.

[ananzh]

This is not a problem for datagrid, but still missed in legacy table.