Skip to content

Commit ce9d253

Browse files
crisbetoandrewseguin
authored andcommitted
fix(focus-trap): focus initial element when zone stabilizes (#4867)
* fix(focus-trap): focus initial element when zone stabilizes * Focuses the first element when the zone stabilizes, instead of when the microtask queue is empty. This avoids issues where the element might be focused before Angular is done doing change detection. * Only runs the `onStable` callback if the zone is unstable. * Avoids an extra DOM lookup by combining a couple of selectors. Fixes #4864. * chore: switch to for loop
1 parent 7e91270 commit ce9d253

File tree

2 files changed

+31
-16
lines changed

2 files changed

+31
-16
lines changed

src/demo-app/dialog/dialog-demo.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ export class DialogDemo {
7373
selector: 'demo-jazz-dialog',
7474
template: `
7575
<p>It's Jazz!</p>
76-
<p><label>How much? <input #howMuch></label></p>
76+
77+
<md-input-container>
78+
<input mdInput placeholder="How much?" #howMuch>
79+
</md-input-container>
80+
7781
<p> {{ data.message }} </p>
7882
<button type="button" (click)="dialogRef.close(howMuch.value)">Close dialog</button>
7983
<button (click)="togglePosition()">Change dimensions</button>`

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

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,24 +81,28 @@ export class FocusTrap {
8181
});
8282
}
8383

84+
/**
85+
* Waits for the zone to stabilize, then either focuses the first element that the
86+
* user specified, or the first tabbable element..
87+
*/
8488
focusInitialElementWhenReady() {
85-
this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusInitialElement());
89+
this._executeOnStable(() => this.focusInitialElement());
8690
}
8791

8892
/**
89-
* Waits for microtask queue to empty, then focuses
93+
* Waits for the zone to stabilize, then focuses
9094
* the first tabbable element within the focus trap region.
9195
*/
9296
focusFirstTabbableElementWhenReady() {
93-
this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusFirstTabbableElement());
97+
this._executeOnStable(() => this.focusFirstTabbableElement());
9498
}
9599

96100
/**
97-
* Waits for microtask queue to empty, then focuses
101+
* Waits for the zone to stabilize, then focuses
98102
* the last tabbable element within the focus trap region.
99103
*/
100104
focusLastTabbableElementWhenReady() {
101-
this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement());
105+
this._executeOnStable(() => this.focusLastTabbableElement());
102106
}
103107

104108
/**
@@ -107,18 +111,16 @@ export class FocusTrap {
107111
* @returns The boundary element.
108112
*/
109113
private _getRegionBoundary(bound: 'start' | 'end'): HTMLElement | null {
110-
let markers = [
111-
...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${bound}]`)),
112-
// Deprecated version of selector, for temporary backwards comparability:
113-
...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${bound}]`)),
114-
];
115-
116-
markers.forEach((el: HTMLElement) => {
117-
if (el.hasAttribute(`cdk-focus-${bound}`)) {
114+
// Contains the deprecated version of selector, for temporary backwards comparability.
115+
let markers = this._element.querySelectorAll(`[cdk-focus-region-${bound}], ` +
116+
`[cdk-focus-${bound}]`) as NodeListOf<HTMLElement>;
117+
118+
for (let i = 0; i < markers.length; i++) {
119+
if (markers[i].hasAttribute(`cdk-focus-${bound}`)) {
118120
console.warn(`Found use of deprecated attribute 'cdk-focus-${bound}',` +
119-
` use 'cdk-focus-region-${bound}' instead.`, el);
121+
` use 'cdk-focus-region-${bound}' instead.`, markers[i]);
120122
}
121-
});
123+
}
122124

123125
if (bound == 'start') {
124126
return markers.length ? markers[0] : this._getFirstTabbableElement(this._element);
@@ -206,6 +208,15 @@ export class FocusTrap {
206208
anchor.classList.add('cdk-focus-trap-anchor');
207209
return anchor;
208210
}
211+
212+
/** Executes a function when the zone is stable. */
213+
private _executeOnStable(fn: () => any): void {
214+
if (this._ngZone.isStable) {
215+
fn();
216+
} else {
217+
this._ngZone.onStable.first().subscribe(fn);
218+
}
219+
}
209220
}
210221

211222

0 commit comments

Comments
 (0)