Skip to content

Commit 5f7680f

Browse files
authored
fix(cdk/a11y): Make focus-trap behavior consistent across zoneful/zoneless (angular#29225)
1 parent e2d70a8 commit 5f7680f

File tree

5 files changed

+20
-31
lines changed

5 files changed

+20
-31
lines changed

src/cdk/a11y/focus-trap/configurable-focus-trap-factory.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
*/
88

99
import {DOCUMENT} from '@angular/common';
10-
import {Inject, Injectable, Optional, NgZone} from '@angular/core';
10+
import {Inject, Injectable, Injector, NgZone, Optional, inject} from '@angular/core';
1111
import {InteractivityChecker} from '../interactivity-checker/interactivity-checker';
1212
import {ConfigurableFocusTrap} from './configurable-focus-trap';
1313
import {ConfigurableFocusTrapConfig} from './configurable-focus-trap-config';
14-
import {FOCUS_TRAP_INERT_STRATEGY, FocusTrapInertStrategy} from './focus-trap-inert-strategy';
1514
import {EventListenerFocusTrapInertStrategy} from './event-listener-inert-strategy';
15+
import {FOCUS_TRAP_INERT_STRATEGY, FocusTrapInertStrategy} from './focus-trap-inert-strategy';
1616
import {FocusTrapManager} from './focus-trap-manager';
1717

1818
/** Factory that allows easy instantiation of configurable focus traps. */
@@ -21,6 +21,8 @@ export class ConfigurableFocusTrapFactory {
2121
private _document: Document;
2222
private _inertStrategy: FocusTrapInertStrategy;
2323

24+
private readonly _injector = inject(Injector);
25+
2426
constructor(
2527
private _checker: InteractivityChecker,
2628
private _ngZone: NgZone,
@@ -65,6 +67,7 @@ export class ConfigurableFocusTrapFactory {
6567
this._focusTrapManager,
6668
this._inertStrategy,
6769
configObject,
70+
this._injector,
6871
);
6972
}
7073
}

src/cdk/a11y/focus-trap/configurable-focus-trap.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {NgZone} from '@angular/core';
9+
import {Injector, NgZone} from '@angular/core';
1010
import {InteractivityChecker} from '../interactivity-checker/interactivity-checker';
11+
import {ConfigurableFocusTrapConfig} from './configurable-focus-trap-config';
1112
import {FocusTrap} from './focus-trap';
12-
import {FocusTrapManager, ManagedFocusTrap} from './focus-trap-manager';
1313
import {FocusTrapInertStrategy} from './focus-trap-inert-strategy';
14-
import {ConfigurableFocusTrapConfig} from './configurable-focus-trap-config';
14+
import {FocusTrapManager, ManagedFocusTrap} from './focus-trap-manager';
1515

1616
/**
1717
* Class that allows for trapping focus within a DOM element.
@@ -41,8 +41,9 @@ export class ConfigurableFocusTrap extends FocusTrap implements ManagedFocusTrap
4141
private _focusTrapManager: FocusTrapManager,
4242
private _inertStrategy: FocusTrapInertStrategy,
4343
config: ConfigurableFocusTrapConfig,
44+
injector?: Injector,
4445
) {
45-
super(_element, _checker, _ngZone, _document, config.defer);
46+
super(_element, _checker, _ngZone, _document, config.defer, injector);
4647
this._focusTrapManager.register(this);
4748
}
4849

src/cdk/a11y/focus-trap/event-listener-inert-strategy.spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import {AfterViewInit, Component, ElementRef, Type, ViewChild, Provider} from '@angular/core';
2-
import {ComponentFixture, fakeAsync, flush, TestBed} from '@angular/core/testing';
1+
import {AfterViewInit, Component, ElementRef, Provider, Type, ViewChild} from '@angular/core';
2+
import {ComponentFixture, TestBed, fakeAsync, flush} from '@angular/core/testing';
33
import {patchElementFocus} from '../../testing/private';
44
import {
55
A11yModule,
6-
ConfigurableFocusTrapFactory,
76
ConfigurableFocusTrap,
7+
ConfigurableFocusTrapFactory,
88
EventListenerFocusTrapInertStrategy,
99
FOCUS_TRAP_INERT_STRATEGY,
1010
} from '../index';
@@ -31,6 +31,7 @@ describe('EventListenerFocusTrapInertStrategy', () => {
3131
const fixture = createComponent(SimpleFocusTrap, providers);
3232
const componentInstance = fixture.componentInstance;
3333
fixture.detectChanges();
34+
flush();
3435

3536
componentInstance.secondFocusableElement.nativeElement.focus();
3637
flush();
@@ -43,6 +44,7 @@ describe('EventListenerFocusTrapInertStrategy', () => {
4344
it('should not intercept focus if it moved outside the trap and back in again', fakeAsync(() => {
4445
const fixture = createComponent(SimpleFocusTrap, providers);
4546
fixture.detectChanges();
47+
flush();
4648
const {secondFocusableElement, outsideFocusableElement} = fixture.componentInstance;
4749

4850
outsideFocusableElement.nativeElement.focus();

src/cdk/a11y/focus-trap/focus-trap.ts

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import {
2525
booleanAttribute,
2626
inject,
2727
} from '@angular/core';
28-
import {take} from 'rxjs/operators';
2928
import {InteractivityChecker} from '../interactivity-checker/interactivity-checker';
3029

3130
/**
@@ -359,27 +358,11 @@ export class FocusTrap {
359358

360359
/** Executes a function when the zone is stable. */
361360
private _executeOnStable(fn: () => any): void {
362-
// TODO(mmalerba): Make this behave consistently across zonefull / zoneless.
363-
if (!this._ngZone.isStable) {
364-
// Subscribing `onStable` has slightly different behavior than `afterNextRender`.
365-
// `afterNextRender` does not wait for state changes queued up in a Promise
366-
// to avoid change after checked errors. In most cases we would consider this an
367-
// acceptable behavior change, the dialog at least made its best effort to focus the
368-
// first element. However, this is particularly problematic when combined with the
369-
// current behavior of the mat-radio-group, which adjusts the tabindex of its child
370-
// radios based on the selected value of the group. When the selected value is bound
371-
// via `[(ngModel)]` it hits this "state change in a promise" edge-case and can wind up
372-
// putting the focus on a radio button that is not supposed to be eligible to receive
373-
// focus. For now, we side-step this whole sequence of events by continuing to use
374-
// `onStable` in zonefull apps, but it should be noted that zoneless apps can still
375-
// suffer from this issue.
376-
this._ngZone.onStable.pipe(take(1)).subscribe(fn);
361+
// TODO: remove this conditional when injector is required in the constructor.
362+
if (this._injector) {
363+
afterNextRender(fn, {injector: this._injector});
377364
} else {
378-
if (this._injector) {
379-
afterNextRender(fn, {injector: this._injector});
380-
} else {
381-
fn();
382-
}
365+
setTimeout(fn);
383366
}
384367
}
385368
}

tools/public_api_guard/cdk/a11y.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export class CdkTrapFocus implements OnDestroy, AfterContentInit, OnChanges, DoC
127127

128128
// @public
129129
export class ConfigurableFocusTrap extends FocusTrap implements ManagedFocusTrap {
130-
constructor(_element: HTMLElement, _checker: InteractivityChecker, _ngZone: NgZone, _document: Document, _focusTrapManager: FocusTrapManager, _inertStrategy: FocusTrapInertStrategy, config: ConfigurableFocusTrapConfig);
130+
constructor(_element: HTMLElement, _checker: InteractivityChecker, _ngZone: NgZone, _document: Document, _focusTrapManager: FocusTrapManager, _inertStrategy: FocusTrapInertStrategy, config: ConfigurableFocusTrapConfig, injector?: Injector);
131131
destroy(): void;
132132
_disable(): void;
133133
_enable(): void;

0 commit comments

Comments
 (0)