-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[data grid] Fix typo on the rowSelectionPropagation
feature
#14907
[data grid] Fix typo on the rowSelectionPropagation
feature
#14907
Conversation
rowSelectionPropagation
feature
Deploy preview: https://deploy-preview-14907--material-ui-x.netlify.app/ Updated pages: |
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.
Thank you 💙
Nitpick: Suggested an alternate grammatical sense since I've seen the usage of 'will' to be discouraged. Feel free to drop the suggestions.
Regarding the other suggestions, totally agree on the first one, will handle in a follow-up PR.
On the second one, we lately started to follow more and more the pattern of passing arguments rather than an options object. But I'm fine with moving to the object pattern in this specific case if there's a clear win.
Noticed those while working on #14899
Other proposed change
I'm also not a fan of having that many flat params on
findRowsToSelect
andfindRowsToDeselect
.I feel like it would be a lot more readable:
props.rowSelectionPropagation
to the function (right now if you are searching the codebase, this concept can be namedrowSelectionPropagation.descendants
orautoSelectDescendants
depending on where you are looking for)