From 5f7ac4bdd4755f9fd8d2e6f3a2d25977cb679557 Mon Sep 17 00:00:00 2001 From: Rajat Date: Mon, 16 Dec 2024 19:20:50 +0530 Subject: [PATCH 1/5] Fix column unpinning with row grouping --- .../useGridRowGroupingPreProcessors.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts b/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts index ac43f038695ab..914ef499f534b 100644 --- a/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts +++ b/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts @@ -116,6 +116,10 @@ export const useGridRowGroupingPreProcessors = ( const groupingColDefs = getGroupingColDefs(columnsState); let newColumnFields: string[] = []; const newColumnsLookup: GridColumnRawLookup = {}; + const prevGroupingfields = columnsState.orderedFields.filter((field) => + isGroupingColumn(field), + ); + const currentGroupingfields: string[] = []; // We only keep the non-grouping columns columnsState.orderedFields.forEach((field) => { @@ -128,6 +132,7 @@ export const useGridRowGroupingPreProcessors = ( // We add the grouping column groupingColDefs.forEach((groupingColDef) => { const matchingGroupingColDef = columnsState.lookup[groupingColDef.field]; + currentGroupingfields.push(groupingColDef.field); if (matchingGroupingColDef) { groupingColDef.width = matchingGroupingColDef.width; groupingColDef.flex = matchingGroupingColDef.flex; @@ -135,14 +140,17 @@ export const useGridRowGroupingPreProcessors = ( newColumnsLookup[groupingColDef.field] = groupingColDef; }); - const startIndex = newColumnFields[0] === GRID_CHECKBOX_SELECTION_FIELD ? 1 : 0; - newColumnFields = [ - ...newColumnFields.slice(0, startIndex), - ...groupingColDefs.map((colDef) => colDef.field), - ...newColumnFields.slice(startIndex), - ]; - columnsState.orderedFields = newColumnFields; + if (prevGroupingfields.length !== currentGroupingfields.length) { + const startIndex = newColumnFields[0] === GRID_CHECKBOX_SELECTION_FIELD ? 1 : 0; + newColumnFields = [ + ...newColumnFields.slice(0, startIndex), + ...groupingColDefs.map((colDef) => colDef.field), + ...newColumnFields.slice(startIndex), + ]; + columnsState.orderedFields = newColumnFields; + } + columnsState.lookup = newColumnsLookup; return columnsState; From 8e20b56c3f437f68345f7767015c98eecd497120 Mon Sep 17 00:00:00 2001 From: Rajat Date: Mon, 16 Dec 2024 23:50:32 +0530 Subject: [PATCH 2/5] add tests --- .../rowGrouping.DataGridPremium.test.tsx | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx index f30b9e4374c7a..2c1058dba3547 100644 --- a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx +++ b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx @@ -2632,6 +2632,85 @@ describe(' - Row grouping', () => { }); }); + describe('column pinning', () => { + clock.withFakeTimers(); + + it('should work correctly with column pinning when groupingColumnMode = "single"', () => { + const { setProps } = render( + , + ); + expect(getColumnHeadersTextContent()).to.deep.equal([ + '', + 'category1', + 'id', + 'category1', + 'category2', + ]); + setProps({ pinnedColumns: { left: ['id'], right: ['__check__'] } }); + expect(getColumnHeadersTextContent()).to.deep.equal([ + 'id', + 'category1', + 'category1', + 'category2', + '', + ]); + setProps({ pinnedColumns: { left: [], right: [] } }); + expect(getColumnHeadersTextContent()).to.deep.equal([ + '', + 'category1', + 'id', + 'category1', + 'category2', + ]); + }); + + it('should work correctly with column pinning when when groupingColumnMode = "multiple"', () => { + const { setProps } = render( + , + ); + expect(getColumnHeadersTextContent()).to.deep.equal([ + '', + 'category1', + 'category2', + 'id', + 'category1', + 'category2', + ]); + setProps({ + pinnedColumns: { + left: ['__row_group_by_columns_group_category2__', 'id'], + right: ['__check__'], + }, + }); + expect(getColumnHeadersTextContent()).to.deep.equal([ + 'category2', + 'id', + 'category1', + 'category1', + 'category2', + '', + ]); + setProps({ pinnedColumns: { left: [], right: [] } }); + expect(getColumnHeadersTextContent()).to.deep.equal([ + '', + 'category1', + 'category2', + 'id', + 'category1', + 'category2', + ]); + }); + }); + describe('apiRef: addRowGroupingCriteria', () => { clock.withFakeTimers(); From b7b9114a7c3a99ade47228ebef53842f5f51068a Mon Sep 17 00:00:00 2001 From: Armin Mehinovic Date: Thu, 19 Dec 2024 20:56:19 +0100 Subject: [PATCH 3/5] Make the test fail with the original code --- .../rowGrouping.DataGridPremium.test.tsx | 51 +++++-------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx index 2c1058dba3547..58bcd182d3270 100644 --- a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx +++ b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx @@ -2632,10 +2632,8 @@ describe(' - Row grouping', () => { }); }); - describe('column pinning', () => { - clock.withFakeTimers(); - - it('should work correctly with column pinning when groupingColumnMode = "single"', () => { + describe.only('column pinning', () => { + it('should keep the checkbox selection column position after column is unpinned when groupingColumnMode = "single"', () => { const { setProps } = render( - Row grouping', () => { defaultGroupingExpansionDepth={-1} />, ); - expect(getColumnHeadersTextContent()).to.deep.equal([ - '', - 'category1', - 'id', - 'category1', - 'category2', - ]); - setProps({ pinnedColumns: { left: ['id'], right: ['__check__'] } }); + const initialColumnOrder = ['', 'category1', 'id', 'category1', 'category2']; + expect(getColumnHeadersTextContent()).to.deep.equal(initialColumnOrder); + setProps({ pinnedColumns: { left: ['id'] } }); expect(getColumnHeadersTextContent()).to.deep.equal([ 'id', - 'category1', - 'category1', - 'category2', - '', - ]); - setProps({ pinnedColumns: { left: [], right: [] } }); - expect(getColumnHeadersTextContent()).to.deep.equal([ '', 'category1', - 'id', 'category1', 'category2', ]); + setProps({ pinnedColumns: { left: [] } }); + expect(getColumnHeadersTextContent()).to.deep.equal(initialColumnOrder); }); - it('should work correctly with column pinning when when groupingColumnMode = "multiple"', () => { + it('should keep the checkbox selection column position after column is unpinned when groupingColumnMode = "multiple"', () => { const { setProps } = render( - Row grouping', () => { defaultGroupingExpansionDepth={-1} />, ); - expect(getColumnHeadersTextContent()).to.deep.equal([ - '', - 'category1', - 'category2', - 'id', - 'category1', - 'category2', - ]); + const initialColumnOrder = ['', 'category1', 'category2', 'id', 'category1', 'category2']; + expect(getColumnHeadersTextContent()).to.deep.equal(initialColumnOrder); setProps({ pinnedColumns: { left: ['__row_group_by_columns_group_category2__', 'id'], - right: ['__check__'], }, }); expect(getColumnHeadersTextContent()).to.deep.equal([ 'category2', 'id', - 'category1', - 'category1', - 'category2', - '', - ]); - setProps({ pinnedColumns: { left: [], right: [] } }); - expect(getColumnHeadersTextContent()).to.deep.equal([ '', 'category1', - 'category2', - 'id', 'category1', 'category2', ]); + setProps({ pinnedColumns: { left: [] } }); + expect(getColumnHeadersTextContent()).to.deep.equal(initialColumnOrder); }); }); From 68b9f2a9dc791afdaa27a0972a476a120e63c064 Mon Sep 17 00:00:00 2001 From: Armin Mehinovic Date: Thu, 19 Dec 2024 20:56:34 +0100 Subject: [PATCH 4/5] Simplify logic --- .../rowGrouping/useGridRowGroupingPreProcessors.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts b/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts index 914ef499f534b..435ac1ca1a7eb 100644 --- a/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts +++ b/packages/x-data-grid-premium/src/hooks/features/rowGrouping/useGridRowGroupingPreProcessors.ts @@ -116,14 +116,13 @@ export const useGridRowGroupingPreProcessors = ( const groupingColDefs = getGroupingColDefs(columnsState); let newColumnFields: string[] = []; const newColumnsLookup: GridColumnRawLookup = {}; - const prevGroupingfields = columnsState.orderedFields.filter((field) => - isGroupingColumn(field), - ); - const currentGroupingfields: string[] = []; + const prevGroupingfields: string[] = []; // We only keep the non-grouping columns columnsState.orderedFields.forEach((field) => { - if (!isGroupingColumn(field)) { + if (isGroupingColumn(field)) { + prevGroupingfields.push(field); + } else { newColumnFields.push(field); newColumnsLookup[field] = columnsState.lookup[field]; } @@ -132,7 +131,6 @@ export const useGridRowGroupingPreProcessors = ( // We add the grouping column groupingColDefs.forEach((groupingColDef) => { const matchingGroupingColDef = columnsState.lookup[groupingColDef.field]; - currentGroupingfields.push(groupingColDef.field); if (matchingGroupingColDef) { groupingColDef.width = matchingGroupingColDef.width; groupingColDef.flex = matchingGroupingColDef.flex; @@ -141,7 +139,7 @@ export const useGridRowGroupingPreProcessors = ( newColumnsLookup[groupingColDef.field] = groupingColDef; }); - if (prevGroupingfields.length !== currentGroupingfields.length) { + if (prevGroupingfields.length !== groupingColDefs.length) { const startIndex = newColumnFields[0] === GRID_CHECKBOX_SELECTION_FIELD ? 1 : 0; newColumnFields = [ ...newColumnFields.slice(0, startIndex), From 3e30f3d30314b0da9d0a067433e806e815c1a499 Mon Sep 17 00:00:00 2001 From: Armin Mehinovic Date: Thu, 19 Dec 2024 20:57:25 +0100 Subject: [PATCH 5/5] Remove only test --- .../src/tests/rowGrouping.DataGridPremium.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx index 58bcd182d3270..6a0979cff342c 100644 --- a/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx +++ b/packages/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx @@ -2632,7 +2632,7 @@ describe(' - Row grouping', () => { }); }); - describe.only('column pinning', () => { + describe('column pinning', () => { it('should keep the checkbox selection column position after column is unpinned when groupingColumnMode = "single"', () => { const { setProps } = render(