[DataGridPro] Fix onColumnWidthChange being called twice on autosize

[shaharyar-shamshi]

Fixes #12005

[mui-bot]

Deploy preview: https://deploy-preview-12140--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against e80c76a

[romgrk]

Can you explain the logic behind this logic?

[shaharyar-shamshi]

So overall logic is.

Mouse double click is combination of 2 mouseup and mousedown event.

But according to our previous logic whenever mousedown event used to occur we add the event listeners to listen mouseup event and resize according.

This logic had a collision with the mouse double click so just to distinguish between mouse double click and mouse up/ mouse down event I compared the time interval between mouse down and mouseup event and their coordinate if it is same this directly means user wants to do double click.

Hope I am able to explain this

[shaharyar-shamshi]

@romgrk added proper comment for understanding the code

[shaharyar-shamshi]

@romgrk Added the test case if you can review

[romgrk]

I seem to be able to reproduce the original issue with this PR: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-f9skhw?file=%2Fsrc%2Fdemo.tsx%3A113%2C1

[shaharyar-shamshi]

I seem to be able to reproduce the original issue with this PR: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-f9skhw?file=%2Fsrc%2Fdemo.tsx%3A113%2C1

It seems this is private I can't access this

[romgrk]

Updated permissions.

[shaharyar-shamshi]

Updated permissions.

Basically React state updates are asynchronous but this element key is the DOM element but state update hasn't yet propagated to the DOM before being accessed that's the reason we are getting the old value in the element key but new value elsewhere like in key colDef and width.

If you have any idea how in MUI we can pass the callback function to the setState so that I can get the latest dom element because I think current call signature of setState doesn't allow this.

@romgrk

[shaharyar-shamshi]

Only way that I can figure out as of now to access the DOM element after state change is done in this commit

4bcd99b

But here I have to disable the eslint because I have to trigger the useEffect on change of isAutosizingRef.current but Mutable values like 'isAutosizingRef.current' aren't valid dependencies because mutating them doesn't re-render the component .

this disabling of the eslint will not cause any bug in future as I just need this to access the updated dom element.

To confirm this I updated the test case as well.

@romgrk if you can suggest any better approach

[romgrk]

Wouldn't it be better to dispatch a columnWidthChange event in the autosizeColumns function?

[shaharyar-shamshi]

Wouldn't it be better to dispatch a columnWidthChange event in the autosizeColumns function?

Problem with this approach is when we disptach the columnWidthChange event we have to access the DOM element as well but State update being asynchronous the width is still not propagated to DOM so we get outdated element.clientWidth

[romgrk]

But the widths are available at gridColDef.width. The widths are set first in the state, then in the DOM, not the other way around.

[shaharyar-shamshi]

But the widths are available at gridColDef.width. The widths are set first in the state, then in the DOM, not the other way around.

apiRef.current.publishEvent('columnWidthChange', {
        element: apiRef.current.getColumnHeaderElement(colDefRef.current.field),
        colDef: { ...colDefRef.current, width },
        width,
      });

here you can see we have to publish three key element, colDef, width here key element is the DOM element which is doesn't get updated immediately that the main reason I have to use useEffect to access the updated element after the DOM updation.

In your example also (https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-f9skhw?file=%2Fsrc%2Fdemo.tsx%3A76%2C6) the element.clientWidth was actually wrong not params.width.

[shaharyar-shamshi]

I hope this is clear or should I make one short video explaining this

[romgrk]

This will trigger for all columns regardless of if they have changed. You have the original columns in columns.

You can avoid typings like this, the inference knows it's a number:

const width: number = newColumn.width as number;

[shaharyar-shamshi]

We are doing something wrong this implementation is correct.

let me Explain in detail

There are two possibility of calling autosizeColumns function

1. through doubleClick event Listener https://github.com/mui/mui-x/blob/next/packages/x-data-grid-pro/src/hooks/features/columnResize/useGridColumnResize.tsx#L743 from here handleColumnSeparatorDoubleClick calls the autosizeColumns and over here it pass the exact column which is being resized
2. At the time of mounting when user pass autoResizeOnMount prop https://github.com/mui/mui-x/blob/next/packages/x-data-grid-pro/src/hooks/features/columnResize/useGridColumnResize.tsx#L716

here each of the column is being resized on mounting

In our case to solve this bug we only need to concentrate on case 1 when the autosizeColumns is called on double click.

So on double click event from https://github.com/mui/mui-x/blob/next/packages/x-data-grid-pro/src/hooks/features/columnResize/useGridColumnResize.tsx#L661

we get only one column that needs to be resized and we need not check on all the columns.

Now in case 2 when autOResizeOnMount is called in that case all the columns should be resized and we need to decide whether to publish event or not.

So from both the case we can figure it out that either all of the column will be resized or only 1 will be resized.

if we want to disable the event publish on all column resize (case 2) we just need to add one condition

if (userOptions?.columns) {
          newColumns.forEach((newColumn) => {
            const width = newColumn.width;
            apiRef.current.publishEvent('columnWidthChange', {
              element: apiRef.current.getColumnHeaderElement(newColumn.field),
              colDef: newColumn,
              width,
            });
          });
        }

[romgrk]

There is no need for an if guard, just check the array and dispatch one event for each column that actually changed. Compare newColumn.width !== columns[index].width in the .forEach loop to figure out which ones changed.

The autosize function is part of the public API and can be called programatically with any number of columns.

[shaharyar-shamshi]

Okk then we need to consider this I thought it is not the public api

[shaharyar-shamshi]

Done