Skip to content

Commit 5be96d4

Browse files
committed
refactor(focus-trap): convert to directive
Refactors the focus trap to be used as a directive, rather than a component. This gives us a couple of advantages: * It can be used on the same node as other components. * It removes a level of nesting in the DOM. This makes it slightly more convenient to style projected in cases like the dialog (see #2546), where flexbox needs to be applied to the closest possible ancestor. Also includes the following improvements: * No longer triggers change detection when focus hits the start/end anchors. * Resets the anchor tab index when trapping is disabled, instead of removing elements from the DOM. * Adds missing unit tests for the disabled and cleanup logic.
1 parent b939cd8 commit 5be96d4

File tree

4 files changed

+95
-44
lines changed

4 files changed

+95
-44
lines changed

src/lib/core/a11y/focus-trap.html

Lines changed: 0 additions & 3 deletions
This file was deleted.

src/lib/core/a11y/focus-trap.spec.ts

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing';
2-
import {By} from '@angular/platform-browser';
3-
import {Component} from '@angular/core';
1+
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
2+
import {Component, ViewChild} from '@angular/core';
43
import {FocusTrap} from './focus-trap';
54
import {InteractivityChecker} from './interactivity-checker';
65
import {Platform} from '../platform/platform';
@@ -21,16 +20,15 @@ describe('FocusTrap', () => {
2120
});
2221

2322
TestBed.compileComponents();
24-
}));
2523

26-
beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => {
2724
fixture = TestBed.createComponent(FocusTrapTestApp);
28-
focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance;
25+
fixture.detectChanges();
26+
focusTrapInstance = fixture.componentInstance.focusTrap;
2927
}));
3028

3129
it('wrap focus from end to start', () => {
3230
// Because we can't mimic a real tab press focus change in a unit test, just call the
33-
// focus event handler directly.
31+
// focus event handlerdiv directly.
3432
focusTrapInstance.focusFirstTabbableElement();
3533

3634
expect(document.activeElement.nodeName.toLowerCase())
@@ -48,6 +46,30 @@ describe('FocusTrap', () => {
4846
expect(document.activeElement.nodeName.toLowerCase())
4947
.toBe(lastElement, `Expected ${lastElement} element to be focused`);
5048
});
49+
50+
it('should clean up its anchor sibling elements on destroy', () => {
51+
const rootElement = fixture.debugElement.nativeElement as HTMLElement;
52+
53+
expect(rootElement.querySelectorAll('div.cdk-visually-hidden').length).toBe(2);
54+
55+
fixture.componentInstance.renderFocusTrap = false;
56+
fixture.detectChanges();
57+
58+
expect(rootElement.querySelectorAll('div.cdk-visually-hidden').length).toBe(0);
59+
});
60+
61+
it('should set the appropriate tabindex on the anchors, based on the disabled state', () => {
62+
const anchors = Array.from(
63+
fixture.debugElement.nativeElement.querySelectorAll('div.cdk-visually-hidden')
64+
) as HTMLElement[];
65+
66+
expect(anchors.every(current => current.getAttribute('tabindex') === '0')).toBe(true);
67+
68+
fixture.componentInstance.isFocusTrapDisabled = true;
69+
fixture.detectChanges();
70+
71+
expect(anchors.every(current => current.getAttribute('tabindex') === '-1')).toBe(true);
72+
});
5173
});
5274

5375
describe('with focus targets', () => {
@@ -61,11 +83,10 @@ describe('FocusTrap', () => {
6183
});
6284

6385
TestBed.compileComponents();
64-
}));
6586

66-
beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => {
6787
fixture = TestBed.createComponent(FocusTrapTargetTestApp);
68-
focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance;
88+
fixture.detectChanges();
89+
focusTrapInstance = fixture.componentInstance.focusTrap;
6990
}));
7091

7192
it('should be able to prioritize the first focus target', () => {
@@ -87,13 +108,17 @@ describe('FocusTrap', () => {
87108

88109
@Component({
89110
template: `
90-
<cdk-focus-trap>
111+
<cdk-focus-trap *ngIf="renderFocusTrap" [disabled]="isFocusTrapDisabled">
91112
<input>
92113
<button>SAVE</button>
93114
</cdk-focus-trap>
94115
`
95116
})
96-
class FocusTrapTestApp { }
117+
class FocusTrapTestApp {
118+
@ViewChild(FocusTrap) focusTrap: FocusTrap;
119+
renderFocusTrap = true;
120+
isFocusTrapDisabled = false;
121+
}
97122

98123

99124
@Component({
@@ -106,4 +131,6 @@ class FocusTrapTestApp { }
106131
</cdk-focus-trap>
107132
`
108133
})
109-
class FocusTrapTargetTestApp { }
134+
class FocusTrapTargetTestApp {
135+
@ViewChild(FocusTrap) focusTrap: FocusTrap;
136+
}

src/lib/core/a11y/focus-trap.ts

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component, ViewEncapsulation, ViewChild, ElementRef, Input, NgZone} from '@angular/core';
1+
import {Directive, ElementRef, Input, NgZone, AfterViewInit, OnDestroy} from '@angular/core';
22
import {InteractivityChecker} from './interactivity-checker';
33
import {coerceBooleanProperty} from '../coercion/boolean-property';
44

@@ -11,48 +11,72 @@ import {coerceBooleanProperty} from '../coercion/boolean-property';
1111
* Things like tabIndex > 0, flex `order`, and shadow roots can cause to two to misalign.
1212
* This will be replaced with a more intelligent solution before the library is considered stable.
1313
*/
14-
@Component({
15-
moduleId: module.id,
16-
selector: 'cdk-focus-trap, focus-trap',
17-
templateUrl: 'focus-trap.html',
18-
encapsulation: ViewEncapsulation.None,
14+
@Directive({
15+
selector: 'cdk-focus-trap, focus-trap, [cdk-focus-trap], [focus-trap]',
1916
})
20-
export class FocusTrap {
21-
@ViewChild('trappedContent') trappedContent: ElementRef;
17+
export class FocusTrap implements AfterViewInit, OnDestroy {
18+
private _startAnchor: HTMLElement = this._createAnchor();
19+
private _endAnchor: HTMLElement = this._createAnchor();
2220

2321
/** Whether the focus trap is active. */
2422
@Input()
2523
get disabled(): boolean { return this._disabled; }
26-
set disabled(val: boolean) { this._disabled = coerceBooleanProperty(val); }
24+
set disabled(val: boolean) {
25+
this._disabled = coerceBooleanProperty(val);
26+
this._startAnchor.tabIndex = this._endAnchor.tabIndex = this._disabled ? -1 : 0;
27+
}
2728
private _disabled: boolean = false;
2829

29-
constructor(private _checker: InteractivityChecker, private _ngZone: NgZone) { }
30+
constructor(
31+
private _checker: InteractivityChecker,
32+
private _ngZone: NgZone,
33+
private _elementRef: ElementRef) { }
34+
35+
ngAfterViewInit() {
36+
this._ngZone.runOutsideAngular(() => {
37+
this._elementRef.nativeElement
38+
.insertAdjacentElement('beforebegin', this._startAnchor)
39+
.addEventListener('focus', () => this.focusLastTabbableElement());
40+
41+
this._elementRef.nativeElement
42+
.insertAdjacentElement('afterend', this._endAnchor)
43+
.addEventListener('focus', () => this.focusFirstTabbableElement());
44+
});
45+
}
46+
47+
ngOnDestroy() {
48+
if (this._startAnchor.parentNode) {
49+
this._startAnchor.parentNode.removeChild(this._startAnchor);
50+
}
51+
52+
if (this._endAnchor.parentNode) {
53+
this._endAnchor.parentNode.removeChild(this._endAnchor);
54+
}
55+
56+
this._startAnchor = this._endAnchor = null;
57+
}
3058

3159
/**
3260
* Waits for microtask queue to empty, then focuses the first tabbable element within the focus
3361
* trap region.
3462
*/
3563
focusFirstTabbableElementWhenReady() {
36-
this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
37-
this.focusFirstTabbableElement();
38-
});
64+
this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusFirstTabbableElement());
3965
}
4066

4167
/**
4268
* Waits for microtask queue to empty, then focuses the last tabbable element within the focus
4369
* trap region.
4470
*/
4571
focusLastTabbableElementWhenReady() {
46-
this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
47-
this.focusLastTabbableElement();
48-
});
72+
this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement());
4973
}
5074

5175
/**
5276
* Focuses the first tabbable element within the focus trap region.
5377
*/
5478
focusFirstTabbableElement() {
55-
let rootElement = this.trappedContent.nativeElement;
79+
let rootElement = this._elementRef.nativeElement;
5680
let redirectToElement = rootElement.querySelector('[cdk-focus-start]') as HTMLElement ||
5781
this._getFirstTabbableElement(rootElement);
5882

@@ -65,14 +89,13 @@ export class FocusTrap {
6589
* Focuses the last tabbable element within the focus trap region.
6690
*/
6791
focusLastTabbableElement() {
68-
let rootElement = this.trappedContent.nativeElement;
69-
let focusTargets = rootElement.querySelectorAll('[cdk-focus-end]');
92+
let focusTargets = this._elementRef.nativeElement.querySelectorAll('[cdk-focus-end]');
7093
let redirectToElement: HTMLElement = null;
7194

7295
if (focusTargets.length) {
7396
redirectToElement = focusTargets[focusTargets.length - 1] as HTMLElement;
7497
} else {
75-
redirectToElement = this._getLastTabbableElement(rootElement);
98+
redirectToElement = this._getLastTabbableElement(this._elementRef.nativeElement);
7699
}
77100

78101
if (redirectToElement) {
@@ -114,4 +137,11 @@ export class FocusTrap {
114137

115138
return null;
116139
}
140+
141+
private _createAnchor(): HTMLElement {
142+
let anchor = document.createElement('div');
143+
anchor.tabIndex = 0;
144+
anchor.classList.add('cdk-visually-hidden');
145+
return anchor;
146+
}
117147
}

src/lib/sidenav/sidenav.scss

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,13 @@
134134
}
135135

136136
.mat-sidenav-focus-trap {
137+
box-sizing: border-box;
137138
height: 100%;
139+
display: block;
140+
overflow-y: auto; // TODO(kara): revisit scrolling behavior for sidenavs
138141

139-
> .cdk-focus-trap-content {
140-
box-sizing: border-box;
141-
height: 100%;
142-
overflow-y: auto; // TODO(kara): revisit scrolling behavior for sidenavs
143-
144-
// Prevents unnecessary repaints while scrolling.
145-
transform: translateZ(0);
146-
}
142+
// Prevents unnecessary repaints while scrolling.
143+
transform: translateZ(0);
147144
}
148145

149146
.mat-sidenav-invalid {

0 commit comments

Comments
 (0)