From 485bc75159033a53c7b576a98e6c4c994487da99 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 10 Oct 2017 15:58:43 -0700 Subject: [PATCH 1/2] fix(focus-monitor): cleanup global listeners and don't require Renderer2 --- src/cdk/a11y/focus-monitor.spec.ts | 4 +- src/cdk/a11y/focus-monitor.ts | 115 ++++++++++++------ src/lib/button-toggle/button-toggle.ts | 20 ++- src/lib/button/button.ts | 6 +- src/lib/checkbox/checkbox.ts | 4 +- src/lib/expansion/expansion-panel-header.ts | 4 +- src/lib/radio/radio.ts | 9 +- src/lib/slide-toggle/slide-toggle.ts | 11 +- src/lib/slider/slider.ts | 4 +- src/lib/stepper/step-header.ts | 22 ++-- src/lib/tooltip/tooltip.ts | 8 +- .../form-field-custom-control-example.ts | 7 +- 12 files changed, 124 insertions(+), 90 deletions(-) diff --git a/src/cdk/a11y/focus-monitor.spec.ts b/src/cdk/a11y/focus-monitor.spec.ts index 693ca9e958c6..7cb6b426c6f0 100644 --- a/src/cdk/a11y/focus-monitor.spec.ts +++ b/src/cdk/a11y/focus-monitor.spec.ts @@ -10,7 +10,6 @@ import {A11yModule} from './index'; describe('FocusMonitor', () => { let fixture: ComponentFixture; let buttonElement: HTMLElement; - let buttonRenderer: Renderer2; let focusMonitor: FocusMonitor; let changeHandler: (origin: FocusOrigin) => void; @@ -28,11 +27,10 @@ describe('FocusMonitor', () => { fixture.detectChanges(); buttonElement = fixture.debugElement.query(By.css('button')).nativeElement; - buttonRenderer = fixture.componentInstance.renderer; focusMonitor = fm; changeHandler = jasmine.createSpy('focus origin change handler'); - focusMonitor.monitor(buttonElement, buttonRenderer, false).subscribe(changeHandler); + focusMonitor.monitor(buttonElement, false).subscribe(changeHandler); patchElementFocus(buttonElement); })); diff --git a/src/cdk/a11y/focus-monitor.ts b/src/cdk/a11y/focus-monitor.ts index a1d34a7ff988..cfe042352909 100644 --- a/src/cdk/a11y/focus-monitor.ts +++ b/src/cdk/a11y/focus-monitor.ts @@ -36,7 +36,6 @@ export type FocusOrigin = 'touch' | 'mouse' | 'keyboard' | 'program' | null; type MonitoredElementInfo = { unlisten: Function, checkChildren: boolean, - renderer: Renderer2, subject: Subject }; @@ -62,22 +61,53 @@ export class FocusMonitor { /** Weak map of elements being monitored to their info. */ private _elementInfo = new WeakMap(); - constructor(private _ngZone: NgZone, private _platform: Platform) { - this._ngZone.runOutsideAngular(() => this._registerDocumentEvents()); + /** A map of global objects to lists of current listeners. */ + private _unregisterGlobalListeners = () => {}; + + /** The number of elements currently being monitored. */ + private get _monitoredElementCount() { + return this._monitoredElementCountGetterSetterBacking; + } + private set _monitoredElementCount(n) { + // Register global listeners when first element is monitored. + if (!this._monitoredElementCountGetterSetterBacking && n) { + this._registerGlobalListeners(); + } + // Unregister global listeners when last element is unmonitored. + else if (this._monitoredElementCountGetterSetterBacking && !n) { + this._unregisterGlobalListeners(); + this._unregisterGlobalListeners = () => {}; + } + this._monitoredElementCountGetterSetterBacking = n; } + private _monitoredElementCountGetterSetterBacking = 0; + constructor(private _ngZone: NgZone, private _platform: Platform) {} + + /** + * @docs-private + * @deprecated renderer param no longer needed. + */ + monitor(element: HTMLElement, renderer: Renderer2, checkChildren: boolean): + Observable; /** * Monitors focus on an element and applies appropriate CSS classes. * @param element The element to monitor - * @param renderer The renderer to use to apply CSS classes to the element. * @param checkChildren Whether to count the element as focused when its children are focused. * @returns An observable that emits when the focus state of the element changes. * When the element is blurred, null will be emitted. */ + monitor(element: HTMLElement, checkChildren: boolean): Observable; monitor( element: HTMLElement, - renderer: Renderer2, - checkChildren: boolean): Observable { + renderer: Renderer2 | boolean, + checkChildren?: boolean): Observable { + // TODO(mmalerba): clean up after deprecated signature is removed. + if (!(renderer instanceof Renderer2)) { + checkChildren = renderer; + } + checkChildren = !!checkChildren; + // Do nothing if we're not on the browser platform. if (!this._platform.isBrowser) { return observableOf(null); @@ -93,10 +123,10 @@ export class FocusMonitor { let info: MonitoredElementInfo = { unlisten: () => {}, checkChildren: checkChildren, - renderer: renderer, subject: new Subject() }; this._elementInfo.set(element, info); + this._monitoredElementCount++; // Start listening. We need to listen in capture phase since focus events don't bubble. let focusListener = (event: FocusEvent) => this._onFocus(event, element); @@ -128,6 +158,7 @@ export class FocusMonitor { this._setClasses(element); this._elementInfo.delete(element); + this._monitoredElementCount--; } } @@ -142,49 +173,69 @@ export class FocusMonitor { } /** Register necessary event listeners on the document and window. */ - private _registerDocumentEvents() { + private _registerGlobalListeners() { // Do nothing if we're not on the browser platform. if (!this._platform.isBrowser) { return; } - // Note: we listen to events in the capture phase so we can detect them even if the user stops - // propagation. - // On keydown record the origin and clear any touch event that may be in progress. - document.addEventListener('keydown', () => { + let documentKeydownListener = () => { this._lastTouchTarget = null; this._setOriginForCurrentEventQueue('keyboard'); - }, true); + }; // On mousedown record the origin only if there is not touch target, since a mousedown can // happen as a result of a touch event. - document.addEventListener('mousedown', () => { + let documentMousedownListener = () => { if (!this._lastTouchTarget) { this._setOriginForCurrentEventQueue('mouse'); } - }, true); + }; // When the touchstart event fires the focus event is not yet in the event queue. This means // we can't rely on the trick used above (setting timeout of 0ms). Instead we wait 650ms to // see if a focus happens. - document.addEventListener('touchstart', (event: TouchEvent) => { + let documentTouchstartListener = (event: TouchEvent) => { if (this._touchTimeout != null) { clearTimeout(this._touchTimeout); } this._lastTouchTarget = event.target; this._touchTimeout = setTimeout(() => this._lastTouchTarget = null, TOUCH_BUFFER_MS); - - // Note that we need to cast the event options to `any`, because at the time of writing - // (TypeScript 2.5), the built-in types don't support the `addEventListener` options param. - }, supportsPassiveEventListeners() ? ({passive: true, capture: true} as any) : true); + }; // Make a note of when the window regains focus, so we can restore the origin info for the // focused element. - window.addEventListener('focus', () => { + let windowFocusListener = () => { this._windowFocused = true; setTimeout(() => this._windowFocused = false, 0); + }; + + // Note: we listen to events in the capture phase so we can detect them even if the user stops + // propagation. + this._ngZone.runOutsideAngular(() => { + document.addEventListener('keydown', documentKeydownListener, true); + document.addEventListener('mousedown', documentMousedownListener, true); + document.addEventListener('touchstart', documentTouchstartListener, + supportsPassiveEventListeners() ? ({passive: true, capture: true} as any) : true); + window.addEventListener('focus', windowFocusListener); }); + + this._unregisterGlobalListeners = () => { + document.removeEventListener('keydown', documentKeydownListener, true); + document.removeEventListener('mousedown', documentMousedownListener, true); + document.removeEventListener('touchstart', documentTouchstartListener, + supportsPassiveEventListeners() ? ({passive: true, capture: true} as any) : true); + window.removeEventListener('focus', windowFocusListener); + }; + } + + private _toggleClass(element: Element, className: string, shouldSet: boolean) { + if (shouldSet) { + element.classList.add(className); + } else { + element.classList.remove(className); + } } /** @@ -196,16 +247,11 @@ export class FocusMonitor { const elementInfo = this._elementInfo.get(element); if (elementInfo) { - const toggleClass = (className: string, shouldSet: boolean) => { - shouldSet ? elementInfo.renderer.addClass(element, className) : - elementInfo.renderer.removeClass(element, className); - }; - - toggleClass('cdk-focused', !!origin); - toggleClass('cdk-touch-focused', origin === 'touch'); - toggleClass('cdk-keyboard-focused', origin === 'keyboard'); - toggleClass('cdk-mouse-focused', origin === 'mouse'); - toggleClass('cdk-program-focused', origin === 'program'); + this._toggleClass(element, 'cdk-focused', !!origin); + this._toggleClass(element, 'cdk-touch-focused', origin === 'touch'); + this._toggleClass(element, 'cdk-keyboard-focused', origin === 'keyboard'); + this._toggleClass(element, 'cdk-mouse-focused', origin === 'mouse'); + this._toggleClass(element, 'cdk-program-focused', origin === 'program'); } } @@ -235,7 +281,7 @@ export class FocusMonitor { // result, this code will still consider it to have been caused by the touch event and will // apply the cdk-touch-focused class rather than the cdk-program-focused class. This is a // relatively small edge-case that can be worked around by using - // focusVia(parentEl, renderer, 'program') to focus the parent element. + // focusVia(parentEl, 'program') to focus the parent element. // // If we decide that we absolutely must handle this case correctly, we can do so by listening // for the first focus event after the touchstart, and then the first blur event after that @@ -323,10 +369,9 @@ export class CdkMonitorFocus implements OnDestroy { private _monitorSubscription: Subscription; @Output() cdkFocusChange = new EventEmitter(); - constructor(private _elementRef: ElementRef, private _focusMonitor: FocusMonitor, - renderer: Renderer2) { + constructor(private _elementRef: ElementRef, private _focusMonitor: FocusMonitor) { this._monitorSubscription = this._focusMonitor.monitor( - this._elementRef.nativeElement, renderer, + this._elementRef.nativeElement, this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus')) .subscribe(origin => this.cdkFocusChange.emit(origin)); } diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts index 5ffd12ddfa31..7a69e66b5fd3 100644 --- a/src/lib/button-toggle/button-toggle.ts +++ b/src/lib/button-toggle/button-toggle.ts @@ -6,30 +6,29 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor} from '@angular/cdk/a11y'; +import {coerceBooleanProperty} from '@angular/cdk/coercion'; +import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; import { + ChangeDetectionStrategy, + ChangeDetectorRef, Component, ContentChildren, Directive, ElementRef, - Renderer2, EventEmitter, + forwardRef, Input, - OnInit, OnDestroy, + OnInit, Optional, Output, QueryList, ViewChild, ViewEncapsulation, - forwardRef, - ChangeDetectionStrategy, - ChangeDetectorRef, } from '@angular/core'; -import {NG_VALUE_ACCESSOR, ControlValueAccessor} from '@angular/forms'; -import {coerceBooleanProperty} from '@angular/cdk/coercion'; +import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {CanDisable, mixinDisabled} from '@angular/material/core'; -import {FocusMonitor} from '@angular/cdk/a11y'; -import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; /** Acceptable types for a button toggle. */ export type ToggleType = 'checkbox' | 'radio'; @@ -386,7 +385,6 @@ export class MatButtonToggle implements OnInit, OnDestroy { @Optional() toggleGroupMultiple: MatButtonToggleGroupMultiple, private _changeDetectorRef: ChangeDetectorRef, private _buttonToggleDispatcher: UniqueSelectionDispatcher, - private _renderer: Renderer2, private _elementRef: ElementRef, private _focusMonitor: FocusMonitor) { @@ -421,7 +419,7 @@ export class MatButtonToggle implements OnInit, OnDestroy { if (this.buttonToggleGroup && this._value == this.buttonToggleGroup.value) { this._checked = true; } - this._focusMonitor.monitor(this._elementRef.nativeElement, this._renderer, true); + this._focusMonitor.monitor(this._elementRef.nativeElement, true); } /** Focuses the button. */ diff --git a/src/lib/button/button.ts b/src/lib/button/button.ts index 9003eb85e9b3..7afa98ef5fe6 100644 --- a/src/lib/button/button.ts +++ b/src/lib/button/button.ts @@ -6,6 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor} from '@angular/cdk/a11y'; +import {Platform} from '@angular/cdk/platform'; import { ChangeDetectionStrategy, Component, @@ -19,7 +21,6 @@ import { Self, ViewEncapsulation, } from '@angular/core'; -import {Platform} from '@angular/cdk/platform'; import { CanColor, CanDisable, @@ -28,7 +29,6 @@ import { mixinDisabled, mixinDisableRipple } from '@angular/material/core'; -import {FocusMonitor} from '@angular/cdk/a11y'; // TODO(kara): Convert attribute selectors to classes when attr maps become available @@ -141,7 +141,7 @@ export class MatButton extends _MatButtonMixinBase private _platform: Platform, private _focusMonitor: FocusMonitor) { super(renderer, elementRef); - this._focusMonitor.monitor(this._elementRef.nativeElement, this._renderer, true); + this._focusMonitor.monitor(this._elementRef.nativeElement, true); } ngOnDestroy() { diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts index 8959caeaf8a4..2d4b7db7c66b 100644 --- a/src/lib/checkbox/checkbox.ts +++ b/src/lib/checkbox/checkbox.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; import {coerceBooleanProperty} from '@angular/cdk/coercion'; import { AfterViewInit, @@ -36,7 +37,6 @@ import { mixinTabIndex, RippleRef, } from '@angular/material/core'; -import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; // Increasing integer for generating unique ids for checkbox components. @@ -209,7 +209,7 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc ngAfterViewInit() { this._focusMonitor - .monitor(this._inputElement.nativeElement, this._renderer, false) + .monitor(this._inputElement.nativeElement, false) .subscribe(focusOrigin => this._onInputFocusChange(focusOrigin)); } diff --git a/src/lib/expansion/expansion-panel-header.ts b/src/lib/expansion/expansion-panel-header.ts index 05baf2b90e93..5cbc7d07b5df 100644 --- a/src/lib/expansion/expansion-panel-header.ts +++ b/src/lib/expansion/expansion-panel-header.ts @@ -19,7 +19,6 @@ import { Host, Input, OnDestroy, - Renderer2, ViewEncapsulation, } from '@angular/core'; import {merge} from 'rxjs/observable/merge'; @@ -85,7 +84,6 @@ export class MatExpansionPanelHeader implements OnDestroy { private _parentChangeSubscription = Subscription.EMPTY; constructor( - renderer: Renderer2, @Host() public panel: MatExpansionPanel, private _element: ElementRef, private _focusMonitor: FocusMonitor, @@ -100,7 +98,7 @@ export class MatExpansionPanelHeader implements OnDestroy { ) .subscribe(() => this._changeDetectorRef.markForCheck()); - _focusMonitor.monitor(_element.nativeElement, renderer, false); + _focusMonitor.monitor(_element.nativeElement, false); } /** Height of the header while the panel is expanded. */ diff --git a/src/lib/radio/radio.ts b/src/lib/radio/radio.ts index f72da0d11940..2d675060b090 100644 --- a/src/lib/radio/radio.ts +++ b/src/lib/radio/radio.ts @@ -6,6 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; +import {coerceBooleanProperty} from '@angular/cdk/coercion'; +import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; import { AfterContentInit, AfterViewInit, @@ -38,9 +41,6 @@ import { mixinDisableRipple, RippleRef, } from '@angular/material/core'; -import {coerceBooleanProperty} from '@angular/cdk/coercion'; -import {UniqueSelectionDispatcher} from '@angular/cdk/collections'; -import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; // Increasing integer for generating unique ids for radio components. let nextUniqueId = 0; @@ -538,8 +538,7 @@ export class MatRadioButton extends _MatRadioButtonMixinBase } ngAfterViewInit() { - this._focusMonitor - .monitor(this._inputElement.nativeElement, this._renderer, false) + this._focusMonitor.monitor(this._inputElement.nativeElement, false) .subscribe(focusOrigin => this._onInputFocusChange(focusOrigin)); } diff --git a/src/lib/slide-toggle/slide-toggle.ts b/src/lib/slide-toggle/slide-toggle.ts index ef3d48ec991a..fe4138a59e2f 100644 --- a/src/lib/slide-toggle/slide-toggle.ts +++ b/src/lib/slide-toggle/slide-toggle.ts @@ -6,6 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; +import {coerceBooleanProperty} from '@angular/cdk/coercion'; +import {Platform} from '@angular/cdk/platform'; import { AfterContentInit, Attribute, @@ -22,8 +25,7 @@ import { ViewChild, ViewEncapsulation } from '@angular/core'; -import {Platform} from '@angular/cdk/platform'; -import {coerceBooleanProperty} from '@angular/cdk/coercion'; +import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import { applyCssTransform, CanColor, @@ -38,8 +40,6 @@ import { mixinTabIndex, RippleRef, } from '@angular/material/core'; -import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; -import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; // Increasing integer for generating unique ids for slide-toggle components. let nextUniqueId = 0; @@ -153,8 +153,7 @@ export class MatSlideToggle extends _MatSlideToggleMixinBase implements OnDestro ngAfterContentInit() { this._slideRenderer = new SlideToggleRenderer(this._elementRef, this._platform); - this._focusMonitor - .monitor(this._inputElement.nativeElement, this._renderer, false) + this._focusMonitor.monitor(this._inputElement.nativeElement, false) .subscribe(focusOrigin => this._onInputFocusChange(focusOrigin)); } diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 0e1033329cbd..15a675235fcd 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; import {Directionality} from '@angular/cdk/bidi'; import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion'; import { @@ -42,7 +43,6 @@ import { mixinColor, mixinDisabled, } from '@angular/material/core'; -import {FocusOrigin, FocusMonitor} from '@angular/cdk/a11y'; import {Subscription} from 'rxjs/Subscription'; /** @@ -424,7 +424,7 @@ export class MatSlider extends _MatSliderMixinBase ngOnInit() { this._focusMonitor - .monitor(this._elementRef.nativeElement, this._renderer, true) + .monitor(this._elementRef.nativeElement, true) .subscribe((origin: FocusOrigin) => { this._isActive = !!origin && origin !== 'keyboard'; this._changeDetectorRef.detectChanges(); diff --git a/src/lib/stepper/step-header.ts b/src/lib/stepper/step-header.ts index 55203b2b690f..d629665835b5 100644 --- a/src/lib/stepper/step-header.ts +++ b/src/lib/stepper/step-header.ts @@ -9,18 +9,17 @@ import {FocusMonitor} from '@angular/cdk/a11y'; import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion'; import { + ChangeDetectionStrategy, + ChangeDetectorRef, Component, + ElementRef, Input, - ViewEncapsulation, - ChangeDetectorRef, OnDestroy, - ElementRef, - Renderer2, - ChangeDetectionStrategy, + ViewEncapsulation, } from '@angular/core'; +import {Subscription} from 'rxjs/Subscription'; import {MatStepLabel} from './step-label'; import {MatStepperIntl} from './stepper-intl'; -import {Subscription} from 'rxjs/Subscription'; @Component({ @@ -78,12 +77,11 @@ export class MatStepHeader implements OnDestroy { private _optional: boolean; constructor( - public _intl: MatStepperIntl, - private _focusMonitor: FocusMonitor, - private _element: ElementRef, - renderer: Renderer2, - changeDetectorRef: ChangeDetectorRef) { - _focusMonitor.monitor(_element.nativeElement, renderer, true); + public _intl: MatStepperIntl, + private _focusMonitor: FocusMonitor, + private _element: ElementRef, + changeDetectorRef: ChangeDetectorRef) { + _focusMonitor.monitor(_element.nativeElement, true); this._intlSubscription = _intl.changes.subscribe(() => changeDetectorRef.markForCheck()); } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 8975731045c5..8e12056e2e13 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -11,15 +11,15 @@ import {Directionality} from '@angular/cdk/bidi'; import {coerceBooleanProperty} from '@angular/cdk/coercion'; import {ESCAPE} from '@angular/cdk/keycodes'; import { + ConnectionPositionPair, + HorizontalConnectionPos, OriginConnectionPosition, Overlay, + OverlayConfig, OverlayConnectionPosition, OverlayRef, - OverlayConfig, RepositionScrollStrategy, ScrollStrategy, - ConnectionPositionPair, - HorizontalConnectionPos, VerticalConnectionPos, } from '@angular/cdk/overlay'; import {Platform} from '@angular/cdk/platform'; @@ -187,7 +187,7 @@ export class MatTooltip implements OnDestroy { renderer.listen(_elementRef.nativeElement, 'mouseleave', () => this.hide()); } - _focusMonitor.monitor(_elementRef.nativeElement, renderer, false).subscribe(origin => { + _focusMonitor.monitor(_elementRef.nativeElement, false).subscribe(origin => { // Note that the focus monitor runs outside the Angular zone. if (!origin) { _ngZone.run(() => this.hide(0)); diff --git a/src/material-examples/form-field-custom-control/form-field-custom-control-example.ts b/src/material-examples/form-field-custom-control/form-field-custom-control-example.ts index 7aa65c0b762b..4a507a8eefe3 100644 --- a/src/material-examples/form-field-custom-control/form-field-custom-control-example.ts +++ b/src/material-examples/form-field-custom-control/form-field-custom-control-example.ts @@ -1,6 +1,6 @@ import {FocusMonitor} from '@angular/cdk/a11y'; import {coerceBooleanProperty} from '@angular/cdk/coercion'; -import {Component, ElementRef, Input, OnDestroy, Renderer2} from '@angular/core'; +import {Component, ElementRef, Input, OnDestroy} from '@angular/core'; import {FormBuilder, FormGroup} from '@angular/forms'; import {MatFormFieldControl} from '@angular/material/form-field'; import {Subject} from 'rxjs/Subject'; @@ -96,15 +96,14 @@ export class MyTelInput implements MatFormFieldControl, OnDestroy { this.stateChanges.next(); } - constructor(fb: FormBuilder, private fm: FocusMonitor, private elRef: ElementRef, - renderer: Renderer2) { + constructor(fb: FormBuilder, private fm: FocusMonitor, private elRef: ElementRef) { this.parts = fb.group({ 'area': '', 'exchange': '', 'subscriber': '', }); - fm.monitor(elRef.nativeElement, renderer, true).subscribe((origin) => { + fm.monitor(elRef.nativeElement, true).subscribe((origin) => { this.focused = !!origin; this.stateChanges.next(); }); From 6585fb7dd64dcb184aac9d8a414ffde1691cfac4 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 27 Oct 2017 14:57:44 -0700 Subject: [PATCH 2/2] address comments --- src/cdk/a11y/focus-monitor.ts | 37 ++++++++++++++++++----------------- src/lib/sidenav/sidenav.md | 2 +- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/cdk/a11y/focus-monitor.ts b/src/cdk/a11y/focus-monitor.ts index cfe042352909..ae8cc1f8eb3c 100644 --- a/src/cdk/a11y/focus-monitor.ts +++ b/src/cdk/a11y/focus-monitor.ts @@ -65,22 +65,7 @@ export class FocusMonitor { private _unregisterGlobalListeners = () => {}; /** The number of elements currently being monitored. */ - private get _monitoredElementCount() { - return this._monitoredElementCountGetterSetterBacking; - } - private set _monitoredElementCount(n) { - // Register global listeners when first element is monitored. - if (!this._monitoredElementCountGetterSetterBacking && n) { - this._registerGlobalListeners(); - } - // Unregister global listeners when last element is unmonitored. - else if (this._monitoredElementCountGetterSetterBacking && !n) { - this._unregisterGlobalListeners(); - this._unregisterGlobalListeners = () => {}; - } - this._monitoredElementCountGetterSetterBacking = n; - } - private _monitoredElementCountGetterSetterBacking = 0; + private _monitoredElementCount = 0; constructor(private _ngZone: NgZone, private _platform: Platform) {} @@ -126,7 +111,7 @@ export class FocusMonitor { subject: new Subject() }; this._elementInfo.set(element, info); - this._monitoredElementCount++; + this._incrementMonitoredElementCount(); // Start listening. We need to listen in capture phase since focus events don't bubble. let focusListener = (event: FocusEvent) => this._onFocus(event, element); @@ -158,7 +143,7 @@ export class FocusMonitor { this._setClasses(element); this._elementInfo.delete(element); - this._monitoredElementCount--; + this._decrementMonitoredElementCount(); } } @@ -350,6 +335,22 @@ export class FocusMonitor { this._setClasses(element); elementInfo.subject.next(null); } + + private _incrementMonitoredElementCount() { + // Register global listeners when first element is monitored. + if (++this._monitoredElementCount == 1) { + this._registerGlobalListeners(); + } + } + + private _decrementMonitoredElementCount() { + // Unregister global listeners when last element is unmonitored. + if (!--this._monitoredElementCount) { + this._unregisterGlobalListeners(); + this._unregisterGlobalListeners = () => {}; + } + } + } diff --git a/src/lib/sidenav/sidenav.md b/src/lib/sidenav/sidenav.md index 7260c637fd71..cdbcaff0baa8 100644 --- a/src/lib/sidenav/sidenav.md +++ b/src/lib/sidenav/sidenav.md @@ -81,4 +81,4 @@ is clicked or ESCAPE is pressed. To add custom logic on backdrop clic -``` \ No newline at end of file +```