Skip to content

Commit

Permalink
fix(unique-selection-dispatcher): remove listeners on destroy (#5164)
Browse files Browse the repository at this point in the history
  • Loading branch information
Eddman authored and jelbourn committed Jun 22, 2017
1 parent 2239668 commit f9bbbe7
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 19 deletions.
22 changes: 16 additions & 6 deletions src/lib/button-toggle/button-toggle.ts
Expand Up @@ -16,6 +16,7 @@ import {
HostBinding,
Input,
OnInit,
OnDestroy,
Optional,
Output,
QueryList,
Expand Down Expand Up @@ -270,7 +271,7 @@ export class MdButtonToggleGroupMultiple extends _MdButtonToggleGroupMixinBase
'class': 'mat-button-toggle'
}
})
export class MdButtonToggle implements OnInit {
export class MdButtonToggle implements OnInit, OnDestroy {
/** Whether or not this button toggle is checked. */
private _checked: boolean = false;

Expand All @@ -286,6 +287,9 @@ export class MdButtonToggle implements OnInit {
/** Whether or not the button toggle is a single selection. */
private _isSingleSelector: boolean = false;

/** Unregister function for _buttonToggleDispatcher **/
private _removeUniqueSelectionListener: () => void = () => {};

@ViewChild('input') _inputElement: ElementRef;

/** The parent button toggle group (exclusive selection). Optional. */
Expand Down Expand Up @@ -371,11 +375,12 @@ export class MdButtonToggle implements OnInit {
this.buttonToggleGroupMultiple = toggleGroupMultiple;

if (this.buttonToggleGroup) {
_buttonToggleDispatcher.listen((id: string, name: string) => {
if (id != this.id && name == this.name) {
this.checked = false;
}
});
this._removeUniqueSelectionListener =
_buttonToggleDispatcher.listen((id: string, name: string) => {
if (id != this.id && name == this.name) {
this.checked = false;
}
});

this._type = 'radio';
this.name = this.buttonToggleGroup.name;
Expand Down Expand Up @@ -445,4 +450,9 @@ export class MdButtonToggle implements OnInit {
event.value = this._value;
this.change.emit(event);
}

// Unregister buttonToggleDispatcherListener on destroy
ngOnDestroy(): void {
this._removeUniqueSelectionListener();
}
}
32 changes: 32 additions & 0 deletions src/lib/core/coordination/unique-selection-dispatcher.spec.ts
@@ -0,0 +1,32 @@
import {UniqueSelectionDispatcher} from './unique-selection-dispatcher';


describe('Unique selection dispatcher', () => {

describe('register', () => {
it('once unregistered the listener must not be called on notify', (done) => {
let dispatcher: UniqueSelectionDispatcher = new UniqueSelectionDispatcher();
let called = false;

// Register first listener
dispatcher.listen(() => {
called = true;
});

// Register a listener
let deregisterFn = dispatcher.listen(() => {
done.fail('Should not be called');
});

// Unregister
deregisterFn();

// Call registered listeners
dispatcher.notify('testId', 'testName');

expect(called).toBeTruthy('Registered listener must be called.');

done();
});
});
});
12 changes: 10 additions & 2 deletions src/lib/core/coordination/unique-selection-dispatcher.ts
Expand Up @@ -36,9 +36,17 @@ export class UniqueSelectionDispatcher {
}
}

/** Listen for future changes to item selection. */
listen(listener: UniqueSelectionDispatcherListener) {
/**
* Listen for future changes to item selection.
* @return Function used to unregister listener
**/
listen(listener: UniqueSelectionDispatcherListener): () => void {
this._listeners.push(listener);
return () => {
this._listeners = this._listeners.filter((registered: UniqueSelectionDispatcherListener) => {
return listener !== registered;
});
};
}
}

Expand Down
17 changes: 11 additions & 6 deletions src/lib/expansion/accordion-item.ts
Expand Up @@ -48,19 +48,24 @@ export class AccordionItem implements OnDestroy {
}
private _expanded: boolean;

/** Unregister function for _expansionDispatcher **/
private _removeUniqueSelectionListener: () => void = () => {};

constructor(@Optional() public accordion: CdkAccordion,
protected _expansionDispatcher: UniqueSelectionDispatcher) {
_expansionDispatcher.listen((id: string, accordionId: string) => {
if (this.accordion && !this.accordion.multi &&
this.accordion.id === accordionId && this.id !== id) {
this.expanded = false;
}
});
this._removeUniqueSelectionListener =
_expansionDispatcher.listen((id: string, accordionId: string) => {
if (this.accordion && !this.accordion.multi &&
this.accordion.id === accordionId && this.id !== id) {
this.expanded = false;
}
});
}

/** Emits an event for the accordion item being destroyed. */
ngOnDestroy() {
this.destroyed.emit();
this._removeUniqueSelectionListener();
}

/** Toggles the expanded state of the accordion item. */
Expand Down
15 changes: 10 additions & 5 deletions src/lib/radio/radio.ts
Expand Up @@ -457,6 +457,9 @@ export class MdRadioButton extends _MdRadioButtonMixinBase
/** Reference to the current focus ripple. */
private _focusRipple: RippleRef | null;

/** Unregister function for _radioDispatcher **/
private _removeUniqueSelectionListener: () => void = () => {};

/** The native `<input type=radio>` element */
@ViewChild('input') _inputElement: ElementRef;

Expand All @@ -472,11 +475,12 @@ export class MdRadioButton extends _MdRadioButtonMixinBase
// TODO(jelbourn): Assert that there's no name binding AND a parent radio group.
this.radioGroup = radioGroup;

_radioDispatcher.listen((id: string, name: string) => {
if (id != this.id && name == this.name) {
this.checked = false;
}
});
this._removeUniqueSelectionListener =
_radioDispatcher.listen((id: string, name: string) => {
if (id != this.id && name == this.name) {
this.checked = false;
}
});
}

/** Focuses the radio button. */
Expand Down Expand Up @@ -512,6 +516,7 @@ export class MdRadioButton extends _MdRadioButtonMixinBase

ngOnDestroy() {
this._focusOriginMonitor.stopMonitoring(this._inputElement.nativeElement);
this._removeUniqueSelectionListener();
}

/** Dispatch change event with current value. */
Expand Down

0 comments on commit f9bbbe7

Please sign in to comment.