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.

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

[DataGrid] Reset sortedRows state on prop change #599

Merged
merged 13 commits into from
Nov 18, 2020
2 changes: 1 addition & 1 deletion packages/grid/_modules_/grid/GridComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
useRows(props.rows, apiRef);
useKeyboard(rootContainerRef, apiRef);
useSelection(apiRef);
useSorting(apiRef);
useSorting(apiRef, props.rows);

useContainerProps(windowRef, apiRef);
const renderCtx = useVirtualRows(columnsHeaderRef, windowRef, renderingZoneRef, apiRef);
Expand Down
12 changes: 10 additions & 2 deletions packages/grid/_modules_/grid/hooks/features/sorting/useSorting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { FeatureModeConstant } from '../../../models/featureMode';
import { CellParams } from '../../../models/params/cellParams';
import { ColParams } from '../../../models/params/colParams';
import { SortModelParams } from '../../../models/params/sortModelParams';
import { RowModel } from '../../../models/rows';
import { RowModel, RowsProp } from '../../../models/rows';
import { FieldComparatorList, SortItem, SortModel } from '../../../models/sortModel';
import { buildCellParams } from '../../../utils/paramsUtils';
import { isDesc, nextSortDirection } from '../../../utils/sortingUtils';
Expand All @@ -29,7 +29,7 @@ import { useGridState } from '../core/useGridState';
import { rowCountSelector, unorderedRowModelsSelector } from '../rows/rowsSelector';
import { SortingState } from './sortingState';

export const useSorting = (apiRef: ApiRef) => {
export const useSorting = (apiRef: ApiRef, rowsProp: RowsProp) => {
const logger = useLogger('useSorting');
const allowMultipleSorting = React.useRef<boolean>(false);
const comparatorList = React.useRef<FieldComparatorList>([]);
Expand Down Expand Up @@ -236,6 +236,14 @@ export const useSorting = (apiRef: ApiRef) => {
const sortApi: SortApi = { getSortModel, setSortModel, onSortModelChange, applySorting };
useApiMethod(apiRef, sortApi, 'SortApi');

React.useEffect(() => {
// When the rows prop change, we reset the sortedRows state.
setGridState((state) => ({
...state,
sorting: { ...state.sorting, sortedRows: rowsProp.map((row) => row.id) },
Copy link
Member

Choose a reason for hiding this comment

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

If we sorted the rows directly, would it solve #599 (comment)?

Copy link
Member Author

Choose a reason for hiding this comment

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

At least we fixed the reported bug and we have a solid test in place, so why not defer a better solution to later. I suspect it will require a refactoring of the state. There is an issue with the control flow somewhere, it could be imperative calls that shouldn't be here, it could be missing dependencies, a desynchronization, no idea :).

I will fix it ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

}));
}, [rowsProp, setGridState]);

React.useEffect(() => {
if (rowCount > 0 && options.sortingMode === FeatureModeConstant.client) {
logger.debug('row changed, applying sortModel');
Expand Down
41 changes: 41 additions & 0 deletions packages/grid/x-grid/src/XGrid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,47 @@ describe('<XGrid />', () => {
fireEvent.click(header);
expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas']);
});

it('should reset the sortedRows state when rows prop change', async () => {
dtassone marked this conversation as resolved.
Show resolved Hide resolved
interface TestCaseProps {
rows: any[];
}
let apiRef;
const TestCase = (props: TestCaseProps) => {
const { rows } = props;
apiRef = useApiRef();
return (
<div style={{ width: 300, height: 300 }}>
<XGrid {...defaultProps} rows={rows} apiRef={apiRef} />
dtassone marked this conversation as resolved.
Show resolved Hide resolved
</div>
);
};

const { setProps } = render(<TestCase rows={defaultProps.rows} />);

let state = apiRef.current.getState();
expect(state.sorting.sortedRows).to.deep.equal([0, 1, 2]);

setProps({
rows: [
{
id: 3,
brand: 'Asics',
},
{
id: 4,
brand: 'RedBull',
},
{
id: 5,
brand: 'Hugo',
},
],
});
await raf(); // wait for the AutoSize's dimension detection logic
dtassone marked this conversation as resolved.
Show resolved Hide resolved
state = apiRef.current.getState();
expect(state.sorting.sortedRows).to.deep.equal([3, 4, 5]);
});
});

describe('state', () => {
Expand Down
58 changes: 58 additions & 0 deletions packages/storybook/src/stories/grid-sorting.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Button } from '@material-ui/core';
import { randomInt } from '@material-ui/x-grid-data-generator';
import * as React from 'react';
import {
ColDef,
Expand All @@ -6,6 +8,7 @@ import {
SortModelParams,
SortModel,
useApiRef,
GridOverlay,
} from '@material-ui/x-grid';
import { withKnobs } from '@storybook/addon-knobs';
import { action } from '@storybook/addon-actions';
Expand Down Expand Up @@ -432,3 +435,58 @@ export const ServerSideSorting = () => {
</div>
);
};

export const ResetSortingRows = () => {
const columns = [
{
field: 'name',
width: 200,
},
{
field: 'team',
width: 200,
},
];
const [rows, setRows] = React.useState<RowsProp>([]);

const createRandomRows = () => {
const randomRows: any[] = [];

for (let i = 0; i < 10; i += 1) {
const id = randomInt(0, 100000).toString();
randomRows.push({ id, name: 'name test', team: id });
}

setRows(randomRows);
};

return (
<div
style={{
height: '1000px',
dtassone marked this conversation as resolved.
Show resolved Hide resolved
}}
>
<Button onClick={() => createRandomRows()}>Random Rows</Button>
<XGrid
rows={rows}
columns={columns}
loading={rows.length === 0}
rowHeight={56}
hideFooter
showCellRightBorder
showColumnRightBorder
disableExtendRowFullWidth
onColumnHeaderClick={() => {}}
dtassone marked this conversation as resolved.
Show resolved Hide resolved
sortModel={[
{
field: 'name',
sort: 'asc',
},
]}
components={{
noRowsOverlay: () => <GridOverlay>No Data</GridOverlay>,
}}
dtassone marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
);
};