mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.07k stars 1.26k forks source link

[data grid] Can not preserve updated width of columns after resizing #14452

Open drashtipatoliyaARIS opened 2 weeks ago

drashtipatoliyaARIS commented 2 weeks ago

The problem in depth

Reproduction Link: Link

Hi MUI DataGrid Team,

I am currently working with the MUI DataGrid and have encountered an issue related to column resizing. Specifically, after resizing columns, any subsequent state updates cause them to revert to their original sizes.

To mitigate this, I attempted to use the export state and restore state solution provided in the documentation. However, this approach requires me to include all state variables in the useEffect dependency array and restore the state each time there is a change. This is cumbersome and affects the child components, inadvertently triggering a column resize on any parent state update.

I am just contacting you to ask if there is a more efficient or global method to handle this issue, rather than storing and resetting the grid state with every state variable update.

I’m looking for a solution that avoids unnecessary re-rendering or a more streamlined way to maintain the column sizes without much manual intervention.

Current behavior Step 1: Resize any column. Step 2: Click on delete button. You will notice that the column size returns to its initial state. This is because it is updating state of other variable However, the column should not be resized.

Expected behavior The column should not be resized.

I would greatly appreciate any guidance or alternative solutions you could provide.

Thank you

Your environment

`npx @mui/envinfo` ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: Resizing of Grid Columns Order ID: 91701

michelengelen commented 2 weeks ago

Hey @drashtipatoliyaARIS .... You can just use the onColumnWidthChange callback:

import * as React from 'react';
import Box from '@mui/material/Box';
import CircularProgress from '@mui/material/CircularProgress';
import { useDemoData } from '@mui/x-data-grid-generator';
import { GridInitialState, DataGridPremium, useGridApiRef } from '@mui/x-data-grid-premium';

export default function SaveAndRestoreStateInitialState() {
  const apiRef = useGridApiRef();
  const { data, loading } = useDemoData({
    dataSet: 'Commodity',
    rowLength: 100,
    maxColumns: 10,
  });

  const saveColumnWidths = React.useCallback(() => {
    if (apiRef?.current?.exportState && localStorage) {
      const currentState = apiRef.current.exportState();
      console.log('currentState', currentState);
      localStorage.setItem(
        'dataGridStateColState',
        JSON.stringify(currentState.columns?.dimensions || {}),
      );
    }
  }, [apiRef]);

  React.useLayoutEffect(() => {
    if (apiRef?.current?.restoreState && localStorage) {
      const stateFromLocalStorage = localStorage?.getItem('dataGridStateColState');
      apiRef.current.restoreState({
        columns: {
          dimensions: JSON.parse(stateFromLocalStorage || '{}'),
        },
      });
    }
  }, [apiRef]);

  return (
    <Box sx={{ height: 300, width: '100%' }}>
      <DataGridPremium
        {...data}
        apiRef={apiRef}
        loading={loading}
        onColumnWidthChange={saveColumnWidths}
      />
    </Box>
  );
}

Does that solve your usecase?

drashtipatoliyaARIS commented 2 weeks ago

Hi @michelengelen , Thank you for the solution. I am already restoring state from exported state using apiRef, but it resets on each state update of file. and our code base consists of 4-5 grid tables on single page. so storing everything in localstorage doesn't seem efficient to me.

michelengelen commented 2 weeks ago

The storage method is not really important. You can use redux, recoil, Zustand, Jotai, a custom middleware/server solution or whatever. This works for a single dataGrid, but should be equally viable for multiple ones

drashtipatoliyaARIS commented 2 weeks ago

@michelengelen I have tried your code in my stackblitz link. it doesn't work with other variable update

michelengelen commented 2 weeks ago

For every state update to take into account you would need to run the useEffect on each rerender. you can do that by removing the dependency array from it.

Although I am not sure why the column dimensions are not preserved through renders. I am going to have a look tomorrow! 👍🏼

michelengelen commented 2 weeks ago

@arminmeh could you take over from here? I have tested this and other states restore just fine (like the filter state, or the sorting state), so why does the dimension state not behave the same?

tleclaire commented 2 weeks ago

Same problem here... dimensions don't restore

arminmeh commented 2 weeks ago

Hi all,

The issue occurs because the column model gets updated on each re-render, since it is not memoized. In the very first alert in column definition page it is stated that

The columns are designed to be definitions, to never change once the component is mounted. Otherwise, you take the risk of losing elements like column width or order.

To improve performance, it is suggested that the same principle is applied to all static props

This is a code snippet that can help with the code provided in this issue

const deleteRows = React.useCallback(
  (id: GridRowId) => {
    setAlert('row ' + id + ' will be deleted');
    // setRows((prevState) => prevState.filter((obj) => obj.id !== id));
  },
  [],
);

const columns = React.useMemo(
  () => [
    { field: 'id' },
    { field: 'username', width: 125, minWidth: 150, maxWidth: 200 },
    { field: 'age', resizable: false },
    {
      field: 'action',
      renderCell: (params: GridRenderCellParams) => (
        <Button onClick={() => deleteRows(params.id)}>Delete</Button>
      ),
    },
  ],
  [deleteRows],
);

return (
  <>
    {alert}
    <div style={{ height: 250, width: '100%' }}>
      <DataGrid
        apiRef={apiRef}
        columns={columns}
        rows={rows}
        onColumnWidthChange={saveColumnWidths}
      />
    </div>
  </>
);

Hope that this helps.

tleclaire commented 2 weeks ago

Does not work for me. I memoize my Columns like this: const memoizedColumns = useMemo(() => props.columns, [props.columns]); And everything is restored except the Dimensions. Update: When I restore the state delayed, it works:

  setTimeout(() => {
          apiRef.current.restoreState(parsedState);
        }, 100);
drashtipatoliyaARIS commented 2 weeks ago

This way I need to add all variables as callback and memo dependency. also I need to wrap all the functions with callback. so adding all variables as dependency doesn't seem reliable to me.

Also only dimension is not preserved in next render. sort and filter works fine.

michelengelen commented 2 weeks ago

Does not work for me. I memoize my Columns like this: const memoizedColumns = useMemo(() => props.columns, [props.columns]); And everything is restored except the Dimensions. Update: When I restore the state delayed, it works:

  setTimeout(() => {
          apiRef.current.restoreState(parsedState);
        }, 100);

Yes, because the state seemingly applies after the columns are rendered and do not update afterwards (see #11576 and #11494)

@arminmeh IMHO this is clearly a bug in the grid, because the other state updates work just fine and only this part is not. This could be a race condition of sorts, or we simply do not apply this specific part of the state. IDK. Unfortunately i do not have enough time looking into it. 🤷🏼

arminmeh commented 2 weeks ago

We can split this in two separate discussions. I will give answer/opinion on each

1.

This way I need to add all variables as callback and memo dependency. also I need to wrap all the functions with callback. so adding all variables as dependency doesn't seem reliable to me.

You don't have to do it for all variables. Column model is expected to be static. Other props can change on re-render. They will cause data grid to re-render, but it will not affect the column width. State setters are considered static over re-renders, so they don't have to be in the dependency list. To reduce the code change from my previous example, handler can be moved inside useMemo hook resulting in

const columns = React.useMemo(() => {
  const deleteRows = (id: GridRowId) => {
    setAlert('row ' + id + ' will be deleted');
    //setRows((prevState) => prevState.filter((obj) => obj.id !== id));
  };

  return [
    { field: 'id' },
    { field: 'username', width: 125, minWidth: 150, maxWidth: 200 },
    { field: 'age', resizable: false },
    {
      field: 'action',
      renderCell: (params: GridRenderCellParams) => (
        <Button onClick={() => deleteRows(params.id)}>Delete</Button>
      ),
    },
  ];
}, []);

2.

Also only dimension is not preserved in next render. sort and filter works fine.

and

IMHO this is clearly a bug in the grid, because the other state updates work just fine and only this part is not.

I would not consider it a bug, since it is stated in our docs what the expectation regarding the column model is. On the other hand, we should not force people to memoize the model if they don't want to do it or do not care about the performance (always working with the small datasets). So, keeping the widths after re-render is something I would also expect. Especially since other user interactions are preserved as well.

@mui/xgrid thoughts?

arminmeh commented 2 weeks ago

@tleclaire

Does not work for me. I memoize my Columns like this: const memoizedColumns = useMemo(() => props.columns, [props.columns]);

I assume you pass the columns from a parent component. Probably in that component your columns are changed on each re-render.

dhruvin-kanani commented 1 week ago

@arminmeh


const columns = React.useMemo(() => {
  const deleteRows = (id: GridRowId) => {
    setAlert('row ' + id + ' will be deleted');
    //setRows((prevState) => prevState.filter((obj) => obj.id !== id));
  };

In this code you've used prevState to update the rows, but if you want to update the rows using other state variables then things will start to look ugly as you've to add those states in the dependency array to get updated states and at that time columns will be re-rendered.

drashtipatoliyaARIS commented 1 week ago

@arminmeh any update on this?

arminmeh commented 1 week ago

@dhruvin-kanani the code that I have posted is based on the example that @drashtipatoliyaARIS has provided in the issue description the line with the state is actually commented out (like in the example)

to give you proper advice, I have to ask you to share all the code relevant to the delete action

drashtipatoliyaARIS commented 2 days ago

We can split this in two separate discussions. I will give answer/opinion on each

1.

This way I need to add all variables as callback and memo dependency. also I need to wrap all the functions with callback. so adding all variables as dependency doesn't seem reliable to me.

You don't have to do it for all variables. Column model is expected to be static. Other props can change on re-render. They will cause data grid to re-render, but it will not affect the column width. State setters are considered static over re-renders, so they don't have to be in the dependency list. To reduce the code change from my previous example, handler can be moved inside useMemo hook resulting in

const columns = React.useMemo(() => {
  const deleteRows = (id: GridRowId) => {
    setAlert('row ' + id + ' will be deleted');
    //setRows((prevState) => prevState.filter((obj) => obj.id !== id));
  };

  return [
    { field: 'id' },
    { field: 'username', width: 125, minWidth: 150, maxWidth: 200 },
    { field: 'age', resizable: false },
    {
      field: 'action',
      renderCell: (params: GridRenderCellParams) => (
        <Button onClick={() => deleteRows(params.id)}>Delete</Button>
      ),
    },
  ];
}, []);

2.

Also only dimension is not preserved in next render. sort and filter works fine.

and

IMHO this is clearly a bug in the grid, because the other state updates work just fine and only this part is not.

I would not consider it a bug, since it is stated in our docs what the expectation regarding the column model is. On the other hand, we should not force people to memoize the model if they don't want to do it or do not care about the performance (always working with the small datasets). So, keeping the widths after re-render is something I would also expect. Especially since other user interactions are preserved as well.

@mui/xgrid thoughts?

@arminmeh is there any update on this feature?

arminmeh commented 2 days ago

@drashtipatoliyaARIS, @MBilalShafi is working on a POC to address this issue You can follow the progress here