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

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

Closed
drashtipatoliyaARIS opened this issue Sep 2, 2024 · 21 comments · Fixed by #14560
Closed

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

drashtipatoliyaARIS opened this issue Sep 2, 2024 · 21 comments · Fixed by #14560
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! support: commercial Support request from paid users support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@drashtipatoliyaARIS
Copy link

drashtipatoliyaARIS commented Sep 2, 2024

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

@drashtipatoliyaARIS drashtipatoliyaARIS added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels Sep 2, 2024
@github-actions github-actions bot added the component: data grid This is the name of the generic UI component, not the React module! label Sep 2, 2024
@michelengelen michelengelen added the support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/ label Sep 2, 2024
@michelengelen michelengelen changed the title [data-grid-pro] Can not preserve updated width of columns after resizing [data grid] Can not preserve updated width of columns after resizing Sep 2, 2024
@michelengelen
Copy link
Member

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?

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 2, 2024
@drashtipatoliyaARIS
Copy link
Author

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.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 2, 2024
@michelengelen
Copy link
Member

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

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 2, 2024
@drashtipatoliyaARIS
Copy link
Author

drashtipatoliyaARIS commented Sep 2, 2024

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

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 2, 2024
@michelengelen
Copy link
Member

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
Copy link
Member

@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
Copy link

tleclaire commented Sep 3, 2024

Same problem here... dimensions don't restore

@arminmeh
Copy link
Contributor

arminmeh commented Sep 4, 2024

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.

@arminmeh arminmeh added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 4, 2024
@tleclaire
Copy link

tleclaire commented Sep 4, 2024

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
Copy link
Author

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.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 4, 2024
@michelengelen
Copy link
Member

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
Copy link
Contributor

arminmeh commented Sep 4, 2024

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
Copy link
Contributor

arminmeh commented Sep 4, 2024

@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.

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 5, 2024
@dhruvin-kanani
Copy link

@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
Copy link
Author

@arminmeh any update on this?

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 7, 2024
@arminmeh
Copy link
Contributor

arminmeh commented Sep 9, 2024

@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

@arminmeh arminmeh added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 9, 2024
@drashtipatoliyaARIS
Copy link
Author

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?

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Sep 16, 2024
@arminmeh
Copy link
Contributor

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

@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 16, 2024
@MBilalShafi MBilalShafi self-assigned this Oct 11, 2024
@drashtipatoliyaARIS
Copy link
Author

any updates on this feature?

@michelengelen
Copy link
Member

@drashtipatoliyaARIS it's already mentioned here that @MBilalShafi is working on a PoC: #14452 (comment)

Copy link

github-actions bot commented Nov 7, 2024

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@drashtipatoliyaARIS How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! support: commercial Support request from paid users support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/
Projects
Status: 👀 In review
Development

Successfully merging a pull request may close this issue.

6 participants