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] Fix undefined row id #10670

Merged
merged 5 commits into from
Nov 1, 2023
Merged

[DataGrid] Fix undefined row id #10670

merged 5 commits into from
Nov 1, 2023

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Oct 13, 2023 •
edited

Closes #10256

Use apiRef.current.getRowId.

Before: https://codesandbox.io/s/mui-filter-issue-vx5jd8?file=/Demo.tsx

@romgrk romgrk added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Oct 13, 2023
@mui-bot
Copy link

mui-bot commented Oct 13, 2023 •
edited

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

Generated by 🚫 dangerJS against 0233ef8

@tnoetzel
Copy link

Possible to get this reviewed and launched?

@romgrk
Copy link
Contributor Author

romgrk commented Oct 22, 2023

@cherniavskii @MBilalShafi Nevermind for the review request, this is not complete yet.

@romgrk romgrk marked this pull request as draft October 22, 2023 17:28
@romgrk romgrk marked this pull request as ready for review October 25, 2023 03:38
@romgrk romgrk requested review from MBilalShafi and cherniavskii and removed request for MBilalShafi and cherniavskii October 25, 2023 03:39
@romgrk
Copy link
Contributor Author

romgrk commented Oct 25, 2023

This is ready for review now. The "after" case should be ready here in a few moments: https://ci.codesandbox.io/status/mui/mui-x/pr/10670/builds/431205

cherniavskii
cherniavskii approved these changes Oct 25, 2023
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.

LGTM!
Could you cover it with a unit test?

@cherniavskii cherniavskii added the feature: Filtering Related to the data grid Filtering feature label Oct 25, 2023
@tnoetzel
Copy link

@romgrk - Any update here? Seems like this is good to go besides a unit test?

@romgrk romgrk enabled auto-merge (squash) October 31, 2023 07:23
@tnoetzel
Copy link

tnoetzel commented Nov 1, 2023

@romgrk @cherniavskii - Anybody able to get this across the finish line? Sorry to keep bugging, but I've got a giant PR waiting on this being fixed.

@romgrk romgrk merged commit 35ac35c into mui:master Nov 1, 2023
16 checks passed
@romgrk romgrk deleted the fix-undefined-row-id branch November 1, 2023 06:58
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Nov 6, 2023
@romgrk @alexfauquette
[DataGrid] Fix undefined row id (mui#10670)
8948ab8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cherniavskii cherniavskii cherniavskii approved these changes

@MBilalShafi MBilalShafi Awaiting requested review from MBilalShafi

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No row with id #undefined found
4 participants
@romgrk @mui-bot @tnoetzel @cherniavskii