-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DataGrid] Add new List view selector #15024
Conversation
Deploy preview: https://deploy-preview-15024--material-ui-x.netlify.app/ |
if (process.env.NODE_ENV !== 'production') { | ||
if (cacheKey.id === 'default') { | ||
warnOnce([ | ||
'MUI X: A selector was called without passing the instance ID, which may impact the performance of the grid.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests are giving this warning, is this issue due to incorrect handling of args in the selector method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romgrk would you like to share some insights on whether I am going in the right direction or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KenanYusuf owns this feature so he could give you more information. Note that we're already actively working on that issue so I'm not sure if this is a good issue for you to take, Kenan can confirm if he's open to splitting that work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I thought it is open for anyone to take
@KenanYusuf are you willing to collaborate for adding this new selector task in list view? if yes we can proceed and if
not feel free to close this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@k-rajat19 thanks for your willingness to help on this issue, however, we are still discussing the implementation of this feature internally and it is not yet ready for contributions.
In future, if you are unsure whether an issue is ready to be picked up, feel free to ask on the issue to avoid wasted effort. Your contributions are appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, thank you
I have learned few things about the grid project while working on this. so it is somewhat worth it for me atleast😅
This PR addresses 1st point in #14997
Based on #14393 (comment)