fix(grid): update children when add or remove column in column group #5837

nrobakova · nrobakova · commit 679d08670ec0 · 2019-09-24T15:32:05.000+03:00
diff --git 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<IgxColumnComponent>;
+    /**
+     * @hidden
+     */
+    protected destroy$ = new Subject<boolean>();
+
     /**
      *@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<IgxColumnComponent>): 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<boolean>();
-
     @ContentChildren(IgxColumnComponent, { read: IgxColumnComponent })
     children = new QueryList<IgxColumnComponent>();
     /**
@@ -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,16 +1991,16 @@ 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;
             // filter out all cols with larger rowStart
             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
@@ -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({
