Skip to content

Commit

Permalink
[DataGrid] Fix error on simultaneous columns and `columnGroupingMod…
Browse files Browse the repository at this point in the history
…el` update (#14368)
  • Loading branch information
cherniavskii committed Sep 2, 2024
1 parent f7f58b3 commit 153d381
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
GridColumnVisibilityModel,
gridColumnPositionsSelector,
gridVisiblePinnedColumnDefinitionsSelector,
gridColumnLookupSelector,
} from '../columns';
import { GridGroupingStructure } from '../columnGrouping/gridColumnGroupsInterfaces';
import { gridColumnGroupsUnwrappedModelSelector } from '../columnGrouping/gridColumnGroupsSelector';
Expand Down Expand Up @@ -106,6 +107,7 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
const columnPositions = useGridSelector(apiRef, gridColumnPositionsSelector);
const renderContext = useGridSelector(apiRef, gridRenderContextColumnsSelector);
const pinnedColumns = useGridSelector(apiRef, gridVisiblePinnedColumnDefinitionsSelector);
const columnsLookup = useGridSelector(apiRef, gridColumnLookupSelector);
const offsetLeft = computeOffsetLeft(
columnPositions,
renderContext,
Expand Down Expand Up @@ -392,7 +394,7 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
firstVisibleColumnIndex,
);
const leftOverflow = hiddenGroupColumns.reduce((acc, field) => {
const column = apiRef.current.getColumn(field);
const column = columnsLookup[field];
return acc + (column.computedWidth ?? 0);
}, 0);

Expand All @@ -411,10 +413,7 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {

const headerInfo: HeaderInfo = {
groupId,
width: columnFields.reduce(
(acc, field) => acc + apiRef.current.getColumn(field).computedWidth,
0,
),
width: columnFields.reduce((acc, field) => acc + columnsLookup[field].computedWidth, 0),
fields: columnFields,
colIndex: columnIndex,
hasFocus,
Expand Down
53 changes: 52 additions & 1 deletion packages/x-data-grid/src/tests/columnGrouping.DataGrid.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { expect } from 'chai';
import { createRenderer, ErrorBoundary, screen } from '@mui/internal-test-utils';
import { createRenderer, ErrorBoundary, fireEvent, screen } from '@mui/internal-test-utils';
import { DataGrid, DataGridProps, GridRowModel, GridColDef } from '@mui/x-data-grid';

const isJSDOM = /jsdom/.test(window.navigator.userAgent);
Expand Down Expand Up @@ -325,6 +326,56 @@ describe('<DataGrid /> - Column grouping', () => {
Array.from(row2Headers).map((header) => header.getAttribute('aria-colindex')),
).to.deep.equal(['1', '2', '3']);
});

// https://github.com/mui/mui-x/issues/13985
it('should not throw when both `columns` and `columnGroupingModel` are updated twice', () => {
function Demo() {
const [props, setProps] = React.useState<
Pick<DataGridProps, 'columns' | 'columnGroupingModel'>
>({
columns: [],
columnGroupingModel: [],
});

const handleClick = () => {
ReactDOM.flushSync(() => {
setProps({
columns: [{ field: `field_0` }],
columnGroupingModel: [{ groupId: 'Group', children: [{ field: `field_0` }] }],
});
});

setProps({
columns: [{ field: `field_1` }],
columnGroupingModel: [{ groupId: 'Group', children: [{ field: `field_1` }] }],
});
};

return (
<div style={{ width: 300, height: 300 }}>
<button onClick={handleClick}>Update columns</button>
<DataGrid rows={[{ id: 1, field_0: 'Value 0', field_1: 'Value 1' }]} {...props} />
</div>
);
}
render(<Demo />);

fireEvent.click(screen.getByRole('button', { name: /Update columns/ }));

const row1Headers = document.querySelectorAll<HTMLElement>(
'[aria-rowindex="1"] [role="columnheader"]',
);
const row2Headers = document.querySelectorAll<HTMLElement>(
'[aria-rowindex="2"] [role="columnheader"]',
);

expect(
Array.from(row1Headers).map((header) => header.getAttribute('aria-label')),
).to.deep.equal(['Group']);
expect(
Array.from(row2Headers).map((header) => header.getAttribute('aria-label')),
).to.deep.equal(['field_1']);
});
});

// TODO: remove the skip. I failed to test if an error is thrown
Expand Down

0 comments on commit 153d381

Please sign in to comment.