Skip to content

Commit

Permalink
fix(sidenav): move focus into sidenav in side mode if autoFocu… (#17995)
Browse files Browse the repository at this point in the history
This PR is the result of the discussions in #17967. Currently we don't move focus into the sidenav if it's in `side` mode, because we don't know the context that the component is used in, however in some cases it makes sense to move focus anyway. These changes make it so that if the consumer explicitly opted into `autoFocus`, we always move focus into the sidenav, no matter what mode it's in.
  • Loading branch information
crisbeto authored and mmalerba committed Dec 29, 2019
1 parent 5e9ca28 commit c9856ee
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 20 deletions.
20 changes: 16 additions & 4 deletions src/material/sidenav/drawer.spec.ts
Expand Up @@ -518,7 +518,7 @@ describe('MatDrawer', () => {
expect(document.activeElement).toBe(firstFocusableElement);
}));

it('should not trap focus when opened in "side" mode', fakeAsync(() => {
it('should not auto-focus by default when opened in "side" mode', fakeAsync(() => {
testComponent.mode = 'side';
fixture.detectChanges();
lastFocusableElement.focus();
Expand All @@ -530,6 +530,19 @@ describe('MatDrawer', () => {
expect(document.activeElement).toBe(lastFocusableElement);
}));

it('should auto-focus when opened in "side" mode when enabled explicitly', fakeAsync(() => {
drawer.autoFocus = true;
testComponent.mode = 'side';
fixture.detectChanges();
lastFocusableElement.focus();

drawer.open();
fixture.detectChanges();
tick();

expect(document.activeElement).toBe(firstFocusableElement);
}));

it('should focus the drawer if there are no focusable elements', fakeAsync(() => {
fixture.destroy();

Expand All @@ -545,7 +558,7 @@ describe('MatDrawer', () => {
}));

it('should be able to disable auto focus', fakeAsync(() => {
testComponent.autoFocus = false;
drawer.autoFocus = false;
testComponent.mode = 'push';
fixture.detectChanges();
lastFocusableElement.focus();
Expand Down Expand Up @@ -981,15 +994,14 @@ class DrawerDynamicPosition {
// to be focusable across all platforms.
template: `
<mat-drawer-container>
<mat-drawer position="start" [mode]="mode" [autoFocus]="autoFocus">
<mat-drawer position="start" [mode]="mode">
<input type="text" class="input1"/>
</mat-drawer>
<input type="text" class="input2"/>
</mat-drawer-container>`,
})
class DrawerWithFocusableElements {
mode: string = 'over';
autoFocus = true;
}

@Component({
Expand Down
38 changes: 23 additions & 15 deletions src/material/sidenav/drawer.ts
Expand Up @@ -173,11 +173,22 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
set disableClose(value: boolean) { this._disableClose = coerceBooleanProperty(value); }
private _disableClose: boolean = false;

/** Whether the drawer should focus the first focusable element automatically when opened. */
/**
* Whether the drawer should focus the first focusable element automatically when opened.
* Defaults to false in when `mode` is set to `side`, otherwise defaults to `true`. If explicitly
* enabled, focus will be moved into the sidenav in `side` mode as well.
*/
@Input()
get autoFocus(): boolean { return this._autoFocus; }
get autoFocus(): boolean {
const value = this._autoFocus;

// Note that usually we disable auto focusing in `side` mode, because we don't know how the
// sidenav is being used, but in some cases it still makes sense to do it. If the consumer
// explicitly enabled `autoFocus`, we take it as them always wanting to enable it.
return value == null ? this.mode !== 'side' : value;
}
set autoFocus(value: boolean) { this._autoFocus = coerceBooleanProperty(value); }
private _autoFocus: boolean = true;
private _autoFocus: boolean | undefined;

/**
* Whether the drawer is opened. We overload this because we trigger an event when it
Expand Down Expand Up @@ -253,11 +264,6 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
*/
readonly _modeChanged = new Subject<void>();

get _isFocusTrapEnabled(): boolean {
// The focus trap is only enabled when the drawer is open in any mode other than side.
return this.opened && this.mode !== 'side';
}

constructor(private _elementRef: ElementRef<HTMLElement>,
private _focusTrapFactory: FocusTrapFactory,
private _focusMonitor: FocusMonitor,
Expand All @@ -276,9 +282,7 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
this._elementFocusedBeforeDrawerWasOpened = this._doc.activeElement as HTMLElement;
}

if (this._isFocusTrapEnabled && this._focusTrap) {
this._trapFocus();
}
this._takeFocus();
} else {
this._restoreFocus();
}
Expand Down Expand Up @@ -316,9 +320,12 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
});
}

/** Traps focus inside the drawer. */
private _trapFocus() {
if (!this.autoFocus) {
/**
* Moves focus into the drawer. Note that this works even if
* the focus trap is disabled in `side` mode.
*/
private _takeFocus() {
if (!this.autoFocus || !this._focusTrap) {
return;
}

Expand Down Expand Up @@ -428,7 +435,8 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
/** Updates the enabled state of the focus trap. */
private _updateFocusTrapState() {
if (this._focusTrap) {
this._focusTrap.enabled = this._isFocusTrapEnabled;
// The focus trap is only enabled when the drawer is open in any mode other than side.
this._focusTrap.enabled = this.opened && this.mode !== 'side';
}
}

Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/material/sidenav.d.ts
Expand Up @@ -8,7 +8,6 @@ export declare class MatDrawer implements AfterContentInit, AfterContentChecked,
_animationState: 'open-instant' | 'open' | 'void';
readonly _closedStream: Observable<void>;
_container?: MatDrawerContainer | undefined;
readonly _isFocusTrapEnabled: boolean;
readonly _modeChanged: Subject<void>;
readonly _openedStream: Observable<void>;
readonly _width: number;
Expand Down

0 comments on commit c9856ee

Please sign in to comment.