Skip to content

Commit 518f1c7

Browse files
committed
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.
1 parent 68c45ad commit 518f1c7

File tree

2 files changed

+76
-32
lines changed

2 files changed

+76
-32
lines changed

src/lib/chips/chip.spec.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,55 @@ describe('Chips', () => {
150150
expect(event.defaultPrevented).toBe(true);
151151
});
152152

153+
it('should not dispatch `selectionChange` event when deselecting a non-selected chip', () => {
154+
chipInstance.deselect();
155+
156+
const spy = jasmine.createSpy('selectionChange spy');
157+
const subscription = chipInstance.selectionChange.subscribe(spy);
158+
159+
chipInstance.deselect();
160+
161+
expect(spy).not.toHaveBeenCalled();
162+
subscription.unsubscribe();
163+
});
164+
165+
it('should not dispatch `selectionChange` event when selecting a selected chip', () => {
166+
chipInstance.select();
167+
168+
const spy = jasmine.createSpy('selectionChange spy');
169+
const subscription = chipInstance.selectionChange.subscribe(spy);
170+
171+
chipInstance.select();
172+
173+
expect(spy).not.toHaveBeenCalled();
174+
subscription.unsubscribe();
175+
});
176+
177+
it('should not dispatch `selectionChange` event when selecting a selected chip via ' +
178+
'user interaction', () => {
179+
chipInstance.select();
180+
181+
const spy = jasmine.createSpy('selectionChange spy');
182+
const subscription = chipInstance.selectionChange.subscribe(spy);
183+
184+
chipInstance.selectViaInteraction();
185+
186+
expect(spy).not.toHaveBeenCalled();
187+
subscription.unsubscribe();
188+
});
189+
190+
it('should not dispatch `selectionChange` through setter if the value did not change', () => {
191+
chipInstance.selected = false;
192+
193+
const spy = jasmine.createSpy('selectionChange spy');
194+
const subscription = chipInstance.selectionChange.subscribe(spy);
195+
196+
chipInstance.selected = false;
197+
198+
expect(spy).not.toHaveBeenCalled();
199+
subscription.unsubscribe();
200+
});
201+
153202
});
154203

155204
describe('keyboard behavior', () => {

src/lib/chips/chip.ts

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
159159
@Input()
160160
get selected(): boolean { return this._selected; }
161161
set selected(value: boolean) {
162-
this._selected = coerceBooleanProperty(value);
163-
this.selectionChange.emit({
164-
source: this,
165-
isUserInput: false,
166-
selected: value
167-
});
162+
const coercedValue = coerceBooleanProperty(value);
163+
164+
if (coercedValue !== this._selected) {
165+
this._selected = coercedValue;
166+
this._dispatchSelectionChange();
167+
}
168168
}
169169
protected _selected: boolean = false;
170170

@@ -263,45 +263,32 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
263263

264264
/** Selects the chip. */
265265
select(): void {
266-
this._selected = true;
267-
this.selectionChange.emit({
268-
source: this,
269-
isUserInput: false,
270-
selected: true
271-
});
266+
if (!this._selected) {
267+
this._selected = true;
268+
this._dispatchSelectionChange();
269+
}
272270
}
273271

274272
/** Deselects the chip. */
275273
deselect(): void {
276-
this._selected = false;
277-
this.selectionChange.emit({
278-
source: this,
279-
isUserInput: false,
280-
selected: false
281-
});
274+
if (this._selected) {
275+
this._selected = false;
276+
this._dispatchSelectionChange();
277+
}
282278
}
283279

284280
/** Select this chip and emit selected event */
285281
selectViaInteraction(): void {
286-
this._selected = true;
287-
// Emit select event when selected changes.
288-
this.selectionChange.emit({
289-
source: this,
290-
isUserInput: true,
291-
selected: true
292-
});
282+
if (!this._selected) {
283+
this._selected = true;
284+
this._dispatchSelectionChange(true);
285+
}
293286
}
294287

295288
/** Toggles the current selected state of this chip. */
296289
toggleSelected(isUserInput: boolean = false): boolean {
297290
this._selected = !this.selected;
298-
299-
this.selectionChange.emit({
300-
source: this,
301-
isUserInput,
302-
selected: this._selected
303-
});
304-
291+
this._dispatchSelectionChange(isUserInput);
305292
return this.selected;
306293
}
307294

@@ -376,6 +363,14 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
376363
});
377364
});
378365
}
366+
367+
private _dispatchSelectionChange(isUserInput = false) {
368+
this.selectionChange.emit({
369+
source: this,
370+
isUserInput,
371+
selected: this._selected
372+
});
373+
}
379374
}
380375

381376

0 commit comments

Comments
 (0)