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

[DataGridPro] Add support for master/detail #3387

Merged
merged 58 commits into from
Feb 3, 2022

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Dec 10, 2021

Preview: https://deploy-preview-3387--material-ui-x.netlify.app/components/data-grid/group-pivot/#master-detail

Closes #211

How does it work?

The master/detail implemented here works in two steps:

The first step is to adjust the height of the row whose we want to show the detail to include the height of the panel too. That way, the final row height is: base row height (defined via rowHeight prop) + detail panel height (defined via getDetailPanelHeight prop). Doing this, we don't need to worry about scrollToIndexes nor fixing the virtualization, because, in theory, it would already have #3218. The rendering of the detail panel is done later. To fill the space left for the panel, a bottom margin is applied to the row.

The second step is to render all of the detail panels. This is achieved by rendering all of them inside a div placed on top of the render zone, which can be understood as a "layer". Each panel is absolute positioned so the top corresponds to the bottom position of the row it belongs to. That way, it creates the impression that the panel is rendered immediately below the row, when it's not. I chose this approach because the alternative would be to render the panel inside the GridRow, which could cause some flickering when the virtualization renders new rows. The idea of adding another layer ensures that the open detail panels are rendered once and never again, no matter how much the user scrolls.

Changelog

@m4theushw m4theushw self-assigned this Dec 14, 2021
@github-actions
Copy link

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 14, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 16, 2021
@m4theushw m4theushw mentioned this pull request Dec 16, 2021
2 tasks
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 16, 2021
@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 Dec 16, 2021
@m4theushw m4theushw marked this pull request as ready for review December 16, 2021 21:45
@m4theushw
Copy link
Member Author

I still want to add some very complex demos (e.g. combine with column pinning and use a form as the detail), but this can be done while the PR is reviewed. Don't focus too much in the changes done to support variable row height, this is not the main concern here. The changes I made are likely to change depending on #3218.


const heights = apiRef.current.unstable_applyPreProcessors(
GridPreProcessingGroup.rowHeight,
{ base: rowHeight }, // We use an object to make simple to check if a size was already added or not
Copy link
Member Author

Choose a reason for hiding this comment

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

Allowing other hooks to pre-process the row height unblocks #2144.

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

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

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

Nice documentation, I enjoyed the DataGrind inside a DataGrid. It should probably have its own page because it does not have that much relation with the tree data.

const ownerState: OwnerState = { classes: rootProps.classes, isExpanded };
const classes = useUtilityClasses(ownerState);

const contentCache = useGridSelector(apiRef, gridExpandedRowsContentCache);
Copy link
Member

Choose a reason for hiding this comment

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

What about adding a function disableDetailPannel which takes the row as an input and returns a boolean?

I would avoid having to cache its value and decouple the action management (allowing/preventing google) and the rendering of the panel.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean to add a hasDetailPanel(row) method to the API? Similar to isColumnPinned(field).

Copy link
Member

Choose a reason for hiding this comment

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

Not exactly. For now, developers have access to two parameters:

  • getDetailPanelContent
  • getDetailPanelHeight

And if you do not want to have a detail panel for some row, the getDetailPanelContent must return null or undefined. With this method, you have to evaluate all the getDetailPanelContent(row) to know if the detail panel is a valid component. I'm proposing to let developers provide a third prop disableDetailPannel(row).

I thought about it for two reasons:

  • Using React.isValidElement seems a bit hacky.
  • I imagine for developers, knowing if the row has a detail panel will be easy. It will probably be a condition such as: "the row status is open"

@HunteRoi
Copy link

I'm eager to be able to use this feature, guys!!

Do you have an ETA for it or should I ask Santa for a late gift? 😄

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

github-actions bot commented Feb 2, 2022

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

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

@DanailH DanailH left a comment

Choose a reason for hiding this comment

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

It looks awesome and the docs and examples are perfect, really great job! Should we aim to merge this and have it in this week's release?

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

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

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

Yeah, the arrow is better but it's already taken. We need to be prepared for the worst scenario: detail panel + row grouping. In this case using the same icon for both doesn't work. We could change to "+" and "-".

Not sure what the best option is here

  1. Use the best icon and document on the doc how to change it in a "Use with the Tree Data or the Row Grouping" section
  2. Use a sub-optimal icon to make sure it is never incoherent

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 3, 2022
@m4theushw
Copy link
Member Author

@flaviendelangle I changed to a "+" and "-" icon. I don't agree that we should tell users to change the icon if they want to use master detail and tree data. Kendo UI uses the same combination that we're now using: arrow for grouping and "+" for master detail.

https://www.telerik.com/kendo-angular-ui/components/grid/master-detail/
https://www.telerik.com/kendo-angular-ui/components/grid/grouping/

@m4theushw m4theushw merged commit 6a77b23 into mui:master Feb 3, 2022
@m4theushw m4theushw deleted the master-detail branch February 3, 2022 15:21
Copy link
Member

@joserodolfofreitas joserodolfofreitas left a comment

Choose a reason for hiding this comment

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

It's looking great!

@oliviertassinari oliviertassinari added component: data grid This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user labels Feb 3, 2022
@corneliugaina
Copy link

Any update about this feature ? Will it be available in v5.x or the upcoming major release ?

@flaviendelangle
Copy link
Member

The feature has been available for quite some time
Here is the documentation

I would be interested to know why you did not find it

@corneliugaina
Copy link

Ok I'm glad it's released and available in datagrid pro, thanks you and mui team !
We were two searching for this and finally been able to find it by looking at released changes, then saw that a props was added to the feature recently (getDetailPanelHeight in 5.13).

For the story,

At first, I stumbled between every documentation page (Rows, Editing, Components) having in mind words like "Edit panel" / "Detail panel" (like in material-table ) and never thought to search in Group&Pivot sub-categories, in my opinion the name 'Master detail' is not very explicit (even if this makes sense now), and maybe should be in or 'Rows' or an independent page. Maybe I'm wrong and it really belongs to Group & Pivot category because it shares some UI parts.

Also, in mui table the equivalent feature has a different name, collapsible table .

Finally, It will be great if such feature appears on main demo , since we often look at it to describe a custom feature, based on what is already available on mui-x datagarid. But I understand that too much on demo can be overhelming, it's just supposed to be a demo not an exhaustive datagrid capabilities.

@flaviendelangle
Copy link
Member

flaviendelangle commented Jul 15, 2022

Thanks for your feedback !

@joserodolfofreitas I think that will interest you

I think there are two three things here:

  1. The "Master detail panel" is a feature named differently based on the project and it would be nice for the search to handle these aliases

  2. The doc page is in "Group & Pivot" which makes no sense, in [docs] Split Rows doc page #5195 I am moving it to "Rows"

  3. The main demo does not use this feature, every major feature should probably be visible without diving into the documentation (related to [docs] Improve main demo to show new functionalities #5292)

@niralivasoya
Copy link

@flaviendelangle @m4theushw, can we close the opened row detail when we click on the other row? Is this possible with detailPanelContent in DataGridPro?

@m4theushw
Copy link
Member Author

@niralivasoya Yes, you need to update the detailPanelExpandedRowIds prop to keep only one panel open when receiving a rowClick event. See https://mui.com/x/react-data-grid/master-detail/#controlling-expanded-detail-panels and https://mui.com/x/react-data-grid/events/#with-the-prop-of-the-event

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request plan: Pro Impact at least one Pro user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Implement Master detail/panel