Skip to content

Commit

Permalink
fix(cdk/a11y): error if FocusMonitor is used on non-element nodes (#2…
Browse files Browse the repository at this point in the history
…0640)

* Fixes an error thrown when trying to stop monitoring a non-element node. Now `FocusMonitor.monitor` is a noop.
* Fixes an error on init if the `cdkMonitorFocus` directive is placed on a non-element node.

Related to #20632.
  • Loading branch information
crisbeto committed Oct 2, 2020
1 parent dd49fa6 commit 9edab81
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
16 changes: 16 additions & 0 deletions src/cdk/a11y/focus-monitor/focus-monitor.spec.ts
Expand Up @@ -351,6 +351,7 @@ describe('cdkMonitorFocus', () => {
ComplexComponentWithMonitorElementFocus,
ComplexComponentWithMonitorSubtreeFocus,
ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus,
FocusMonitorOnCommentNode,
],
}).compileComponents();
});
Expand Down Expand Up @@ -548,6 +549,14 @@ describe('cdkMonitorFocus', () => {
expect(childElement.classList).toContain('cdk-keyboard-focused');
}));
});

it('should not throw when trying to monitor focus on a non-element node', () => {
expect(() => {
const fixture = TestBed.createComponent(FocusMonitorOnCommentNode);
fixture.detectChanges();
fixture.destroy();
}).not.toThrow();
});
});

describe('FocusMonitor observable stream', () => {
Expand Down Expand Up @@ -610,7 +619,14 @@ class ComplexComponentWithMonitorElementFocus {}
})
class ComplexComponentWithMonitorSubtreeFocus {}


@Component({
template: `<div cdkMonitorSubtreeFocus><button cdkMonitorElementFocus></button></div>`
})
class ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus {}


@Component({
template: `<ng-container cdkMonitorElementFocus></ng-container>`
})
class FocusMonitorOnCommentNode {}
15 changes: 8 additions & 7 deletions src/cdk/a11y/focus-monitor/focus-monitor.ts
Expand Up @@ -225,13 +225,13 @@ export class FocusMonitor implements OnDestroy {

monitor(element: HTMLElement | ElementRef<HTMLElement>,
checkChildren: boolean = false): Observable<FocusOrigin> {
// Do nothing if we're not on the browser platform.
if (!this._platform.isBrowser) {
const nativeElement = coerceElement(element);

// Do nothing if we're not on the browser platform or the passed in node isn't an element.
if (!this._platform.isBrowser || nativeElement.nodeType !== 1) {
return observableOf(null);
}

const nativeElement = coerceElement(element);

// If the element is inside the shadow DOM, we need to bind our focus/blur listeners to
// the shadow root, rather than the `document`, because the browser won't emit focus events
// to the `document`, if focus is moving within the same shadow root.
Expand Down Expand Up @@ -570,10 +570,11 @@ export class CdkMonitorFocus implements AfterViewInit, OnDestroy {
constructor(private _elementRef: ElementRef<HTMLElement>, private _focusMonitor: FocusMonitor) {}

ngAfterViewInit() {
const element = this._elementRef.nativeElement;
this._monitorSubscription = this._focusMonitor.monitor(
this._elementRef,
this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus'))
.subscribe(origin => this.cdkFocusChange.emit(origin));
element,
element.nodeType === 1 && element.hasAttribute('cdkMonitorSubtreeFocus'))
.subscribe(origin => this.cdkFocusChange.emit(origin));
}

ngOnDestroy() {
Expand Down

0 comments on commit 9edab81

Please sign in to comment.