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.

Sign up for GitHub

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

Jump to bottom

[Fix] Datagrid Custom Body Render general fixes #8028

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+23 −3
Draft

[Fix] Datagrid Custom Body Render general fixes #8028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58e9a98
fix: render custom body fixes
logeekal Sep 18, 2024
6b4fcdd
update docs
logeekal Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/eui/src-docs/src/views/datagrid/advanced/custom_renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ export default () => {
visibleColumns,
visibleRowData,
setCustomGridBodyProps,
headerRow,
footerRow,
}: EuiDataGridCustomBodyProps) => {
// Ensure we're displaying correctly-paginated rows
const visibleRows = raw_data.slice(
Expand Down Expand Up @@ -238,6 +240,7 @@ export default () => {

return (
<>
{headerRow}
{visibleRows.map((row, rowIndex) => (
<div role="row" css={styles.row} key={rowIndex}>
<div css={styles.rowCellsWrapper}>
Expand All @@ -264,6 +267,7 @@ export default () => {
)}
</div>
))}
{footerRow}
</>
);
},
Expand Down
10 changes: 9 additions & 1 deletion packages/eui/src/components/datagrid/body/cell/data_grid_cell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,12 @@ export class EuiDataGridCell extends Component<

cellProps.style = {
...style, // set by react-window or the custom renderer
height:
this.props.columnId === 'timeline-event-detail-row'
? undefined
: style?.height, // row height, can be undefined
top: style?.top ? 0 : undefined, // The cell's row will handle top positioning
width, // column width, can be undefined
width: width === 0 ? '100%' : width, // column width, can be undefined
Comment on lines +614 to +618
Copy link
Member

@cee-chen cee-chen Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes feel a little spicy to me, I'm not a huge fan of them. If they're for the custom row detail, don't forget that you can override the cellProps.style via setCellProps within your renderCellValue component, e.g.

<EuiDataGrid
  renderCustomGridBody={...}
  renderCellValue={({ columnId, setCellProps }) => {
    useEffect(() => {
      if (columnId === 'timeline-event-detail-row') {
        setCellProps({ style: { width: '100%', height: undefined } })
      }
    }, [columnId, setCellProps]);

    return ...
  }}
/>

Copy link
Author

@logeekal logeekal Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @cee-chen,

Thanks for the comment.

Below change is gone and is not not needed. It was to debug something. I left it here by mistake.

        this.props.columnId === 'timeline-event-detail-row'
          ? undefined
          : style?.height, // row height, can be undefined

About this change :

      width: width === 0 ? '100%' : width, // column width, can be undefined

Problem

I do not disagree but i have not been able to find the better alternative. I have setup an example for you here. This is almost exactly what i am implementing in Security Solution : https://codesandbox.io/p/sandbox/custom-body-render-react-window-vm9m6k?file=%2Fdemo.tsx%3A119%2C3-119%2C15

Problem is that when setCellProps is run, it does not actually updates the Cell component which is really important to measure the size of cell component.

  • This is because it only updates the css.
  • That css is actually overriden by the these recaculateAutoHeight and recalculateLineHeight functions.
  • Now i was thinking what could be the best way to override or calculate height settings for Extra Row. Long term solution could be accept extra row component and treat it separately instead of applying settings of all the cells to that cell. But for now, i thought that do combine above change + this change to get around this problem.

Demo

Now in our case of Custom Row, the width is initially 0 as passed to EuiDataGrid in trailingColumns config ( which also feels incorrect imo ). This creates a problem when the row is actually rendered, it starts with a width of 0 which is measured as soon as it is rendered and additionally height is autoHeight which is actually equal to the height of all other cells. But that Custom Row cell is not like other cells so it should not be treated as such.

To see that in demo, go to the codesandbox and search for Row-0 in console messages. It will show the transition in the dimensions of the DataGrid Rows. You will observe that event setCellProps has been triggered, the Cell with custom row is only updated with initial dimension.

That is why, I updated the logic to have correct initial dimension as 0 seemed incorrect. Let me know if there is a better way I can go around it.

lineHeight: rowHeightsOptions?.lineHeight ?? undefined, // lineHeight configuration
...cellPropsStyle, // apply anything from setCellProps({ style })
};
Expand All @@ -621,6 +625,10 @@ export class EuiDataGridCell extends Component<
rowHeightsOptions
);

console.log(
`rowHeight for Column ID : ${this.props.columnId} is ${rowHeight}`
);

const row =
rowManager && !IS_JEST_ENVIRONMENT
? rowManager.getRow({
Expand Down
5 changes: 3 additions & 2 deletions packages/eui/src/components/datagrid/body/data_grid_body_custom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export const EuiDataGridBodyCustomRender: FunctionComponent<
const Cell = useCallback<EuiDataGridCustomBodyProps['Cell']>(
({ colIndex, visibleRowIndex, ...rest }) => {
const style = {
...rest.style,
height: rowHeightUtils.isAutoHeight(visibleRowIndex, rowHeightsOptions)
? 'auto'
: getRowHeight(visibleRowIndex),
Expand Down Expand Up @@ -192,14 +193,14 @@ export const EuiDataGridBodyCustomRender: FunctionComponent<
customGridBodyProps?.className
)}
>
{headerRow}
{renderCustomGridBody!({
visibleColumns,
visibleRowData: visibleRows,
Cell,
setCustomGridBodyProps,
headerRow,
footerRow,
})}
{footerRow}
</div>
);
};
3 changes: 3 additions & 0 deletions packages/eui/src/components/datagrid/data_grid_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@ export interface EuiDataGridCustomBodyProps {
* It's best to wrap calls to `setCustomGridBodyProps` in a `useEffect` hook
*/
setCustomGridBodyProps: (props: EuiDataGridSetCustomGridBodyProps) => void;

headerRow?: ReactElement;
footerRow?: ReactElement | null;
}

export type EuiDataGridSetCustomGridBodyProps = CommonProps &
Expand Down
4 changes: 4 additions & 0 deletions packages/eui/src/components/datagrid/utils/row_heights.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,10 @@ export const useDefaultRowHeight = ({
);
}

console.log(
`Height of Row ${rowIndex} is ${rowHeight || defaultRowHeight}`
);

// Use the row-specific height if it exists, if not, fall back to the default
return rowHeight || defaultRowHeight;
},
Expand Down
Loading