Skip to content

Commit 2985131

Browse files
Copilotkdinevhanastasov
authored
Fix column width issues in grid state persistence (#16901)
* Initial plan * Fix column width issue when all columns are hidden in grid state - Prevent setting _columnWidth to "0px" when all columns are hidden - Preserve valid column widths for when columns are unhidden - Add test to verify column widths are preserved after state restoration Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * Update test to properly validate column width preservation - Adjust test expectations to handle columns constrained by minWidth - Verify that columns don't all end up with the same minimum width - Test passes successfully Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * Update column-hiding test to reflect correct behavior - Update test expectation - column width should not be 0px when all columns hidden - This allows proper width restoration when columns are unhidden - All column hiding tests pass successfully Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * Address code review feedback - Use explicit check for '0px' instead of parseFloat for clarity - Remove hardcoded width assertion in test for better maintainability - All tests still pass successfully Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * Refactor to eliminate code duplication - Extract _updateColumnDefaultWidths helper method - Remove redundant condition check - Improve code maintainability - All tests still pass Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * Fix: Only persist user-set column widths in grid state - Only save width to state if widthSetByUser or columnWidthSetByUser is true - Prevents auto-calculated widths from becoming fixed after state restore - Handle width restoration to skip if undefined - All tests pass Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> * test(pivot grid state): fixing test to not persist calculated column width * refactor: Extract duplicated width restoration logic into helper method - Extract restoreColumnState helper to avoid code duplication - Handles width extraction, assignment, and conditional restoration - Used for both column groups and regular columns - All tests pass Co-authored-by: hanastasov <14248932+hanastasov@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kdinev <1472513+kdinev@users.noreply.github.com> Co-authored-by: Konstantin Dinev <kdinev@infragistics.com> Co-authored-by: hanastasov <14248932+hanastasov@users.noreply.github.com> Co-authored-by: Hristo Anastasov <hanastasov@infragistics.com>
1 parent 4dd351e commit 2985131

5 files changed

Lines changed: 91 additions & 12 deletions

File tree

‎projects/igniteui-angular/grids/core/src/state-base.directive.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export class IgxGridStateBaseDirective {
203203
dataType: c.dataType,
204204
hasSummary: c.hasSummary,
205205
field: c.field,
206-
width: c.width,
206+
width: ((c as IgxColumnComponent).widthSetByUser || context.currGrid.columnWidthSetByUser) ? c.width : undefined,
207207
header: c.header,
208208
resizable: c.resizable,
209209
searchable: c.searchable,
@@ -226,6 +226,21 @@ export class IgxGridStateBaseDirective {
226226
},
227227
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IColumnState[]): void => {
228228
const newColumns = [];
229+
230+
// Helper to restore column state without auto-persisting widths
231+
const restoreColumnState = (column: IgxColumnComponent | IgxColumnGroupComponent, colState: IColumnState) => {
232+
// Extract width to handle it separately
233+
const width = colState.width;
234+
delete colState.width;
235+
236+
Object.assign(column, colState);
237+
238+
// Only restore width if it was explicitly set by the user (not undefined)
239+
if (width !== undefined) {
240+
column.width = width;
241+
}
242+
};
243+
229244
state.forEach((colState) => {
230245
const hasColumnGroup = colState.columnGroup;
231246
const hasColumnLayouts = colState.columnLayout;
@@ -242,7 +257,9 @@ export class IgxGridStateBaseDirective {
242257
} else {
243258
ref1.children.reset([]);
244259
}
245-
Object.assign(ref1, colState);
260+
261+
restoreColumnState(ref1, colState);
262+
246263
ref1.grid = context.currGrid;
247264
if (colState.parent || colState.parentKey) {
248265
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref1.parent));
@@ -259,7 +276,8 @@ export class IgxGridStateBaseDirective {
259276
component.changeDetectorRef.detectChanges();
260277
}
261278

262-
Object.assign(ref, colState);
279+
restoreColumnState(ref, colState);
280+
263281
ref.grid = context.currGrid;
264282
if (colState.parent || colState.parentKey) {
265283
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref.parent));

‎projects/igniteui-angular/grids/core/src/state.directive.spec.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,55 @@ describe('IgxGridState - input properties #grid', () => {
802802
expect(prodIdColumn.colStart).toBe(1);
803803
expect(prodIdColumn.colEnd).toBe(1);
804804
});
805+
806+
it('should preserve column widths when restoring state with all columns hidden', () => {
807+
const fix = TestBed.createComponent(IgxGridStateComponent);
808+
fix.detectChanges();
809+
const grid = fix.componentInstance.grid;
810+
const state = fix.componentInstance.state;
811+
812+
// Store initial column widths - don't hardcode expected values
813+
const initialWidths = grid.columns.map(col => col.width);
814+
815+
// Hide all columns
816+
grid.columns.forEach(col => col.hidden = true);
817+
fix.detectChanges();
818+
819+
// Verify all columns are hidden
820+
expect(grid.columns.every(col => col.hidden)).toBe(true);
821+
822+
// Get and save the state with all columns hidden
823+
const gridState = state.getState(false) as IGridState;
824+
expect(gridState.columns.every(col => col.hidden)).toBe(true);
825+
826+
// Restore the state
827+
state.setState(gridState);
828+
fix.detectChanges();
829+
830+
// Verify all columns are still hidden
831+
expect(grid.columns.every(col => col.hidden)).toBe(true);
832+
833+
// Unhide all columns
834+
grid.columns.forEach(col => col.hidden = false);
835+
fix.detectChanges();
836+
837+
// Verify column widths are preserved and not set to minimum
838+
// The issue was that when all columns were hidden, _columnWidth would be set to "0px"
839+
// and all columns would end up with minimum width when unhidden
840+
grid.columns.forEach((col, index) => {
841+
expect(col.width).toBe(initialWidths[index], `Column ${index} width should be preserved`);
842+
// The calcWidth should be based on the column width or grid default, not forced to 0px
843+
const calcWidth = parseFloat(col.calcWidth);
844+
// Note: some columns may be constrained by minWidth which is expected
845+
// The key is they shouldn't all be the same minimum width
846+
expect(calcWidth).toBeGreaterThan(0, `Column ${index} calcWidth should be greater than 0`);
847+
});
848+
849+
// Verify that not all columns have the same width (which would indicate the bug)
850+
const calcWidths = grid.columns.map(col => parseFloat(col.calcWidth));
851+
const allSameWidth = calcWidths.every(w => w === calcWidths[0]);
852+
expect(allSameWidth).toBe(false, 'Columns should not all have the same width');
853+
});
805854
});
806855

807856
class HelperFunctions {

‎projects/igniteui-angular/grids/core/src/state.pivotgrid.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ describe('IgxPivotGridState #pivotGrid :', () => {
3434
const jsonString = state.getState(true);
3535
const expectedObj = {
3636
"columns": [
37-
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Bulgaria", "width": "220px", "header": "Bulgaria", "resizable": false, "searchable": true, "selectable": true, "key": "Bulgaria", "columnGroup": false, "disableHiding": false, "disablePinning": false },
38-
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "US", "width": "220px", "header": "US", "resizable": false, "searchable": true, "selectable": true, "key": "US", "columnGroup": false, "disableHiding": false, "disablePinning": false },
39-
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Uruguay", "width": "220px", "header": "Uruguay", "resizable": false, "searchable": true, "selectable": true, "key": "Uruguay", "columnGroup": false, "disableHiding": false, "disablePinning": false },
40-
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "UK", "width": "220px", "header": "UK", "resizable": false, "searchable": true, "selectable": true, "key": "UK", "columnGroup": false, "disableHiding": false, "disablePinning": false },
41-
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Japan", "width": "220px", "header": "Japan", "resizable": false, "searchable": true, "selectable": true, "key": "Japan", "columnGroup": false, "disableHiding": false, "disablePinning": false }
37+
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Bulgaria", "header": "Bulgaria", "resizable": false, "searchable": true, "selectable": true, "key": "Bulgaria", "columnGroup": false, "disableHiding": false, "disablePinning": false },
38+
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "US", "header": "US", "resizable": false, "searchable": true, "selectable": true, "key": "US", "columnGroup": false, "disableHiding": false, "disablePinning": false },
39+
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Uruguay", "header": "Uruguay", "resizable": false, "searchable": true, "selectable": true, "key": "Uruguay", "columnGroup": false, "disableHiding": false, "disablePinning": false },
40+
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "UK", "header": "UK", "resizable": false, "searchable": true, "selectable": true, "key": "UK", "columnGroup": false, "disableHiding": false, "disablePinning": false },
41+
{ "pinned": false, "sortable": true, "filterable": true, "sortingIgnoreCase": true, "filteringIgnoreCase": true, "headerClasses": "", "headerGroupClasses": "", "groupable": false, "hidden": false, "dataType": "number", "hasSummary": false, "field": "Japan", "header": "Japan", "resizable": false, "searchable": true, "selectable": true, "key": "Japan", "columnGroup": false, "disableHiding": false, "disablePinning": false }
4242
],
4343
"filtering": { "filteringOperands": [], "operator": 0, "type": 0 },
4444
"advancedFiltering": {}, "sorting": [], "cellSelection": [], "rowSelection": [], "columnSelection": [],

‎projects/igniteui-angular/grids/grid/src/column-hiding.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,8 +536,10 @@ describe('Column Hiding UI #grid', () => {
536536
grid.columnList.forEach((col) => col.hidden = true);
537537
tick(30);
538538
fix.detectChanges();
539+
// Column widths should be preserved when all columns are hidden
540+
// This allows proper width restoration when columns are unhidden
539541
grid.columnList.forEach((col) => {
540-
expect(col.width).toBe('0px');
542+
expect(col.width).not.toBe('0px', 'Column width should not be 0px when hidden');
541543
});
542544
fixEl = fix.nativeElement;
543545
gridEl = grid.nativeElement;

‎projects/igniteui-angular/grids/grid/src/grid-base.directive.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6796,23 +6796,33 @@ export abstract class IgxGridBaseDirective implements GridType,
67966796
const possibleWidth = this.getPossibleColumnWidth();
67976797
if (possibleWidth === "0px") {
67986798
// all columns - hidden
6799-
this._columnWidth = possibleWidth;
6799+
// Do not update _columnWidth to preserve valid column widths for when columns are unhidden
6800+
// Only update column defaultWidth if _columnWidth is already set and not '0px'
6801+
if (this._columnWidth && this._columnWidth !== '0px') {
6802+
this._updateColumnDefaultWidths();
6803+
}
6804+
this.resetCachedWidths();
6805+
return;
68006806
} else if (this.width !== null) {
68016807
this._columnWidth = Math.max(parseFloat(possibleWidth), this.minColumnWidth) + 'px'
68026808
} else {
68036809
this._columnWidth = this.minColumnWidth + 'px';
68046810
}
68056811
}
6812+
this._updateColumnDefaultWidths();
6813+
this.resetCachedWidths();
6814+
}
6815+
6816+
private _updateColumnDefaultWidths() {
68066817
this._columns.forEach((column: IgxColumnComponent) => {
6807-
if (this.hasColumnLayouts && parseFloat(this._columnWidth)) {
6818+
if (this.hasColumnLayouts) {
68086819
const columnWidthCombined = parseFloat(this._columnWidth) * (column.colEnd ? column.colEnd - column.colStart : 1);
68096820
column.defaultWidth = columnWidthCombined + 'px';
68106821
} else {
68116822
column.defaultWidth = this._columnWidth;
68126823
column.resetCaches();
68136824
}
68146825
});
6815-
this.resetCachedWidths();
68166826
}
68176827

68186828
protected resetNotifyChanges() {

0 commit comments

Comments
 (0)