[datagrid] V7 API: sortColumn doesn't work as documented

[elkind00]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Link to live example:

https://codesandbox.io/s/pensive-blackwell-2hjnrj

Steps:

1. Click on "Sort Desc". Nothing happens.
2. Click on the sort icon of the ID column so that it is in descending order: 3, 2, 1
3. Click on "Sort Asc". Nothing happens

Current behavior 😯

When the "Sort Desc" or "Sort Asc" buttons are clicked on, nothing happens.

Expected behavior 🤔

The sort of the DataGrid should change based on the values in the ID column

Context 🔦

I'd like to be able to perform sorts and filters from buttons.

Apologies for previous bug report. I had tried several different approaches and it went unnoticed when the "() =>" portion was accidentally deleted.

Your environment 🌎

I use the Chrome browser.

npx @mui/envinfo

System:
OS: macOS 13.3.1
Binaries:
Node: 19.7.0 - ~/.nvm/versions/node/v19.7.0/bin/node
Yarn: 1.22.19 - /usr/local/bin/yarn
npm: 9.5.0 - ~/.nvm/versions/node/v19.7.0/bin/npm
Browsers:
Chrome: 114.0.5735.133
Edge: Not Found
Safari: 16.4

Order ID or Support key 💳 (optional)

No response

[romgrk]

Here is a fixed version of that codesandbox: https://codesandbox.io/s/romantic-engelbart-nvqh4p?file=/src/App.tsx

Make sure to read our documentation on sorting.

[elkind00]

Thanks for the corrections. I did some testing, and here's a little additional information for possible readers of this issue.

Apparently only adding "sortable: true" to the Columns definition was necessary to make sorting work from Buttons. It isn't necessary if you're only using the sort icons in the column header, but if you want to sort from outside the grid using apiRef.current.sortColumn from a Button then "sortable: true" is necessary. The documentation says "Sorting is enabled by default" (twice!), but it apparently only means from within the DataGrid. When using apiRef.current.sortColumn you need the sortable property set to true.

"sortable: true" doesn't have to be added to the Columns definition. You can instead just include it in the call to apiRef.current.sortColumn() that's called by the Button. The onClick portion of the definition would look like this

onClick={() => apiRef.current.sortColumn({field: 'id', sortable: true}, "asc", false)}

I've put a live example at:

https://codesandbox.io/s/trusting-zeh-xphfvp

[Serpent317]

Thank you for actually explaining how the problem was fixed.

[romgrk]

After re-reading the comment above, I think either our docs need to be fixed, or the behavior of the code.

[MBilalShafi]

This should be handled along with #10378 in the v7.
I'll close this one, we can track it there.

[romgrk]

Disagree, this one is about the behavior of sortColumns. As mentioned above, in the docs we say "sorting is enabled by default", but in the codebase we require the sortable: true flag to sort columns. We also need to adapt according to our discussion regarding the meaning of flags.

[MBilalShafi]

AFAIU, sortable is needed because of the fact that we expect GridColDef from the user and then check from the same colDef if the column is sortable.

mui-x/packages/grid/x-data-grid/src/hooks/features/sorting/useGridSorting.ts

Lines 188 to 192 in 34a9216

	const sortColumn = React.useCallback<GridSortApi['sortColumn']>(
	(column, direction, allowMultipleSorting) => {
	if (!column.sortable) {
	return;
	}

But when we shift this to accept GridColDef['field'] only (the main goal of #10378), we will then fetch the actual colDef internally which will already have the sortable boolean value. Due to this reason, I thought this one would be auto-solved with #10378. Please enlighten me if there is another aspect to this that I am not aware of. 🙏

[romgrk]

As we discussed elsewhere, the flags like .sortable and .resizable shouldn't prevent the action through the API, they should prevent the browser-user from performing the action through our UI.

Also, our doc says that sorting is enabled by default. So the sortable: true flag should not be necessary. At the very least, the API check should be if (!(column.sortable ?? true)) { return } to be consistent with what the documentation says.

[cherniavskii]

Superseded by #10552