feat(datagrid): use only clrDgItemsTrackBy for selection tracking (#…

…1144)

I removed two tests that appeared to test an optimization to skip work
if there are no items. These optimizations are invalid because it may
appear that the datagrid has no items when it just doesn't have items
yet.

CDE-71

BREAKING CHANGE: The row iterator `trackBy` function will no longer be
used for selection tracking. Update your code to pass
`clrDgItemsTrackBy` to the `clr-datagrid` component if you pass
`trackBy` to the row iterator. Note that the `clrDgItemsTrackBy`
signature does not include the element index.

projects/angular/src/data/datagrid/all.spec.ts

@@ -30,7 +30,6 @@ import DatagridColumnSpecs from './datagrid-column.spec';
 import DatagridFilterSpecs from './datagrid-filter.spec';
 import DatagridFooterSpecs from './datagrid-footer.spec';
 import DatagridHideableColumnDirectiveSpec from './datagrid-hideable-column.spec';
-import DatagridItemsTrackBySpecs from './datagrid-items-trackby.spec';
 import DatagridItemsSpecs from './datagrid-items.spec';
 import DatagridPageSizeSpecs from './datagrid-page-size.spec';
 import DatagridPaginationIntegrationSpecs from './datagrid-pagination.integration.spec';
@@ -82,7 +81,6 @@ describe('Datagrid', function () {
     DatagridColumnSpecs();
     DatagridColumnSeparatorSpecs();
     DatagridItemsSpecs();
-    DatagridItemsTrackBySpecs();
     DatagridRowSpecs();
     DatagridRowDetailSpecs();
     DatagridPageSizeSpecs();

projects/angular/src/data/datagrid/datagrid-items.spec.ts

@@ -135,10 +135,6 @@ export default function (): void {
         expect(this.clarityDirective.iterableProxy.ngForTrackBy).toBe(this.testComponent.trackBy);
       });
 
-      it('items receive the provided trackBy option', function () {
-        expect(this.clarityDirective.items.iteratorTrackBy).toBe(this.testComponent.trackBy);
-      });
-
       it('correctly mutates and resets an array with trackBy', function () {
         // Initial state
         this.fixture.nativeElement.querySelectorAll('li:first-child').forEach(li => (li.style.color = 'red'));

projects/angular/src/data/datagrid/datagrid-items.ts

@@ -52,7 +52,6 @@ export class ClrDatagridItems<T> implements DoCheck, OnDestroy {
 
   @Input('clrDgItemsTrackBy')
   set trackBy(value: TrackByFunction<T>) {
-    this.items.iteratorTrackBy = value;
     this.iterableProxy.ngForTrackBy = value;
   }
 

projects/angular/src/data/datagrid/datagrid-row.spec.ts

@@ -4,7 +4,7 @@
  * The full license information can be found in LICENSE in the root directory of this project.
  */
 
-import { Component, TrackByFunction } from '@angular/core';
+import { Component } from '@angular/core';
 import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
 
 import { LoadingListener } from '../../utils/loading/loading-listener';
@@ -185,54 +185,26 @@ export default function (): void {
     });
 
     describe('Conditional Selection with *ngFor', () => {
-      describe('DatagridWithNgForTrackBy', () => {
-        let fixture: ComponentFixture<NgForDatagridWithNgForTrackBy>;
-        let nativeElement: HTMLElement;
-
-        beforeEach(() => {
-          TestBed.configureTestingModule({
-            imports: [ClrDatagridModule],
-            declarations: [NgForDatagridWithNgForTrackBy],
-          });
-
-          fixture = TestBed.createComponent(NgForDatagridWithNgForTrackBy);
-          nativeElement = fixture.nativeElement;
-        });
-
-        it('does NOT disable the selection checkbox when clrDgSelectable is false (broken behavior)', () => {
-          fixture.componentInstance.clrDgSelectable = false;
-          fixture.detectChanges();
+      let fixture: ComponentFixture<NgForDatagridWithTrackBy>;
+      let nativeElement: HTMLElement;
 
-          const checkboxElement = nativeElement.querySelector("input[type='checkbox']");
-
-          expect(checkboxElement.getAttribute('disabled')).toBeNull();
+      beforeEach(() => {
+        TestBed.configureTestingModule({
+          imports: [ClrDatagridModule],
+          declarations: [NgForDatagridWithTrackBy],
         });
-      });
 
-      [NgForDatagridWithDatagridTrackBy, NgForDatagridWithDatagridTrackByAndNgForTrackBy].forEach(testComponentType => {
-        describe(testComponentType.name, () => {
-          let fixture: ComponentFixture<{ clrDgSelectable: boolean }>;
-          let nativeElement: HTMLElement;
-
-          beforeEach(() => {
-            TestBed.configureTestingModule({
-              imports: [ClrDatagridModule],
-              declarations: [testComponentType],
-            });
-
-            fixture = TestBed.createComponent<{ clrDgSelectable: boolean }>(testComponentType);
-            nativeElement = fixture.nativeElement;
-          });
+        fixture = TestBed.createComponent(NgForDatagridWithTrackBy);
+        nativeElement = fixture.nativeElement;
+      });
 
-          it('does disable the selection checkbox when clrDgSelectable is false', () => {
-            fixture.componentInstance.clrDgSelectable = false;
-            fixture.detectChanges();
+      it('does disable the selection checkbox when clrDgSelectable is false', () => {
+        fixture.componentInstance.clrDgSelectable = false;
+        fixture.detectChanges();
 
-            const checkboxElement: HTMLInputElement = nativeElement.querySelector("input[type='checkbox']");
+        const checkboxElement: HTMLInputElement = nativeElement.querySelector("input[type='checkbox']");
 
-            expect(checkboxElement.getAttribute('disabled')).toBeDefined();
-          });
-        });
+        expect(checkboxElement.getAttribute('disabled')).toBeDefined();
       });
     });
 
@@ -632,53 +604,16 @@ class ExpandTest {
   clrDgDetailCloseLabel = 'Close Me';
 }
 
-@Component({
-  template: `
-    <clr-datagrid [clrDgSelected]="[]">
-      <clr-dg-row
-        *ngFor="let item of items; trackBy: trackBy"
-        [clrDgItem]="item"
-        [clrDgSelectable]="clrDgSelectable"
-      ></clr-dg-row>
-    </clr-datagrid>
-  `,
-})
-class NgForDatagridWithNgForTrackBy {
-  clrDgSelectable = true;
-
-  readonly items: Item[] = [{ id: 42 }];
-  readonly trackBy: TrackByFunction<Item> = (_index, item) => item.id;
-}
-
 @Component({
   template: `
     <clr-datagrid [clrDgSelected]="[]" [clrDgItemsTrackBy]="trackBy">
       <clr-dg-row *ngFor="let item of items" [clrDgItem]="item" [clrDgSelectable]="clrDgSelectable"></clr-dg-row>
     </clr-datagrid>
   `,
 })
-class NgForDatagridWithDatagridTrackBy {
+class NgForDatagridWithTrackBy {
   clrDgSelectable = true;
 
   readonly items: Item[] = [{ id: 42 }];
   readonly trackBy: ClrDatagridItemsTrackByFunction<Item> = item => item.id;
 }
-
-@Component({
-  template: `
-    <clr-datagrid [clrDgSelected]="[]" [clrDgItemsTrackBy]="trackBy">
-      <clr-dg-row
-        *ngFor="let item of items; trackBy: iteratorTrackBy"
-        [clrDgItem]="item"
-        [clrDgSelectable]="clrDgSelectable"
-      ></clr-dg-row>
-    </clr-datagrid>
-  `,
-})
-class NgForDatagridWithDatagridTrackByAndNgForTrackBy {
-  clrDgSelectable = true;
-
-  readonly items: Item[] = [{ id: 42 }];
-  readonly datagridTrackBy: ClrDatagridItemsTrackByFunction<Item> = item => item.id;
-  readonly iteratorTrackBy: TrackByFunction<Item> = (_index, item) => item.id;
-}

projects/angular/src/data/datagrid/datagrid.module.ts

@@ -53,7 +53,6 @@ import { ClrDatagridFooter } from './datagrid-footer';
 import { ClrDatagridHideableColumn } from './datagrid-hideable-column';
 import { ClrIfDetail } from './datagrid-if-detail';
 import { ClrDatagridItems } from './datagrid-items';
-import { ClrDatagridItemsTrackBy } from './datagrid-items-trackby';
 import { ClrDatagridPageSize } from './datagrid-page-size';
 import { ClrDatagridPagination } from './datagrid-pagination';
 import { ClrDatagridPlaceholder } from './datagrid-placeholder';
@@ -83,7 +82,6 @@ export const CLR_DATAGRID_DIRECTIVES: Type<any>[] = [
   ClrDatagridFooter,
   ClrDatagridHideableColumn,
   ClrDatagridItems,
-  ClrDatagridItemsTrackBy,
   ClrDatagridPageSize,
   ClrDatagridPagination,
   ClrDatagridPlaceholder,