Merge pull request #5850 from IgniteUI/nrobakova/fix-issue-5837-81

kdinev · web-flow · commit 5756c2523bff · 2019-09-26T14:27:04.000+03:00
Update children collection when add or remove column in column group
diff --git a/projects/igniteui-angular/src/lib/grids/column.component.ts b/projects/igniteui-angular/src/lib/grids/column.component.ts
@@ -9,7 +9,7 @@ import {
     QueryList,
     TemplateRef,
     forwardRef,
-    AfterViewInit
+    OnDestroy
 } from '@angular/core';
 import { DataType } from '../data-operations/data-util';
 import { GridBaseAPIService } from './api.service';
@@ -39,6 +39,8 @@ import { IgxGridHeaderGroupComponent } from './grid-header-group.component';
 import { DeprecateProperty } from '../core/deprecateDecorators';
 import { MRLColumnSizeInfo, MRLResizeColumnInfo } from '../data-operations/multi-row-layout.interfaces';
 import { DisplayDensity } from '../core/displayDensity';
+import { Subject } from 'rxjs';
+import { takeUntil } from 'rxjs/operators';
 
 /**
  * **Ignite UI for Angular Column** -
@@ -54,7 +56,7 @@ import { DisplayDensity } from '../core/displayDensity';
     selector: 'igx-column',
     template: ``
 })
-export class IgxColumnComponent implements AfterContentInit {
+export class IgxColumnComponent implements AfterContentInit, OnDestroy {
     private _filterable = true;
     private _groupable = false;
     /**
@@ -879,13 +881,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;
     }
@@ -1020,6 +1022,10 @@ export class IgxColumnComponent implements AfterContentInit {
      * @memberof IgxColumnComponent
      */
     children: QueryList<IgxColumnComponent>;
+    /**
+     * @hidden
+     */
+    protected destroy$ = new Subject<boolean>();
     /**
      *@hidden
      */
@@ -1207,7 +1213,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;
@@ -1279,8 +1285,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;
@@ -1328,8 +1334,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[] {
@@ -1343,7 +1349,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 });
             }
         }
 
@@ -1422,7 +1428,7 @@ export class IgxColumnComponent implements AfterContentInit {
         grid.resetCaches();
         grid.cdr.detectChanges();
         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);
@@ -1485,7 +1491,7 @@ export class IgxColumnComponent implements AfterContentInit {
 
         grid.cdr.detectChanges();
         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);
@@ -1695,6 +1701,14 @@ export class IgxColumnComponent implements AfterContentInit {
      * @hidden
      */
     public populateVisibleIndexes() { }
+
+    /**
+     * @hidden
+     */
+    public ngOnDestroy() {
+        this.destroy$.next(true);
+        this.destroy$.complete();
+    }
 }
 
 
@@ -1704,7 +1718,7 @@ 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 {
 
     @ContentChildren(IgxColumnComponent, { read: IgxColumnComponent })
     children = new QueryList<IgxColumnComponent>();
@@ -1836,6 +1850,19 @@ 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((change) => {
+                if (change.length > 1 && change.first === this) {
+                    this.children.reset(this.children.toArray().slice(1));
+                    this.children.forEach(child => {
+                        child.parent = this;
+                    });
+                }
+            });
     }
     /**
      * Returns the children columns collection.
@@ -1881,7 +1908,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)}`;
@@ -1902,7 +1929,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.
@@ -1987,7 +2014,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());
         }
     }
 
@@ -2026,16 +2053,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
@@ -1452,6 +1452,39 @@ describe('IgxGrid - multi-column headers', () => {
         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({
