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.

Sign up for GitHub

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

Jump to bottom

[DataGrid] Rename GridColumnsState['all'] to GridColumnsState['orderedFields'] #6562

Merged
merged 2 commits into from
Oct 26, 2022
Merged

[DataGrid] Rename GridColumnsState['all'] to GridColumnsState['orderedFields'] #6562

merged 2 commits into from
Oct 26, 2022

Conversation

DanailH
Copy link
Member

@DanailH DanailH commented Oct 19, 2022 •
edited
Loading

Fixes #5607

Breaking changes

- state.columns.all
+ state.columns.orderedFields

@DanailH DanailH added breaking change component: data grid This is the name of the generic UI component, not the React module! labels Oct 19, 2022
@DanailH DanailH self-assigned this Oct 19, 2022
@mui-bot
Copy link

mui-bot commented Oct 19, 2022 •
edited
Loading

Messages
📖 Netlify deploy preview: https://deploy-preview-6562--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 486 743.3 698.9 631.1 116.338
Sort 100k rows ms 550 1,175.7 813.6 860.94 199.755
Select 100k rows ms 206.7 255.1 215.3 225.12 19.479
Deselect 100k rows ms 129.5 212.8 204.1 189.36 30.8

Generated by 🚫 dangerJS against bed4939

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 19, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 20, 2022
cherniavskii
cherniavskii approved these changes Oct 24, 2022
View reviewed changes
Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR doesn't introduce any breaking changes in our public API, right?

@DanailH
Copy link
Member Author

DanailH commented Oct 26, 2022

This PR doesn't introduce any breaking changes in our public API, right?

am It does change the state structure, that's why I marked it as a breaking change. I've updated the description.

@DanailH DanailH merged commit 3cb20db into mui:next Oct 26, 2022
@cherniavskii
Copy link
Member

@DanailH should we add it to https://next.mui.com/x/migration/migration-data-grid-v5/ then?

On the other hand, in our docs we mention that state is not considered a public API https://next.mui.com/x/react-data-grid/state/#direct-selector-access

@DanailH
Copy link
Member Author

DanailH commented Oct 26, 2022

@cherniavskii true, I'm not sure if it's needed to add this to the migration page. The other PR with the deprecated APIs will need a docs update. I'll do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m4theushw m4theushw m4theushw approved these changes

@cherniavskii cherniavskii cherniavskii approved these changes

@MBilalShafi MBilalShafi Awaiting requested review from MBilalShafi

Assignees

@DanailH DanailH

Labels
breaking change component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[data grid] Rename GridColumnsState all field
4 participants
@DanailH @mui-bot @cherniavskii @m4theushw