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

[RFR] Added isRowExpandable prop to Datagrid component #5941

Merged
merged 16 commits into from
Feb 26, 2021

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented Feb 21, 2021

Implements #5888

@WiXSL WiXSL changed the title Added isRowExpandable prop to Datagrid component [RFR] Added isRowExpandable prop to Datagrid component Feb 21, 2021
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Thanks! I think you forgot to update the computeNbColumns function. Adding a test would be nice :)

@djhi
Copy link
Contributor

djhi commented Feb 22, 2021

Please ignore the previous comment except for the test.

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

We're removing props injection little by little, and replace them with contexts. I think this is a good occasion to introduce a DatagridContext, which will contain only the isRowExpandable function for a start. That way, you don't need to update the underlying components to sanitize the prop.

docs/List.md Outdated Show resolved Hide resolved
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Looks good so far!

@djhi djhi added this to the 3.13 milestone Feb 23, 2021
@WiXSL WiXSL requested a review from djhi February 23, 2021 13:00
@WiXSL WiXSL requested a review from djhi February 23, 2021 14:09
@fzaninotto fzaninotto removed this from the 3.13 milestone Feb 24, 2021
@fzaninotto fzaninotto merged commit 8bbd8a4 into marmelab:next Feb 26, 2021
@fzaninotto
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants