From 70f8d57b253fc6fd3c2a43a662439080912216b4 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 29 May 2017 20:33:23 +0200 Subject: [PATCH 1/2] 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. --- src/demo-app/dialog/dialog-demo.ts | 6 +++++- src/lib/core/a11y/focus-trap.ts | 33 ++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/demo-app/dialog/dialog-demo.ts b/src/demo-app/dialog/dialog-demo.ts index f0e5875d17bb..e3bd57ef0c87 100644 --- a/src/demo-app/dialog/dialog-demo.ts +++ b/src/demo-app/dialog/dialog-demo.ts @@ -72,7 +72,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 1d853ae21ed1..69e26f111383 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,13 +111,11 @@ 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) => { + Array.prototype.forEach.call(markers, (el: HTMLElement) => { if (el.hasAttribute(`cdk-focus-${bound}`)) { console.warn(`Found use of deprecated attribute 'cdk-focus-${bound}',` + ` use 'cdk-focus-region-${bound}' instead.`, el); @@ -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); + } + } } From a5852d25da6cfab12a6ceb2d0b1fef6f0246711e Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 31 May 2017 06:24:03 +0200 Subject: [PATCH 2/2] chore: switch to for loop --- src/lib/core/a11y/focus-trap.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts index 69e26f111383..2906e7fc362c 100644 --- a/src/lib/core/a11y/focus-trap.ts +++ b/src/lib/core/a11y/focus-trap.ts @@ -115,12 +115,12 @@ export class FocusTrap { let markers = this._element.querySelectorAll(`[cdk-focus-region-${bound}], ` + `[cdk-focus-${bound}]`) as NodeListOf; - Array.prototype.forEach.call(markers, (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);