From 6f983fdf4411e794e040b3f5de4b6b491b93cf73 Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Mon, 17 Sep 2018 21:59:29 +0200
Subject: [PATCH] fix(chips): selectionChange event firing when value has not
 changed

Fixes the `selectionChange` event always firing for the setter, `select`, `deselect` and `selectViaInteraction`, even if the value hasn't changed. Also gets rid of some repetition around emitting the event.
---
 src/lib/chips/chip.spec.ts | 49 +++++++++++++++++++++++++++++++
 src/lib/chips/chip.ts      | 59 +++++++++++++++++---------------------
 2 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/src/lib/chips/chip.spec.ts b/src/lib/chips/chip.spec.ts
index 9a0328f3df95..25416a371835 100644
--- a/src/lib/chips/chip.spec.ts
+++ b/src/lib/chips/chip.spec.ts
@@ -150,6 +150,55 @@ describe('Chips', () => {
         expect(event.defaultPrevented).toBe(true);
       });
 
+      it('should not dispatch `selectionChange` event when deselecting a non-selected chip', () => {
+        chipInstance.deselect();
+
+        const spy = jasmine.createSpy('selectionChange spy');
+        const subscription = chipInstance.selectionChange.subscribe(spy);
+
+        chipInstance.deselect();
+
+        expect(spy).not.toHaveBeenCalled();
+        subscription.unsubscribe();
+      });
+
+      it('should not dispatch `selectionChange` event when selecting a selected chip', () => {
+        chipInstance.select();
+
+        const spy = jasmine.createSpy('selectionChange spy');
+        const subscription = chipInstance.selectionChange.subscribe(spy);
+
+        chipInstance.select();
+
+        expect(spy).not.toHaveBeenCalled();
+        subscription.unsubscribe();
+      });
+
+      it('should not dispatch `selectionChange` event when selecting a selected chip via ' +
+        'user interaction', () => {
+          chipInstance.select();
+
+          const spy = jasmine.createSpy('selectionChange spy');
+          const subscription = chipInstance.selectionChange.subscribe(spy);
+
+          chipInstance.selectViaInteraction();
+
+          expect(spy).not.toHaveBeenCalled();
+          subscription.unsubscribe();
+        });
+
+      it('should not dispatch `selectionChange` through setter if the value did not change', () => {
+        chipInstance.selected = false;
+
+        const spy = jasmine.createSpy('selectionChange spy');
+        const subscription = chipInstance.selectionChange.subscribe(spy);
+
+        chipInstance.selected = false;
+
+        expect(spy).not.toHaveBeenCalled();
+        subscription.unsubscribe();
+      });
+
     });
 
     describe('keyboard behavior', () => {
diff --git a/src/lib/chips/chip.ts b/src/lib/chips/chip.ts
index 99f9d0e04fea..d53e3c08ee26 100644
--- a/src/lib/chips/chip.ts
+++ b/src/lib/chips/chip.ts
@@ -159,12 +159,12 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
   @Input()
   get selected(): boolean { return this._selected; }
   set selected(value: boolean) {
-    this._selected = coerceBooleanProperty(value);
-    this.selectionChange.emit({
-      source: this,
-      isUserInput: false,
-      selected: value
-    });
+    const coercedValue = coerceBooleanProperty(value);
+
+    if (coercedValue !== this._selected) {
+      this._selected = coercedValue;
+      this._dispatchSelectionChange();
+    }
   }
   protected _selected: boolean = false;
 
@@ -262,45 +262,32 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
 
   /** Selects the chip. */
   select(): void {
-    this._selected = true;
-    this.selectionChange.emit({
-      source: this,
-      isUserInput: false,
-      selected: true
-    });
+    if (!this._selected) {
+      this._selected = true;
+      this._dispatchSelectionChange();
+    }
   }
 
   /** Deselects the chip. */
   deselect(): void {
-    this._selected = false;
-    this.selectionChange.emit({
-      source: this,
-      isUserInput: false,
-      selected: false
-    });
+    if (this._selected) {
+      this._selected = false;
+      this._dispatchSelectionChange();
+    }
   }
 
   /** Select this chip and emit selected event */
   selectViaInteraction(): void {
-    this._selected = true;
-    // Emit select event when selected changes.
-    this.selectionChange.emit({
-      source: this,
-      isUserInput: true,
-      selected: true
-    });
+    if (!this._selected) {
+      this._selected = true;
+      this._dispatchSelectionChange(true);
+    }
   }
 
   /** Toggles the current selected state of this chip. */
   toggleSelected(isUserInput: boolean = false): boolean {
     this._selected = !this.selected;
-
-    this.selectionChange.emit({
-      source: this,
-      isUserInput,
-      selected: this._selected
-    });
-
+    this._dispatchSelectionChange(isUserInput);
     return this.selected;
   }
 
@@ -375,6 +362,14 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
         });
       });
   }
+
+  private _dispatchSelectionChange(isUserInput = false) {
+    this.selectionChange.emit({
+      source: this,
+      isUserInput,
+      selected: this._selected
+    });
+  }
 }