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 column pinning #2946

Merged
merged 23 commits into from
Dec 3, 2021
Merged

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Oct 23, 2021

Closes #193

Preview: https://deploy-preview-2946--material-ui-x.netlify.app/components/data-grid/columns/#column-pinning

Preview: https://deploy-preview-2946--material-ui-x.netlify.app/components/data-grid/demo/

How it was implemented

The virtualization is composed of 3 components: the viewport, the content and the render zone, where the rows are actually rendered. The render zone is also the part which is moved horizontally and vertically when the user scroll a certain amount. To render a determined set of rows and columns, a RenderContext object is given to getRows with the indexes to render. The idea behind the column pinning is to make the render zone to never render a pinned column, while the pinned columns are rendered outside of it with position: sticky. To achieve that, I changed the main render context to begin rendering after the last left pinned column and to end rendering before the first right pinned column. Between the render zone, two render contexts are created to render only the pinned columns. One problem of this approach is that for each visible row, there're can be 3 .MuiDataGrid-row elements, so the hover styles doesn't work properly. To fix that, I'm manually adding the .Mui-hovered class to the sibling row elements to make them appear as part of one single row.

Props added

  • pinnedColumns: controls which columns are pinned. It's an object of type: { left?: string[]; right?: string[] }.
  • onPinnedColumnsChange: fired whenever a column is pinned or unpinned. It receives an object containing the new pinned columns.
  • disableColumnPinning: allows to opt-out from the column pinning feature. It removes the buttons from the column menu
  • and throws an error if any API method is called while true
  • initialState.pinnedColumns: allows to initialize the grid with some pinned columns. This prop should be used to avoid having to add another onPinnedColumnsChange

Future work

  • Take screenshot of the print preview with pinned columns

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 26, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 28, 2021
@m4theushw m4theushw force-pushed the column-pinning branch 2 times, most recently from db31477 to 68d632a Compare November 2, 2021 03:05
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 2, 2021
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 4, 2021
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@mui mui deleted a comment from github-actions bot Nov 16, 2021
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 27, 2021
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 29, 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 Nov 29, 2021
@flaviendelangle
Copy link
Member

If you switch disableColumnPinning from true to false it does not stop pinning the column: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-fyqbe

If you pin a column both left and right, it pin it left and pin another random one right (the last one I suppose): https://codesandbox.io/s/datagridpro-v5-quick-start-forked-e6toc
I think it would be nice to at least warn when a column is registered in both. This clearly is an edge case and can be resolved later.

If you change the position of the pinned column it pins another-one and crashes: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-f9m8z
Not sure what the correct behavior is here. Should we throw when trying to set the index of a pinned column ?

Otherwise, I tried playing with the pinned columns (hide them / change their position / ...) and it behave fine.

@m4theushw
Copy link
Member Author

If you switch disableColumnPinning from true to false it does not stop pinning the column: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-fyqbe

The column pinning, different from the tree data, is opt-out. This prop is meant to allow users to completely disable the column pinning and have the grid as it was before this feature. I didn't invest time into making it to react to changes, it was created to be set initially and never changed.

If you pin a column both left and right, it pin it left and pin another random one right (the last one I suppose): https://codesandbox.io/s/datagridpro-v5-quick-start-forked-e6toc
I think it would be nice to at least warn when a column is registered in both. This clearly is an edge case and can be resolved later.

Fixed.

If you change the position of the pinned column it pins another-one and crashes: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-f9m8z

It was crashing when hovering a row, but this was because your CodeSandbox was using old code. Here's the updated one: https://codesandbox.io/s/datagridpro-v5-quick-start-forked-f9m8z?file=/src/App.tsx

I didn't fix the behavior of setColumnIndex because it's on the same columns hook and I don't want to put code related to column pinning there. Once this method is moved to a hook dedicated to column reorder I'll check there.

@flaviendelangle
Copy link
Member

It's good for me
I'll approve once the CI is green 👍

@m4theushw
Copy link
Member Author

Because of #3264 changed the argument passed to the hydrateColumns pre-processors, I added new gridVisibleColumnFieldsSelector selector to get only the field names of the visible columns.

@m4theushw m4theushw merged commit 2a2f1d3 into mui:master Dec 3, 2021
@m4theushw m4theushw deleted the column-pinning branch December 3, 2021 18:00
@DanailH
Copy link
Member

DanailH commented Dec 9, 2021

Demo for the release:
ezgif-3-13a2abdfb0b3

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

Successfully merging this pull request may close these issues.

[DataGrid] Implement Column pinning
4 participants