Skip to content

Commit 3c55caa

Browse files
crisbetovivian-hu-zz
authored andcommitted
fix(a11y): not being able to escape disabled focus trap using arrow keys (#13133)
Currently when a focus trap is disabled, we set the `tabindex` of the anchors to -1 in order to allow people to tab out of it. This doesn't work if somebody is navigating with the arrow keys using a screen reader, because the element is still focusable which means that the screen reader will focus it eventually, causing focus to be trapped. These changes remove the `tabindex` if the focus trap is disabled instead. **Note:** An alternate approach to this can be to hide the element using `display: none`, but I opted to remove the `tabindex` in order to avoid a style recalculation. Fixes #13132.
1 parent 730e6a3 commit 3c55caa

File tree

2 files changed

+17
-5
lines changed

2 files changed

+17
-5
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe('FocusTrap', () => {
104104
fixture.componentInstance._isFocusTrapEnabled = false;
105105
fixture.detectChanges();
106106

107-
expect(anchors.every(current => current.getAttribute('tabindex') === '-1')).toBe(true);
107+
expect(anchors.every(current => !current.hasAttribute('tabindex'))).toBe(true);
108108
});
109109
});
110110

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,12 @@ export class FocusTrap {
3737

3838
/** Whether the focus trap is active. */
3939
get enabled(): boolean { return this._enabled; }
40-
set enabled(val: boolean) {
41-
this._enabled = val;
40+
set enabled(value: boolean) {
41+
this._enabled = value;
4242

4343
if (this._startAnchor && this._endAnchor) {
44-
this._startAnchor.tabIndex = this._endAnchor.tabIndex = this._enabled ? 0 : -1;
44+
this._toggleAnchorTabIndex(value, this._startAnchor);
45+
this._toggleAnchorTabIndex(value, this._endAnchor);
4546
}
4647
}
4748
private _enabled: boolean = true;
@@ -278,12 +279,23 @@ export class FocusTrap {
278279
/** Creates an anchor element. */
279280
private _createAnchor(): HTMLElement {
280281
const anchor = this._document.createElement('div');
281-
anchor.tabIndex = this._enabled ? 0 : -1;
282+
this._toggleAnchorTabIndex(this._enabled, anchor);
282283
anchor.classList.add('cdk-visually-hidden');
283284
anchor.classList.add('cdk-focus-trap-anchor');
284285
return anchor;
285286
}
286287

288+
/**
289+
* Toggles the `tabindex` of an anchor, based on the enabled state of the focus trap.
290+
* @param isEnabled Whether the focus trap is enabled.
291+
* @param anchor Anchor on which to toggle the tabindex.
292+
*/
293+
private _toggleAnchorTabIndex(isEnabled: boolean, anchor: HTMLElement) {
294+
// Remove the tabindex completely, rather than setting it to -1, because if the
295+
// element has a tabindex, the user might still hit it when navigating with the arrow keys.
296+
isEnabled ? anchor.setAttribute('tabindex', '0') : anchor.removeAttribute('tabindex');
297+
}
298+
287299
/** Executes a function when the zone is stable. */
288300
private _executeOnStable(fn: () => any): void {
289301
if (this._ngZone.isStable) {

0 commit comments

Comments
 (0)