Skip to content

[DataGridPro] Fix pinned columns order in column management #17950

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

Merged
merged 8 commits into from
May 29, 2025

Conversation

alisasanib
Copy link
Contributor

@alisasanib alisasanib commented May 21, 2025

Fixes #17889

The column management section was not respecting order of pinned columns when users were changing visibility of columns. As a result this user interaction led to change in the order of the columns in column panel.

This fix ensures column management section keeps the same order as the pinned columns in the grid.

Copy link

github-actions bot commented May 21, 2025

Thanks for adding a type label to the PR! 👍

@mui-bot
Copy link

mui-bot commented May 21, 2025

Deploy preview: https://deploy-preview-17950--material-ui-x.netlify.app/

Bundle size report

Total Size Change:${\tiny{\color{red}▲}}$+56B(0.00%) - Total Gzip Change:${\tiny{\color{red}▲}}$+116B(0.00%)
Files: 118 total (0 added, 0 removed, 6 changed)

Show 6 more bundle changes

@mui/x-data-gridparsed:${\tiny{\color{red}▲}}$+33B(+0.01%) gzip:${\tiny{\color{red}▲}}$+27B(+0.02%)
@mui/x-data-grid/DataGridparsed:${\tiny{\color{red}▲}}$+33B(+0.01%) gzip:${\tiny{\color{red}▲}}$+20B(+0.02%)
@mui/x-data-grid-premiumparsed:${\tiny{\color{green}▼}}$-3B(0.00%) gzip:${\tiny{\color{red}▲}}$+17B(+0.01%)
@mui/x-data-grid-proparsed:${\tiny{\color{green}▼}}$-3B(0.00%) gzip: 0B(0.00%)
@mui/x-data-grid-premium/DataGridPremiumparsed:${\tiny{\color{green}▼}}$-2B(0.00%) gzip:${\tiny{\color{red}▲}}$+16B(+0.01%)
@mui/x-data-grid-pro/DataGridProparsed:${\tiny{\color{green}▼}}$-2B(0.00%) gzip:${\tiny{\color{red}▲}}$+36B(+0.03%)

Details of bundle changes

Generated by 🚫 dangerJS against bd1c94e

@alisasanib alisasanib changed the title Fix column management order [DataGridPro] Fix pinned columns order in column management May 21, 2025
@alisasanib alisasanib marked this pull request as ready for review May 21, 2025 18:06
@KenanYusuf KenanYusuf requested a review from a team May 22, 2025 08:26
@zannager zannager added the scope: data grid Changes or issues related to the data grid product label May 23, 2025
@KenanYusuf KenanYusuf added type: bug A bug or unintended behavior in the components. feature: Column pinning Related to the data grid Column pinning feature labels May 27, 2025
@arminmeh
Copy link
Contributor

Hi @alisasanib
Thank you for this pull request!

Your changes seem to impact the commercially licensed code. For any changes of this nature, we require contributors to sign the MUI’s Contributor License Agreement (CLA). However, I can’t find a CLA signed that could cover these changes.

Please follow the steps at mui-org.notion.site/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 to sign the document.

@arminmeh arminmeh added the CLA: required See https://www.notion.so/mui-org/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 label May 27, 2025
@alisasanib
Copy link
Contributor Author

Hi @alisasanib Thank you for this pull request!

Your changes seem to impact the commercially licensed code. For any changes of this nature, we require contributors to sign the MUI’s Contributor License Agreement (CLA). However, I can’t find a CLA signed that could cover these changes.

Please follow the steps at mui-org.notion.site/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 to sign the document.

@arminmeh Done! I just forwarded the confirmation email to [email protected]

@zannager zannager added CLA: signed See https://www.notion.so/mui-org/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 and removed CLA: required See https://www.notion.so/mui-org/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 labels May 27, 2025
@zannager
Copy link
Member

@arminmeh CLA signed, thanks.

@arminmeh arminmeh self-assigned this May 27, 2025
@arminmeh arminmeh added needs cherry-pick The PR should be auto-cherry-picked to a version branch after merge—add a version label as target v7.x labels May 27, 2025
@arminmeh
Copy link
Contributor

@alisasanib I have checked and updated your PR

  • Instead of changing the selector, I have changed the way pinning pre-processor works. It includes hidden columns now in the ordered columns state
  • I have changed one of the tests to cover the pinning state updates. Reset button goes through the same flow as the click on an item, so I think that we don't need that one anymore.

Since I have also made a change I would ask someone else from @mui/xgrid to approve this.

@arminmeh arminmeh removed their assignment May 27, 2025
@arminmeh arminmeh force-pushed the fix-column-management-order branch from 8b7db65 to b141324 Compare May 27, 2025 12:45
Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @alisasanib and @arminmeh for the fix.

@MBilalShafi MBilalShafi merged commit 29a7b6c into mui:master May 29, 2025
23 checks passed
Copy link

Cherry-pick PRs will be created targeting branches: v7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: signed See https://www.notion.so/mui-org/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0 feature: Column pinning Related to the data grid Column pinning feature needs cherry-pick The PR should be auto-cherry-picked to a version branch after merge—add a version label as target scope: data grid Changes or issues related to the data grid product type: bug A bug or unintended behavior in the components. v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Column management dialog: pinned column's visibility toggled: jumps back and forth
6 participants