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] onSortModelChange now gets passed sorts with a direction of null. #12271

Closed
mbiggs-gresham opened this issue Mar 1, 2024 · 3 comments · Fixed by #12325
Closed
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Sorting Related to the data grid Sorting feature regression A bug, but worse support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@mbiggs-gresham
Copy link

mbiggs-gresham commented Mar 1, 2024

Steps to reproduce

No response

Current behavior

In v6 of the datagrid when a sort is removed, the sort model passed to the onSortModelChange would also have the entire sort entry removed for that field.

In v7 (beta 4) of the datagrid when a sort is removed, the sort model contains an entry for that field with a sort direction of null. This means the sortmodel will just continue to grow. Is this intended as it appears to be a breaking change?

Expected behavior

The sort model should have the entry removed when a sort is removed.

Context

No response

Your environment

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

Search keywords: onSortModelChange
Order ID: 45466

@mbiggs-gresham mbiggs-gresham added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 1, 2024
@michelengelen
Copy link
Member

@MBilalShafi I cannot find the respective PR commit that introduced this. Could you have a look?

@michelengelen michelengelen changed the title [DataGridPremium] onSortModelChange now gets passed sorts with a direction of null. [data grid] onSortModelChange now gets passed sorts with a direction of null. Mar 1, 2024
@michelengelen michelengelen added component: data grid This is the name of the generic UI component, not the React module! feature: Sorting Related to the data grid Sorting feature support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ labels Mar 1, 2024
@MBilalShafi
Copy link
Member

@mbiggs-gresham Thanks for reporting the issue.

Yes, this is an update in v7 (v6 vs v7 examples), it was introduced in #11512 while fixing a bug with the API.

I think we do need to clean up the sortModel for direction: null use-cases to avoid unnecessary items in the sortModel.

@MBilalShafi MBilalShafi added regression A bug, but worse and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 4, 2024
@MBilalShafi MBilalShafi self-assigned this Mar 4, 2024
Copy link

github-actions bot commented Mar 5, 2024

⚠️ This issue has been closed.
If you have a similar problem, please open a new issue and provide details about your specific problem.
If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @mbiggs-gresham?
Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief 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! feature: Sorting Related to the data grid Sorting feature regression A bug, but worse support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants