From 7089fb859c3766bccbb8bb63e853410913fd1ce5 Mon Sep 17 00:00:00 2001
From: Jeeyun Lim <jeeyun.lim@gmail.com>
Date: Thu, 21 Jun 2018 15:46:12 -0700
Subject: [PATCH] [ng] Datagrid bug fix for when selection is cleared by
 applying filter

Signed-off-by: Jeeyun Lim <jeeyun.lim@gmail.com>
---
 .../data/datagrid/providers/selection.spec.ts | 25 +++++++++
 .../data/datagrid/providers/selection.ts      | 56 ++++++++++---------
 2 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/src/clr-angular/data/datagrid/providers/selection.spec.ts b/src/clr-angular/data/datagrid/providers/selection.spec.ts
index 2c0983f0f7..b9642259cd 100644
--- a/src/clr-angular/data/datagrid/providers/selection.spec.ts
+++ b/src/clr-angular/data/datagrid/providers/selection.spec.ts
@@ -184,6 +184,31 @@ export default function(): void {
                 expect(nbChanges).toBe(3);
             });
 
+            it("does not emit selection change twice after a filter is applied", function() {
+                let nbChanges = 0;
+                let currentSelection: any;
+                selectionInstance.change.subscribe((items: any) => {
+                    nbChanges++;
+                    currentSelection = items;
+                });
+
+                selectionInstance.selectionType = SelectionType.Multi;
+                expect(nbChanges).toBe(1);  // current is initialized to [] at this point
+
+                selectionInstance.current = [4, 2];
+                expect(nbChanges).toBe(2);
+
+                const evenFilter: EvenFilter = new EvenFilter();
+                filtersInstance.add(<ClrDatagridFilterInterface<any>>evenFilter);
+                evenFilter.toggle();
+
+                // current is set to [] because filter is applied, and nbChanges is 3.
+                // there isn't an additional change that would have been fired to
+                // update current selection given the new data set post filter.
+                expect(selectionInstance.current.length).toBe(0);
+                expect(nbChanges).toBe(3);
+            });
+
             it("clears selection when a filter is added", function() {
                 selectionInstance.selectionType = SelectionType.Multi;
                 selectionInstance.current = [4, 2];
diff --git a/src/clr-angular/data/datagrid/providers/selection.ts b/src/clr-angular/data/datagrid/providers/selection.ts
index 300b7fc988..0073df7342 100644
--- a/src/clr-angular/data/datagrid/providers/selection.ts
+++ b/src/clr-angular/data/datagrid/providers/selection.ts
@@ -80,34 +80,39 @@ export class Selection {
                     const trackBy: TrackByFunction<any> = this._items.trackBy;
                     let selectionUpdated: boolean = false;
 
-                    updatedItems.forEach((item, index) => {
-                        const ref = trackBy(index, item);
-                        // Look in current selected refs array if item is selected, and update actual value
-                        const selectedIndex = this.prevSelectionRefs.indexOf(ref);
-                        if (selectedIndex > -1) {
-                            leftOver[selectedIndex] = item;
-                            selectionUpdated = true;
-                        }
-                    });
+                    // TODO: revisit this when we work on https://github.com/vmware/clarity/issues/2342
+                    // currently, the selection is cleared when filter is applied, so the logic inside
+                    // the if statement below results in broken behavior.
+                    if (leftOver.length > 0) {
+                        updatedItems.forEach((item, index) => {
+                            const ref = trackBy(index, item);
+                            // Look in current selected refs array if item is selected, and update actual value
+                            const selectedIndex = this.prevSelectionRefs.indexOf(ref);
+                            if (selectedIndex > -1) {
+                                leftOver[selectedIndex] = item;
+                                selectionUpdated = true;
+                            }
+                        });
 
-                    // Filter out any unmatched items if we're using smart datagrids where we expect all items to be
-                    // present
-                    if (this._items.smart) {
-                        leftOver = leftOver.filter(selected => updatedItems.indexOf(selected) > -1);
-                        if (this.current.length !== leftOver.length) {
-                            selectionUpdated = true;
+                        // Filter out any unmatched items if we're using smart datagrids where we expect all items to be
+                        // present
+                        if (this._items.smart) {
+                            leftOver = leftOver.filter(selected => updatedItems.indexOf(selected) > -1);
+                            if (this.current.length !== leftOver.length) {
+                                selectionUpdated = true;
+                            }
                         }
-                    }
 
-                    // TODO: Discussed this with Eudes and this is fine for now.
-                    // But we need to figure out a different pattern for the
-                    // child triggering the parent change detection problem.
-                    // Using setTimeout for now to fix this.
-                    setTimeout(() => {
-                        if (selectionUpdated) {
-                            this.current = leftOver;
-                        }
-                    }, 0);
+                        // TODO: Discussed this with Eudes and this is fine for now.
+                        // But we need to figure out a different pattern for the
+                        // child triggering the parent change detection problem.
+                        // Using setTimeout for now to fix this.
+                        setTimeout(() => {
+                            if (selectionUpdated) {
+                                this.current = leftOver;
+                            }
+                        }, 0);
+                    }
                     break;
                 }
 
@@ -118,6 +123,7 @@ export class Selection {
 
     public clearSelection(): void {
         this.current.length = 0;
+        this.prevSelectionRefs = [];
         this.emitChange();
     }