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

Co-authored-by: Armin Mehinovic <armin@mui.com>

Author: alisasanib

packages/x-data-grid-pro/src/hooks/features/columnPinning/useGridColumnPinningPreProcessors.ts

@@ -3,7 +3,7 @@ import { RefObject } from '@mui/x-internals/types';
 import {
   GridPipeProcessor,
   useGridRegisterPipeProcessor,
-  gridVisiblePinnedColumnDefinitionsSelector,
+  gridExistingPinnedColumnSelector,
 } from '@mui/x-data-grid/internals';
 import { DataGridProProcessedProps } from '../../../models/dataGridProProps';
 import { GridPrivateApiPro } from '../../../models/gridApiPro';
@@ -30,13 +30,13 @@ export const useGridColumnPinningPreProcessors = (
       const savedState = apiRef.current.state;
       apiRef.current.state = { ...savedState, columns: columnsState as unknown as any };
 
-      const visibleColumns = gridVisiblePinnedColumnDefinitionsSelector(apiRef);
+      const pinnedColumns = gridExistingPinnedColumnSelector(apiRef);
 
       apiRef.current.state = savedState;
       // HACK: Ends here //
 
-      const leftPinnedColumns = visibleColumns.left.map((c) => c.field);
-      const rightPinnedColumns = visibleColumns.right.map((c) => c.field);
+      const leftPinnedColumns = pinnedColumns.left;
+      const rightPinnedColumns = pinnedColumns.right;
 
       let newOrderedFields: string[];
       const allPinnedColumns = [...leftPinnedColumns, ...rightPinnedColumns];

packages/x-data-grid-pro/src/tests/columnPinning.DataGridPro.test.tsx

@@ -720,4 +720,104 @@ describe('<DataGridPro /> - Column pinning', () => {
       expect(ageCellColumnGroupHeader.textContent).to.equal('Basic info');
     });
   });
+
+  describe('pinned columns order in column management', () => {
+    it('should keep pinned column order in column management panel when toggling columns', async () => {
+      const { user } = render(
+        <TestCase
+          rows={[{ id: 1, brand: 'Nike' }]}
+          columns={[{ field: 'id' }, { field: 'brand' }]}
+          showToolbar
+          initialState={{
+            pinnedColumns: {
+              left: ['brand'],
+            },
+          }}
+        />,
+      );
+
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+
+      const columnCheckboxes = screen.getAllByRole('checkbox');
+
+      expect(columnCheckboxes[0]).to.have.attribute('name', 'brand');
+      expect(columnCheckboxes[1]).to.have.attribute('name', 'id');
+
+      await user.click(columnCheckboxes[1]);
+
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+
+      const checkboxesAfterToggle = screen.getAllByRole('checkbox');
+
+      expect(checkboxesAfterToggle[0]).to.have.attribute('name', 'brand');
+      expect(checkboxesAfterToggle[1]).to.have.attribute('name', 'id');
+    });
+
+    it('should keep pinned column order in column management panel when clicking show/hide all checkbox', async () => {
+      const { user } = render(
+        <TestCase
+          rows={[{ id: 0, brand: 'Nike' }]}
+          columns={[{ field: 'id' }, { field: 'brand' }]}
+          showToolbar
+          initialState={{
+            pinnedColumns: {
+              left: ['brand'],
+            },
+          }}
+        />,
+      );
+
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+
+      const columnCheckboxes = screen.getAllByRole('checkbox');
+
+      expect(columnCheckboxes[0]).to.have.attribute('name', 'brand');
+      expect(columnCheckboxes[1]).to.have.attribute('name', 'id');
+
+      await user.click(columnCheckboxes[columnCheckboxes.length - 1]);
+
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+
+      const checkboxesAfterToggle = screen.getAllByRole('checkbox');
+      expect(checkboxesAfterToggle[0]).to.have.attribute('name', 'brand');
+      expect(checkboxesAfterToggle[1]).to.have.attribute('name', 'id');
+    });
+
+    it('should update column order when pinned columns are updated', async () => {
+      const { user } = render(
+        <TestCase
+          rows={[{ id: 0, brand: 'Nike', price: 100 }]}
+          columns={[{ field: 'id' }, { field: 'brand' }, { field: 'price' }]}
+          showToolbar
+          initialState={{
+            pinnedColumns: {},
+            columns: {
+              columnVisibilityModel: { id: false },
+            },
+          }}
+        />,
+      );
+
+      await user.click(screen.getByRole('button', { name: 'Columns' }));
+
+      const columnCheckboxes = screen.getAllByRole('checkbox');
+
+      expect(columnCheckboxes[0]).to.have.attribute('name', 'id');
+      expect(columnCheckboxes[1]).to.have.attribute('name', 'brand');
+      expect(columnCheckboxes[2]).to.have.attribute('name', 'price');
+
+      await act(() => {
+        apiRef.current?.pinColumn('brand', GridPinnedColumnPosition.LEFT);
+        apiRef.current?.pinColumn('id', GridPinnedColumnPosition.RIGHT);
+      });
+
+      const checkboxesAfterPinning = screen.getAllByRole('checkbox');
+
+      expect(checkboxesAfterPinning[0]).to.have.attribute('name', 'brand');
+      expect(checkboxesAfterPinning[1]).to.have.attribute('name', 'price');
+      expect(checkboxesAfterPinning[2]).to.have.attribute('name', 'id');
+    });
+  });
 });

packages/x-data-grid/src/hooks/features/columns/gridColumnsSelector.ts

@@ -96,6 +96,18 @@ export const gridPinnedColumnsSelector = createRootSelector(
   (state: GridStateCommunity) => state.pinnedColumns,
 );
 
+/**
+ * Get all existing pinned columns. Place the columns on the side that depends on the rtl state.
+ * @category Pinned Columns
+ * @ignore - Do not document
+ */
+export const gridExistingPinnedColumnSelector = createSelectorMemoized(
+  gridPinnedColumnsSelector,
+  gridColumnFieldsSelector,
+  gridIsRtlSelector,
+  (model, orderedFields, isRtl) => filterMissingColumns(model, orderedFields, isRtl),
+);
+
 /**
  * Get the visible pinned columns.
  * @category Visible Columns
@@ -106,7 +118,7 @@ export const gridVisiblePinnedColumnDefinitionsSelector = createSelectorMemoized
   gridVisibleColumnFieldsSelector,
   gridIsRtlSelector,
   (columnsState, model, visibleColumnFields, isRtl) => {
-    const visiblePinnedFields = filterVisibleColumns(model, visibleColumnFields, isRtl);
+    const visiblePinnedFields = filterMissingColumns(model, visibleColumnFields, isRtl);
     const visiblePinnedColumns = {
       left: visiblePinnedFields.left.map((field) => columnsState.lookup[field]),
       right: visiblePinnedFields.right.map((field) => columnsState.lookup[field]),
@@ -115,7 +127,7 @@ export const gridVisiblePinnedColumnDefinitionsSelector = createSelectorMemoized
   },
 );
 
-function filterVisibleColumns(
+function filterMissingColumns(
   pinnedColumns: GridPinnedColumnFields,
   columns: string[],
   invert?: boolean,