Skip to content

Commit

Permalink
fix(popover): cleanup subscriptions within the DropdownFocusHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt authored and coryrylan committed Mar 21, 2022
1 parent b95729e commit 6ea37a8
Showing 1 changed file with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { isPlatformBrowser } from '@angular/common';
import { Inject, Injectable, Optional, PLATFORM_ID, Renderer2, SkipSelf } from '@angular/core';
import { Inject, Injectable, OnDestroy, Optional, PLATFORM_ID, Renderer2, SkipSelf } from '@angular/core';
import { Observable, of, ReplaySubject } from 'rxjs';
import { map, take } from 'rxjs/operators';
import { ClrPopoverToggleService } from '../../../utils/popover/providers/popover-toggle.service';
Expand All @@ -18,7 +18,7 @@ import { Linkers } from '../../../utils/focus/focusable-item/linkers';
import { wrapObservable } from '../../../utils/focus/wrap-observable';

@Injectable()
export class DropdownFocusHandler implements FocusableItem {
export class DropdownFocusHandler implements OnDestroy, FocusableItem {
constructor(
@Inject(UNIQUE_ID) public id: string,
private renderer: Renderer2,
Expand All @@ -42,7 +42,7 @@ export class DropdownFocusHandler implements FocusableItem {
* If the dropdown was opened by clicking on the trigger, we automatically move to the first item
*/
moveToFirstItemWhenOpen() {
this.toggleService.openChange.subscribe(open => {
const subscription = this.toggleService.openChange.subscribe(open => {
if (open && this.toggleService.originalEvent) {
// Even if we properly waited for ngAfterViewInit, the container still wouldn't be attached to the DOM.
// So setTimeout is the only way to wait for the container to be ready to move focus to first item.
Expand All @@ -56,6 +56,8 @@ export class DropdownFocusHandler implements FocusableItem {
});
}
});

this._unlistenFuncs.push(() => subscription.unsubscribe());
}

private focusBackOnTrigger = false;
Expand All @@ -64,7 +66,7 @@ export class DropdownFocusHandler implements FocusableItem {
* Focus on the menu when it opens, and focus back on the root trigger when the whole dropdown becomes closed
*/
handleRootFocus() {
this.toggleService.openChange.subscribe(open => {
const subscription = this.toggleService.openChange.subscribe(open => {
if (!open) {
// We reset the state of the focus service both on initialization and when closing.
this.focusService.reset(this);
Expand All @@ -75,6 +77,8 @@ export class DropdownFocusHandler implements FocusableItem {
}
this.focusBackOnTrigger = open;
});

this._unlistenFuncs.push(() => subscription.unsubscribe());
}

private _trigger: HTMLElement;
Expand Down

0 comments on commit 6ea37a8

Please sign in to comment.