refactor(ngrid): simplify working with rows

BREAKING CHANGE: Binding `[grid]` and `[row]` is no longer required. Same for `detailRow` and `infiniteRow`
shlomiassaf · Dec 3, 2020 · a81b1af · a81b1af
1 parent 170c2d4
commit a81b1af
Show file tree

Hide file tree

Showing 20 changed files with 272 additions and 266 deletions.
diff --git a/apps/ngrid-docs-app/content/demos/complex-demo1/complex-demo1.component.html b/apps/ngrid-docs-app/content/demos/complex-demo1/complex-demo1.component.html
@@ -32,10 +32,7 @@
     <!-- DEFINING DEFAULT PAGINATOR TEMPLATE FOR THIS PBL-NGRID -->
 
     <!-- DETAIL ROW TEMPLATE DEFINITION-->
-    <pbl-ngrid-row *pblNgridDetailRowParentRef="let row;"
-                  [detailRow]="row"
-                  matRipple>
-    </pbl-ngrid-row>
+    <pbl-ngrid-row *pblNgridDetailRowParentRef="let row;" detailRow matRipple></pbl-ngrid-row>
     <div *pblNgridDetailRowDef="let row; animation as animation; hasAnimation: 'interaction'"
          class="pbl-detail-row"
          [@.disabled]="animation.fromRender" (@detailExpand.done)="animation.end()" [@detailExpand]>

diff --git a/...nt/demos/infinite-scroll-performance-demo/infinite-scroll-performance-demo.component.html b/...nt/demos/infinite-scroll-performance-demo/infinite-scroll-performance-demo.component.html
@@ -25,7 +25,7 @@
             [ngridCellClass]="value < 0 ? col.type.data.pbl : col.type.data.pos">{{ value | currency:col.type.data.meta.currency(row):'symbol':col.type.data.format }}</div>
       <div *pblNgridCellTypeDef="'flagAndCountry'; col as col; row as row">{{ col.type.data.flagAndCountry(row) }}</div>
 
-      <pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row; grid as grid" class="pbl-ngrid-infinite-virtual-row" [grid]="grid" [infiniteRow]="row">
+      <pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row;" class="pbl-ngrid-infinite-virtual-row" infiniteRow>
          <mat-spinner diameter="24"></mat-spinner>
       </pbl-ngrid-row>
 

diff --git a/apps/ngrid-docs-app/content/features/built-in-plugins/detail-row/detail-row.component.html b/apps/ngrid-docs-app/content/features/built-in-plugins/detail-row/detail-row.component.html
@@ -1,5 +1,5 @@
 <!-- This should be declared in a top-level component (e.g. app root) -->
-
+<pbl-ngrid-row *pblNgridDetailRowParentRef="let row;" detailRow matRipple></pbl-ngrid-row>
 <pbl-ngrid blockUi [dataSource]="ds" [columns]="columns" vScrollNone
            detailRow (toggleChange)="onToggleChange($event)">
   <div *pblNgridDetailRowDef="let row" class="pbl-detail-row">
@@ -8,8 +8,6 @@ <h1>Detail Row</h1>
       <pre>{{row | json}}</pre>
     </div>
   </div>
-
-  <pbl-ngrid-row *pblNgridDetailRowParentRef="let row; grid as grid" [grid]="grid" [detailRow]="row" matRipple></pbl-ngrid-row>
 </pbl-ngrid>
 
 <div>

diff --git a/apps/ngrid-docs-app/content/features/grid/infinite-scroll/index-based-paging.component.html b/apps/ngrid-docs-app/content/features/grid/infinite-scroll/index-based-paging.component.html
@@ -1,7 +1,7 @@
 <pbl-ngrid [dataSource]="ds" [columns]="columns">
   <mat-progress-bar *ngIf="ds.adapter.virtualRowsLoading | async" mode="indeterminate"></mat-progress-bar>
 
-  <pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row; grid as grid" class="pbl-ngrid-infinite-virtual-row" [grid]="grid" [row]="row">
+  <pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row;" class="pbl-ngrid-infinite-virtual-row" row>
   </pbl-ngrid-row>
 
 </pbl-ngrid>
diff --git a/libs/ngrid/detail-row/src/lib/detail-row/detail-row-plugin.ts b/libs/ngrid/detail-row/src/lib/detail-row/detail-row-plugin.ts
@@ -70,7 +70,7 @@ export class PblNgridDetailRowPluginDirective<T> implements OnDestroy {
       this._forceSingle = value;
       if (value && this._openedRow && this._openedRow.expended) {
         for (const detailRow of this._detailRowRows) {
-          if (detailRow.row !== this._openedRow.row) {
+          if (detailRow.context.$implicit !== this._openedRow.row) {
             detailRow.toggle(false);
           }
         }
@@ -187,7 +187,7 @@ export class PblNgridDetailRowPluginDirective<T> implements OnDestroy {
 
   toggleDetailRow(row: any, forceState?: boolean): boolean | void {
     for (const detailRow of this._detailRowRows) {
-      if (detailRow.row === row) {
+      if (detailRow.context.$implicit === row) {
         detailRow.toggle(forceState);
         return detailRow.expended;
       }

diff --git a/libs/ngrid/detail-row/src/lib/detail-row/directives.ts b/libs/ngrid/detail-row/src/lib/detail-row/directives.ts
@@ -111,6 +111,6 @@ export class PblNgridDetailRowParentRefDirective<T> extends PblNgridRowDef<T> im
  */
 @Component({
   selector: 'pbl-ngrid-default-detail-row-parent',
-  template: `<pbl-ngrid-row *pblNgridDetailRowParentRef="let row; grid as grid" [grid]="grid" [detailRow]="row"></pbl-ngrid-row>`,
+  template: `<pbl-ngrid-row *pblNgridDetailRowParentRef="let row;" detailRow></pbl-ngrid-row>`,
 })
 export class PblNgridDefaultDetailRowParentComponent { }
diff --git a/libs/ngrid/detail-row/src/lib/detail-row/row.ts b/libs/ngrid/detail-row/src/lib/detail-row/row.ts
@@ -1,18 +1,16 @@
 import {
   ChangeDetectionStrategy,
   Component,
-  Input,
-  ElementRef,
-  OnDestroy, Optional,
+  OnInit,
+  OnDestroy,
   ViewEncapsulation,
   ViewContainerRef,
   ViewChild,
-  ChangeDetectorRef,
 } from '@angular/core';
 import { ENTER, SPACE } from '@angular/cdk/keycodes';
 import { CdkRow } from '@angular/cdk/table';
 
-import { PblNgridRowComponent, utils, PblNgridComponent, PblRowContext } from '@pebula/ngrid';
+import { PblNgridRowComponent, utils } from '@pebula/ngrid';
 import { PblNgridDetailRowPluginDirective, PblDetailsRowToggleEvent, PLUGIN_KEY } from './detail-row-plugin';
 import { DetailRowController } from './detail-row-controller';
 
@@ -30,7 +28,7 @@ export const PBL_NGRID_ROW_TEMPLATE = '<ng-content select=".pbl-ngrid-row-prefix
   host: { // tslint:disable-line:no-host-metadata-property
     class: 'pbl-ngrid-row pbl-row-detail-parent',
     role: 'row',
-    '[attr.tabindex]': 'grid?.rowFocus',
+    '[attr.tabindex]': 'grid.rowFocus',
     '(keydown)': 'handleKeydown($event)'
   },
   template: PBL_NGRID_ROW_TEMPLATE,
@@ -41,7 +39,7 @@ export const PBL_NGRID_ROW_TEMPLATE = '<ng-content select=".pbl-ngrid-row-prefix
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
 })
-export class PblNgridDetailRowComponent extends PblNgridRowComponent implements OnDestroy {
+export class PblNgridDetailRowComponent extends PblNgridRowComponent implements OnInit, OnDestroy, PblDetailsRowToggleEvent {
 
   get expended(): boolean {
     return this.opened;
@@ -51,67 +49,74 @@ export class PblNgridDetailRowComponent extends PblNgridRowComponent implements
     return super.height + this.controller.getDetailRowHeight(this);
   }
 
+  get row() { return this.context.$implicit; }
+
   // We must explicitly define the inherited properties which have angular annotations
   // Because angular will not detect them when building this library.
   // TODO: When moving up to IVY see if this one get fixed
-  /**
-   * Optional grid instance, required only if the row is declared outside the scope of the grid.
-   */
-  @Input() grid: PblNgridComponent;
-  @ViewChild('viewRef', { read: ViewContainerRef }) _viewRef: ViewContainerRef;
-
-  get row() { return this._row; }
-  @Input('detailRow') set row(value: any) {
-    super.row = value;
-    if (this._row !== value) {
-      this._row = value;
-      this.plugin?.markForCheck();
-      this.controller?.detectChanges(this);
-    }
-  }
+  @ViewChild('viewRef', { read: ViewContainerRef, static: true }) _viewRef: ViewContainerRef;
 
-  private _row: any;
   private opened = false;
   private plugin: PblNgridDetailRowPluginDirective<any>;
   private controller: DetailRowController;
 
-  constructor(@Optional() grid: PblNgridComponent,
-              cdRef: ChangeDetectorRef,
-              el: ElementRef<HTMLElement>) {
-    super(grid, cdRef, el);
+  ngOnInit(): void {
+    super.ngOnInit();
+    this.plugin.addDetailRow(this);
+    const tradeEvents = this._extApi.pluginCtrl.getPlugin('targetEvents');
+
+    tradeEvents.cellClick
+      .pipe(utils.unrx(this))
+      .subscribe( event => {
+        if (event.type === 'data' && event.row === this.context.$implicit) {
+          const { excludeToggleFrom } = this.plugin;
+          if (!excludeToggleFrom || !excludeToggleFrom.some( c => event.column.id === c )) {
+            this.toggle();
+          }
+        }
+      });
+
+    tradeEvents.rowClick
+      .pipe(utils.unrx(this))
+      .subscribe( event => {
+        if (!event.root && event.type === 'data' && event.row === this.context.$implicit) {
+          this.toggle();
+        }
+      });
   }
 
   ngOnDestroy(): void {
     utils.unrx.kill(this);
-    this.plugin?.removeDetailRow(this);
+    this.plugin.removeDetailRow(this);
     this.controller.clearDetailRow(this, true);
     super.ngOnDestroy();
   }
 
-  updateRow(): void {
-    super.updateRow();
-    if (this.context) {
-      if (this.currRow !== this.prevRow) { // only if row has changed (TODO: use identity based change detection?)
-        switch (this.plugin.whenContextChange) {
-          case 'context':
-            const isContextOpened = !!this.context.getExternal('detailRow');
-            isContextOpened && this.opened
-              ? this.controller.updateDetailRow(this) // if already opened, just update the context
-              : this.toggle(isContextOpened, true) // if not opened, force to the context state
-            ;
-            break;
-          case 'render':
-            if (this.opened) {
-              this.controller.updateDetailRow(this);
-            }
-            break;
-          case 'close':
-            this.toggle(false, true);
-            break;
-        }
-        this.plugin.toggledRowContextChange.next(this);
+  updateRow() {
+    if (super.updateRow()) { // only if row has changed (TODO: use identity based change detection?)
+      switch (this.plugin.whenContextChange) {
+        case 'context':
+          const isContextOpened = !!this.context.getExternal('detailRow');
+          isContextOpened && this.opened
+            ? this.controller.updateDetailRow(this) // if already opened, just update the context
+            : this.toggle(isContextOpened, true) // if not opened, force to the context state
+          ;
+          break;
+        case 'render':
+          if (this.opened) {
+            this.controller.updateDetailRow(this);
+          }
+          break;
+        case 'close':
+          this.toggle(false, true);
+          break;
       }
+      this.plugin.markForCheck();
+      this.controller.detectChanges(this);
+      this.plugin.toggledRowContextChange.next(this);
+      return true;
     }
+    return false;
   }
 
   toggle(forceState?: boolean, fromRender = false): void {
@@ -147,32 +152,9 @@ export class PblNgridDetailRowComponent extends PblNgridRowComponent implements
     }
   }
 
-  protected init() {
+  protected onCtor() {
+    super.onCtor();
     this.plugin = this._extApi.pluginCtrl.getPlugin(PLUGIN_KEY); // TODO: THROW IF NO PLUGIN...
     this.controller = this.plugin.detailRowCtrl;
-    if (this._row) {
-      this.plugin.markForCheck();
-    }
-    super.init();
-    this.plugin.addDetailRow(this);
-    const tradeEvents = this._extApi.pluginCtrl.getPlugin('targetEvents');
-    tradeEvents.cellClick
-      .pipe(utils.unrx(this))
-      .subscribe( event => {
-        if (event.type === 'data' && event.row === this.context.$implicit) {
-          const { excludeToggleFrom } = this.plugin;
-          if (!excludeToggleFrom || !excludeToggleFrom.some( c => event.column.id === c )) {
-            this.toggle();
-          }
-        }
-      });
-
-    tradeEvents.rowClick
-      .pipe(utils.unrx(this))
-      .subscribe( event => {
-        if (!event.root && event.type === 'data' && event.row === this.context.$implicit) {
-          this.toggle();
-        }
-      });
   }
 }
diff --git a/...e-scroll/src/lib/default-infinite-virtual-row/default-infinite-virtual-row.component.html b/...e-scroll/src/lib/default-infinite-virtual-row/default-infinite-virtual-row.component.html
@@ -1,3 +1,3 @@
-<pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row; grid as grid" class="pbl-ngrid-infinite-virtual-row" [grid]="grid" [infiniteRow]="row">
+<pbl-ngrid-row in *pblNgridInfiniteVirtualRowDef="let row;" class="pbl-ngrid-infinite-virtual-row" infiniteRow>
   ...Loading
 </pbl-ngrid-row>
diff --git a/libs/ngrid/infinite-scroll/src/lib/infinite-virtual-row/row.ts b/libs/ngrid/infinite-scroll/src/lib/infinite-virtual-row/row.ts
@@ -1,6 +1,6 @@
-import { ChangeDetectionStrategy, Component, Input, ViewChild, ViewContainerRef, ViewEncapsulation } from '@angular/core';
+import { ChangeDetectionStrategy, Component, ViewEncapsulation } from '@angular/core';
 import { CdkRow } from '@angular/cdk/table';
-import { PblNgridRowComponent, PblNgridComponent } from '@pebula/ngrid';
+import { PblNgridRowComponent } from '@pebula/ngrid';
 
 export const PBL_NGRID_ROW_TEMPLATE  = `<ng-content select=".pbl-ngrid-row-prefix"></ng-content><ng-content></ng-content><ng-content select=".pbl-ngrid-row-suffix"></ng-content>`;
 
@@ -20,17 +20,6 @@ export const PBL_NGRID_ROW_TEMPLATE  = `<ng-content select=".pbl-ngrid-row-prefi
 })
 export class PblNgridInfiniteRowComponent<T = any> extends PblNgridRowComponent<T> {
 
-  // We must explicitly define the inherited properties which have angular annotations
-  // Because angular will not detect them when building this library.
-  // TODO: When moving up to IVY see if this one get fixed
-  /**
-   * Optional grid instance, required only if the row is declared outside the scope of the grid.
-   */
-  @Input() grid: PblNgridComponent<T>;
-  @ViewChild('viewRef', { read: ViewContainerRef }) _viewRef: ViewContainerRef;
-
-  @Input('infiniteRow') set row(value: any) { this.updateRow(); }
-
   canCreateCell() {
     return false;
   }

diff --git a/libs/ngrid/src/lib/grid/context/api.ts b/libs/ngrid/src/lib/grid/context/api.ts
@@ -403,7 +403,7 @@ export class ContextApi<T = any> {
       rowContext.dsIndex = dsIndex;
       rowContext.identity = identity;
       rowContext.fromState(this.getCreateState(rowContext));
-      this.addToViewCache(rowContext._attachedRow.rowIndex, rowContext)
+      this.addToViewCache(renderRowIndex, rowContext)
     }
   }
 

diff --git a/libs/ngrid/src/lib/grid/context/row.ts b/libs/ngrid/src/lib/grid/context/row.ts
@@ -43,7 +43,7 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
   outOfView: boolean;
 
   readonly grid: PblNgridComponent<T>;
-  _attachedRow: PblNgridRowComponent<T>;
+  private _attachedRow: PblNgridRowComponent<T>;
   private external: any = {};
 
   /**
@@ -153,9 +153,7 @@ export class PblRowContext<T> implements PblNgridRowContext<T> {
     if (this._attachedRow) {
       this._updatePending = false;
       this.extApi.contextApi._updateRowContext(this, this._attachedRow.rowIndex);
-      if (this._attachedRow.row !== this._$implicit) {
-        this._attachedRow.row = this._$implicit;
-      }
+      this._attachedRow.updateRow();
     } else {
       this._updatePending = !!this._$implicit;
     }

diff --git a/libs/ngrid/src/lib/grid/ngrid.component.html b/libs/ngrid/src/lib/grid/ngrid.component.html
@@ -34,7 +34,7 @@
 
       <!-- GRID RECORD ROW DEFINITION TEMPLATES -->
       <!-- We dont need columns because we implement them internally -->
-      <pbl-ngrid-row *pblNgridRowDef="let row;" [row]="row"></pbl-ngrid-row>
+      <pbl-ngrid-row *pblNgridRowDef="let row;" row></pbl-ngrid-row>
       <!-- GRID RECORD ROW DEFINITION TEMPLATES -->
     </pbl-cdk-table>
   </pbl-cdk-virtual-scroll-viewport>