From ce9d253331cc5c319b8af270888faae8ed841c5f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 5 Jun 2017 23:44:42 +0200 Subject: [PATCH] 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 --- src/demo-app/dialog/dialog-demo.ts | 6 ++++- src/lib/core/a11y/focus-trap.ts | 39 +++++++++++++++++++----------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/demo-app/dialog/dialog-demo.ts b/src/demo-app/dialog/dialog-demo.ts index 4c2b2282d8d..5a572e2b781 100644 --- a/src/demo-app/dialog/dialog-demo.ts +++ b/src/demo-app/dialog/dialog-demo.ts @@ -73,7 +73,11 @@ export class DialogDemo { selector: 'demo-jazz-dialog', template: `

It's Jazz!

-

+ + + + +

{{ data.message }}

` diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts index 1d853ae21ed..2906e7fc362 100644 --- a/src/lib/core/a11y/focus-trap.ts +++ b/src/lib/core/a11y/focus-trap.ts @@ -81,24 +81,28 @@ export class FocusTrap { }); } + /** + * Waits for the zone to stabilize, then either focuses the first element that the + * user specified, or the first tabbable element.. + */ focusInitialElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusInitialElement()); + this._executeOnStable(() => this.focusInitialElement()); } /** - * Waits for microtask queue to empty, then focuses + * Waits for the zone to stabilize, then focuses * the first tabbable element within the focus trap region. */ focusFirstTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusFirstTabbableElement()); + this._executeOnStable(() => this.focusFirstTabbableElement()); } /** - * Waits for microtask queue to empty, then focuses + * Waits for the zone to stabilize, then focuses * the last tabbable element within the focus trap region. */ focusLastTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement()); + this._executeOnStable(() => this.focusLastTabbableElement()); } /** @@ -107,18 +111,16 @@ export class FocusTrap { * @returns The boundary element. */ private _getRegionBoundary(bound: 'start' | 'end'): HTMLElement | null { - let markers = [ - ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${bound}]`)), - // Deprecated version of selector, for temporary backwards comparability: - ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${bound}]`)), - ]; + // Contains the deprecated version of selector, for temporary backwards comparability. + let markers = this._element.querySelectorAll(`[cdk-focus-region-${bound}], ` + + `[cdk-focus-${bound}]`) as NodeListOf; - markers.forEach((el: HTMLElement) => { - if (el.hasAttribute(`cdk-focus-${bound}`)) { + for (let i = 0; i < markers.length; i++) { + if (markers[i].hasAttribute(`cdk-focus-${bound}`)) { console.warn(`Found use of deprecated attribute 'cdk-focus-${bound}',` + - ` use 'cdk-focus-region-${bound}' instead.`, el); + ` use 'cdk-focus-region-${bound}' instead.`, markers[i]); } - }); + } if (bound == 'start') { return markers.length ? markers[0] : this._getFirstTabbableElement(this._element); @@ -206,6 +208,15 @@ export class FocusTrap { anchor.classList.add('cdk-focus-trap-anchor'); return anchor; } + + /** Executes a function when the zone is stable. */ + private _executeOnStable(fn: () => any): void { + if (this._ngZone.isStable) { + fn(); + } else { + this._ngZone.onStable.first().subscribe(fn); + } + } }