Skip to content

Commit

Permalink
fix(focus-trap): warn if initial element is not focusable (#13960)
Browse files Browse the repository at this point in the history
Currently if the consumer sets the `cdkFocusInitial` group on a non-focusable element, nothing will happen. These changes add a warning if the app is running in dev mode.

Relates to #13953.
  • Loading branch information
crisbeto authored and vivian-hu-zz committed Nov 7, 2018
1 parent 4be1b06 commit 27347b5
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
23 changes: 23 additions & 0 deletions src/cdk/a11y/focus-trap/focus-trap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('FocusTrap', () => {
FocusTrapWithSvg,
FocusTrapWithoutFocusableElements,
FocusTrapWithAutoCapture,
FocusTrapUnfocusableTarget,
],
});

Expand Down Expand Up @@ -138,6 +139,18 @@ describe('FocusTrap', () => {
focusTrapInstance.focusLastTabbableElement();
expect(document.activeElement!.id).toBe('last');
});

it('should warn if the initial focus target is not focusable', () => {
const alternateFixture = TestBed.createComponent(FocusTrapUnfocusableTarget);
alternateFixture.detectChanges();
focusTrapInstance = fixture.componentInstance.focusTrapDirective.focusTrap;

spyOn(console, 'warn');
focusTrapInstance.focusInitialElement();

expect(console.warn).toHaveBeenCalled();
});

});

describe('special cases', () => {
Expand Down Expand Up @@ -235,6 +248,16 @@ class FocusTrapTargets {
@ViewChild(CdkTrapFocus) focusTrapDirective: CdkTrapFocus;
}

@Component({
template: `
<div cdkTrapFocus>
<div cdkFocusInitial></div>
</div>
`
})
class FocusTrapUnfocusableTarget {
@ViewChild(CdkTrapFocus) focusTrapDirective: CdkTrapFocus;
}

@Component({
template: `
Expand Down
7 changes: 7 additions & 0 deletions src/cdk/a11y/focus-trap/focus-trap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
NgZone,
OnDestroy,
DoCheck,
isDevMode,
} from '@angular/core';
import {take} from 'rxjs/operators';
import {InteractivityChecker} from '../interactivity-checker/interactivity-checker';
Expand Down Expand Up @@ -189,6 +190,12 @@ export class FocusTrap {
`will be removed in 8.0.0`, redirectToElement);
}

// Warn the consumer if the element they've pointed to
// isn't focusable, when not in production mode.
if (isDevMode() && !this._checker.isFocusable(redirectToElement)) {
console.warn(`Element matching '[cdkFocusInitial]' is not focusable.`, redirectToElement);
}

redirectToElement.focus();
return true;
}
Expand Down

0 comments on commit 27347b5

Please sign in to comment.