From 14e1e2ebeac1bd557bd0940f081e2082804305c7 Mon Sep 17 00:00:00 2001 From: Nadia Robakova Date: Tue, 24 Sep 2019 11:14:10 +0300 Subject: [PATCH 1/4] fix(grid): update group children when add/delete a column #5837 --- .../src/lib/grids/column.component.ts | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/column.component.ts b/projects/igniteui-angular/src/lib/grids/column.component.ts index 2d52780d302..70c787b4c5a 100644 --- a/projects/igniteui-angular/src/lib/grids/column.component.ts +++ b/projects/igniteui-angular/src/lib/grids/column.component.ts @@ -8,7 +8,8 @@ import { Input, QueryList, TemplateRef, - forwardRef + forwardRef, + OnDestroy } from '@angular/core'; import { DataType } from '../data-operations/data-util'; import { GridBaseAPIService } from './api.service'; @@ -33,6 +34,8 @@ import { DeprecateProperty } from '../core/deprecateDecorators'; import { MRLColumnSizeInfo, MRLResizeColumnInfo } from '../data-operations/multi-row-layout.interfaces'; import { DisplayDensity } from '../core/displayDensity'; import { notifyChanges } from './watch-changes'; +import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; import { IgxCellTemplateDirective, IgxCellHeaderTemplateDirective, @@ -1646,7 +1649,8 @@ export class IgxColumnComponent implements AfterContentInit { selector: 'igx-column-group', template: `` }) -export class IgxColumnGroupComponent extends IgxColumnComponent implements AfterContentInit { +export class IgxColumnGroupComponent extends IgxColumnComponent implements AfterContentInit, OnDestroy { + private destroy$ = new Subject(); @ContentChildren(IgxColumnComponent, { read: IgxColumnComponent }) children = new QueryList(); @@ -1774,11 +1778,22 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After if (this.headTemplate && this.headTemplate.length) { this._headerTemplate = this.headTemplate.toArray()[0].template; } - this.children.reset(this.children.toArray().slice(1)); - this.children.forEach(child => { - child.parent = this; + this.updateChildren(); + this.children.changes.pipe(takeUntil(this.destroy$)) + .subscribe(() => { + this.updateChildren(); }); } + + protected updateChildren() { + if (this.children && this.children.toArray().length > 0 && this.children.toArray()[0] === this) { + this.children.reset(this.children.toArray().slice(1)); + this.children.forEach(child => { + child.parent = this; + }); + } + } + /** * Returns the children columns collection. * ```typescript @@ -1836,6 +1851,14 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After // D.P. constructor duplication due to es6 compilation, might be obsolete in the future super(gridAPI, cdr); } + + /** + * @hidden + */ + public ngOnDestroy() { + this.destroy$.next(true); + this.destroy$.complete(); + } } @Component({ @@ -1980,5 +2003,4 @@ export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements this.childrenVisibleIndexes.push({column: child, index: vIndex}); }); } - } From 679d08670ec0fc43655f27e21a722601d91dd0cf Mon Sep 17 00:00:00 2001 From: Nadia Robakova Date: Tue, 24 Sep 2019 15:32:05 +0300 Subject: [PATCH 2/4] fix(grid): update children when add or remove column in column group #5837 --- .../src/lib/grids/column.component.ts | 82 +++++++++---------- .../src/lib/grids/grid/column-group.spec.ts | 33 ++++++++ 2 files changed, 74 insertions(+), 41 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/column.component.ts b/projects/igniteui-angular/src/lib/grids/column.component.ts index 70c787b4c5a..fe01105dff1 100644 --- a/projects/igniteui-angular/src/lib/grids/column.component.ts +++ b/projects/igniteui-angular/src/lib/grids/column.component.ts @@ -57,7 +57,7 @@ import { selector: 'igx-column', template: `` }) -export class IgxColumnComponent implements AfterContentInit { +export class IgxColumnComponent implements AfterContentInit, OnDestroy { /** * Sets/gets the `field` value. * ```typescript @@ -838,13 +838,13 @@ export class IgxColumnComponent implements AfterContentInit { return false; } - /** - * Returns a boolean indicating if the column is a child of a `ColumnLayout` for multi-row layout. - * ```typescript - * let columnLayoutChild = this.column.columnLayoutChild; - * ``` - * @memberof IgxColumnComponent - */ + /** + * Returns a boolean indicating if the column is a child of a `ColumnLayout` for multi-row layout. + * ```typescript + * let columnLayoutChild = this.column.columnLayoutChild; + * ``` + * @memberof IgxColumnComponent + */ get columnLayoutChild() { return this.parent && this.parent.columnLayout; } @@ -979,6 +979,11 @@ export class IgxColumnComponent implements AfterContentInit { * @memberof IgxColumnComponent */ children: QueryList; + /** + * @hidden + */ + protected destroy$ = new Subject(); + /** *@hidden */ @@ -1162,7 +1167,7 @@ export class IgxColumnComponent implements AfterContentInit { if (!col.colStart) { return; } - const newWidthSet = col.widthSetByUser && columnSizes[col.colStart - 1] && !columnSizes[col.colStart - 1].widthSetByUser; + const newWidthSet = col.widthSetByUser && columnSizes[col.colStart - 1] && !columnSizes[col.colStart - 1].widthSetByUser; const newSpanSmaller = columnSizes[col.colStart - 1] && columnSizes[col.colStart - 1].colSpan > col.gridColumnSpan; const bothWidthsSet = col.widthSetByUser && columnSizes[col.colStart - 1] && columnSizes[col.colStart - 1].widthSetByUser; const bothWidthsNotSet = !col.widthSetByUser && columnSizes[col.colStart - 1] && !columnSizes[col.colStart - 1].widthSetByUser; @@ -1234,8 +1239,8 @@ export class IgxColumnComponent implements AfterContentInit { for (; j < columnSizes[i].colSpan && i + j + 1 < columnSizes[i].colEnd; j++) { if (columnSizes[i + j] && ((!columnSizes[i].width && columnSizes[i + j].width) || - (!columnSizes[i].width && !columnSizes[i + j].width && columnSizes[i + j].colSpan <= columnSizes[i].colSpan) || - (!!columnSizes[i + j].width && columnSizes[i + j].colSpan <= columnSizes[i].colSpan))) { + (!columnSizes[i].width && !columnSizes[i + j].width && columnSizes[i + j].colSpan <= columnSizes[i].colSpan) || + (!!columnSizes[i + j].width && columnSizes[i + j].colSpan <= columnSizes[i].colSpan))) { // If we reach an already defined column that has width and the current doesn't have or // if the reached column has bigger colSpan we stop. break; @@ -1283,8 +1288,8 @@ export class IgxColumnComponent implements AfterContentInit { } protected getColumnSizesString(children: QueryList): string { - const res = this.getFilledChildColumnSizes(children); - return res.join(' '); + const res = this.getFilledChildColumnSizes(children); + return res.join(' '); } public getResizableColUnderEnd(): MRLResizeColumnInfo[] { @@ -1298,7 +1303,7 @@ export class IgxColumnComponent implements AfterContentInit { for (let i = 0; i < columnSized.length; i++) { if (this.colStart <= i + 1 && i + 1 < colEnd) { - targets.push({ target: columnSized[i].ref, spanUsed: 1}); + targets.push({ target: columnSized[i].ref, spanUsed: 1 }); } } @@ -1377,7 +1382,7 @@ export class IgxColumnComponent implements AfterContentInit { grid.resetCaches(); grid.notifyChanges(); if (this.columnLayoutChild) { - this.grid.columns.filter(x => x.columnLayout).forEach( x => x.populateVisibleIndexes()); + this.grid.columns.filter(x => x.columnLayout).forEach(x => x.populateVisibleIndexes()); } this.grid.filteringService.refreshExpressions(); // this.grid.refreshSearch(true); @@ -1440,7 +1445,7 @@ export class IgxColumnComponent implements AfterContentInit { grid.notifyChanges(); if (this.columnLayoutChild) { - this.grid.columns.filter(x => x.columnLayout).forEach( x => x.populateVisibleIndexes()); + this.grid.columns.filter(x => x.columnLayout).forEach(x => x.populateVisibleIndexes()); } this.grid.filteringService.refreshExpressions(); // this.grid.refreshSearch(true); @@ -1640,6 +1645,14 @@ export class IgxColumnComponent implements AfterContentInit { * @hidden */ public populateVisibleIndexes() { } + + /** + * @hidden + */ + public ngOnDestroy() { + this.destroy$.next(true); + this.destroy$.complete(); + } } @@ -1650,8 +1663,6 @@ export class IgxColumnComponent implements AfterContentInit { template: `` }) export class IgxColumnGroupComponent extends IgxColumnComponent implements AfterContentInit, OnDestroy { - private destroy$ = new Subject(); - @ContentChildren(IgxColumnComponent, { read: IgxColumnComponent }) children = new QueryList(); /** @@ -1778,20 +1789,17 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After if (this.headTemplate && this.headTemplate.length) { this._headerTemplate = this.headTemplate.toArray()[0].template; } - this.updateChildren(); - this.children.changes.pipe(takeUntil(this.destroy$)) - .subscribe(() => { - this.updateChildren(); + this.children.reset(this.children.toArray().slice(1)); + this.children.forEach(child => { + child.parent = this; }); - } - - protected updateChildren() { - if (this.children && this.children.toArray().length > 0 && this.children.toArray()[0] === this) { + this.children.changes.pipe(takeUntil(this.destroy$)) + .subscribe(() => { this.children.reset(this.children.toArray().slice(1)); this.children.forEach(child => { child.parent = this; }); - } + }); } /** @@ -1838,7 +1846,7 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After return acc; } if (typeof val.width === 'string' && val.width.indexOf('%') !== -1) { - isChildrenWidthInPercent = true; + isChildrenWidthInPercent = true; } return acc + parseInt(val.width, 10); }, 0)}`; @@ -1851,14 +1859,6 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After // D.P. constructor duplication due to es6 compilation, might be obsolete in the future super(gridAPI, cdr); } - - /** - * @hidden - */ - public ngOnDestroy() { - this.destroy$.next(true); - this.destroy$.complete(); - } } @Component({ @@ -1867,7 +1867,7 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After selector: 'igx-column-layout', template: `` }) -export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements AfterContentInit { +export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements AfterContentInit, OnDestroy { public childrenVisibleIndexes = []; /** * Gets the width of the column layout. @@ -1952,7 +1952,7 @@ export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements this.children.forEach(child => child.hidden = value); if (this.grid && this.grid.columns && this.grid.columns.length > 0) { // reset indexes in case columns are hidden/shown runtime - this.grid.columns.filter(x => x.columnGroup).forEach( x => x.populateVisibleIndexes()); + this.grid.columns.filter(x => x.columnGroup).forEach(x => x.populateVisibleIndexes()); } } @@ -1991,8 +1991,8 @@ export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements const grid = this.gridAPI.grid; const columns = grid && grid.pinnedColumns && grid.unpinnedColumns ? grid.pinnedColumns.concat(grid.unpinnedColumns) : []; const orderedCols = columns - .filter(x => !x.columnGroup && !x.hidden) - .sort((a, b) => a.rowStart - b.rowStart || columns.indexOf(a.parent) - columns.indexOf(b.parent) || a.colStart - b.colStart); + .filter(x => !x.columnGroup && !x.hidden) + .sort((a, b) => a.rowStart - b.rowStart || columns.indexOf(a.parent) - columns.indexOf(b.parent) || a.colStart - b.colStart); this.children.forEach(child => { const rs = child.rowStart || 1; let vIndex = 0; @@ -2000,7 +2000,7 @@ export class IgxColumnLayoutComponent extends IgxColumnGroupComponent implements const cols = orderedCols.filter(c => !c.columnGroup && (c.rowStart || 1) <= rs); vIndex = cols.indexOf(child); - this.childrenVisibleIndexes.push({column: child, index: vIndex}); + this.childrenVisibleIndexes.push({ column: child, index: vIndex }); }); } } diff --git a/projects/igniteui-angular/src/lib/grids/grid/column-group.spec.ts b/projects/igniteui-angular/src/lib/grids/grid/column-group.spec.ts index a3f39929649..a8422a0f2ad 100644 --- a/projects/igniteui-angular/src/lib/grids/grid/column-group.spec.ts +++ b/projects/igniteui-angular/src/lib/grids/grid/column-group.spec.ts @@ -1553,6 +1553,39 @@ describe('IgxGrid - multi-column headers #grid', () => { expect(firstColumnGroup.header).toEqual(expectedColumnName); expect(expectedColumnListLength).toEqual(columnLength); }); + + it('There shouldn\'t be any errors when dynamically removing or adding a column in column group', () => { + const fixture = TestBed.createComponent(DynamicColGroupsGridComponent); + fixture.detectChanges(); + + const grid = fixture.componentInstance.grid; + + expect(grid.columnList.length).toEqual(10); + + expect(() => { + // Delete column + fixture.componentInstance.columnGroups[0].columns.splice(0, 1); + fixture.detectChanges(); + }).not.toThrow(); + + expect(grid.columnList.length).toEqual(9); + + expect(() => { + // Add column + fixture.componentInstance.columnGroups[0].columns.push({ field: 'Fax', type: 'string' }); + fixture.detectChanges(); + }).not.toThrow(); + + expect(grid.columnList.length).toEqual(10); + + expect(() => { + // Update column + fixture.componentInstance.columnGroups[0].columns[1] = { field: 'City', type: 'string' }; + fixture.detectChanges(); + }).not.toThrow(); + + expect(grid.columnList.length).toEqual(10); + }); }); @Component({ From a948eb7f38ff3224b9c5afde72dd96ba388de71f Mon Sep 17 00:00:00 2001 From: Nadia Robakova Date: Tue, 24 Sep 2019 16:25:20 +0300 Subject: [PATCH 3/4] fix(grid): add check to remove first child, only when it is needed #5837 --- .../igniteui-angular/src/lib/grids/column.component.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/column.component.ts b/projects/igniteui-angular/src/lib/grids/column.component.ts index fe01105dff1..e76e9b469eb 100644 --- a/projects/igniteui-angular/src/lib/grids/column.component.ts +++ b/projects/igniteui-angular/src/lib/grids/column.component.ts @@ -1795,10 +1795,12 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After }); this.children.changes.pipe(takeUntil(this.destroy$)) .subscribe(() => { - this.children.reset(this.children.toArray().slice(1)); - this.children.forEach(child => { - child.parent = this; - }); + if (this.children && this.children.toArray().length > 0 && this.children.toArray()[0] === this) { + this.children.reset(this.children.toArray().slice(1)); + this.children.forEach(child => { + child.parent = this; + }); + } }); } From aded0db1da76bb728f8586adc96eaae70d7a5809 Mon Sep 17 00:00:00 2001 From: Nadia Robakova Date: Wed, 25 Sep 2019 13:44:30 +0300 Subject: [PATCH 4/4] chore(*): use changes of children instead of children --- .../igniteui-angular/src/lib/grids/column.component.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/projects/igniteui-angular/src/lib/grids/column.component.ts b/projects/igniteui-angular/src/lib/grids/column.component.ts index e76e9b469eb..a8b3a5031b3 100644 --- a/projects/igniteui-angular/src/lib/grids/column.component.ts +++ b/projects/igniteui-angular/src/lib/grids/column.component.ts @@ -1793,9 +1793,13 @@ export class IgxColumnGroupComponent extends IgxColumnComponent implements After this.children.forEach(child => { child.parent = this; }); + /* + TO DO: In Angular 9 this need to be removed, because the @ContentChildren will not return the `parent` + component in the query list. + */ this.children.changes.pipe(takeUntil(this.destroy$)) - .subscribe(() => { - if (this.children && this.children.toArray().length > 0 && this.children.toArray()[0] === this) { + .subscribe((change) => { + if (change.length > 1 && change.first === this) { this.children.reset(this.children.toArray().slice(1)); this.children.forEach(child => { child.parent = this;