AnalyticalTable: The width and the style of the columns cannot be used correctly

[CristianDDias]

Describe the bug
In the AnalyticalTable component, the width and the style of the columns cannot be used correctly. For example:

• When I set width: 20, it's not using it.
• When I apply a CSS class or style, it's not using it.

Besides that, when you click 2x on the same row, it loses the focus (selected row style).

To Reproduce
I created 2 Codesandbox examples, one using ReactTable (the same component used by AnalyticalTable) and another one using AnalyticalTable.

• AnalyticalTable
• ReactTable

Both Codesandbox have the same idea, create a table with 600px width, create a column with a text left-aligned and a button right-aligned, and apply a CSS style to this column. But this is not possible by using the AnalyticalTable component. You can see that the width of the columns is different between both Codesandbox, but only the ReactTable Codesandbox seems right.

Expected behavior

• When I set a column width, it is used.
• When I apply a CSS class or style to a column, it is used.

I think that adding width: '100%' in the tableCellContent class (AnayticalTable.jss.ts) would solve the width problem.

Desktop (please complete the following information):

• OS: Windows
• Browser Chrome
• Version 77

[MarcusNotheis]

here are my first comments:

Setting the width option on column level

The width is accepted as long it is greater than the minWidth option which defaults to 60.
We have a bug if you set the width to higher values than the available horiziontal space, then the columns will shrink.

CSS Classes

This feature hasn't been implemented yet - we'll add that.

Clicking a row twice

This is intended - otherwise you wouldn't be able to deselect a column.

As a general remark: We are using the v7 of react-table which is a headless table (without any UI) - but we try to have all features working that were working with the v6 version.

[CristianDDias]

Hi @MarcusNotheis!

Thanks for the comments.

About the issue related to the column with a text left-aligned and a button right-aligned, I've been analysing the source code and I would like to suggest a fix (a pull request). On the Cell.tsx file, if the cell.render function already returns a component, there is no need to put this component inside a div element.

Bellow is the code that I would like to suggest on the renderedCell function inside the Cell.tsx file:

    const renderCell = cell.render('Cell');
    if (React.isValidElement(renderCell)) {
      return renderCell;
    }
    return <div className={classes.tableCellContent}>{renderCell}</div>;

I already have a branch for this, but before I submit a PR, I think that I need a contribution welcome tag on this issue.

[MarcusNotheis]

Hi @CristianDDias,

thanks for the offer, your proposal sounds very reasonable.
If thats possible, I would like to get rid of the tableCellContent class completely by merging its contents into the tableCell class - could you look into that as well? As far as I remember the tableCellContent class is used in two other places as well.

Thanks for your support!

[MarcusNotheis]

The issue with not accepted style classes is fixed by #207

[CristianDDias]

Hi @MarcusNotheis,

I've tried some approaches to get rid of the tableCellContent class, but at the moment, I'm not able to do this (at least considering my current knowledge about the ui5-webcomponents-react).

If we used only the tableCell class, we would need to migrate the styles from the tableCellContent class to the tableCell class, so that the text could be center aligned (only on those cells that don't have a Cell method to render an element). Besides that, to get the text center aligned, we would need to change the display: flex to display: block, so that we can use the textOverflow: ellipsis, and we would need to set the lineHeight considering the rowHeight. Using these changes, I think we could get rid of the tableCellContent class, at least for the common cells (without considering groupable and expandable cells).

At the moment, I would like to submit a PR that would fix only the problem related to not use a div element if a Cell method was defined on the columns property of the AnalyticalTable component.

[MarcusNotheis]

Hi @CristianDDias, sure, you can just open a PR with your suggestion and then we can move on from there.

[MarcusNotheis]

Hi @CristianDDias,

we applied some changes to Table Cell rendering and got rid of the tableCellContent class. Could you check if the table cells are now behaving as expected (version 0.7.0-rc.1)?

Thanks!

[CristianDDias]

Hi @MarcusNotheis,

Sorry for not submitting a PR before, I had some problems and couldn't prioritize this task :/

But I've tested it using the same Codesandbox example and it's working properly!

Thank you!

From my side, I think we could close this issue.

[MarcusNotheis]

No worries @CristianDDias, I'm pretty sure there will be another issue where you can submit a PR 😄
I'll close the issue.