Skip to content

Commit

Permalink
fix(select): don't restore focus unless an option was selected (#8964)
Browse files Browse the repository at this point in the history
Currently `mat-select` always restores focus to its trigger whenever the panel is closed. This can be problematic, because the user might've scrolled away and restoring focus would scroll the page back up. These changes switch to only restoring focus if the user made a selection.

Fixes #8915.
  • Loading branch information
crisbeto authored and jelbourn committed Jan 29, 2018
1 parent 92896a0 commit 960b7cf
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
19 changes: 19 additions & 0 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,25 @@ describe('MatSelect', () => {
expect(document.activeElement).toBe(select, 'Expected trigger to be focused.');
}));

it('should not restore focus to the host element when clicking outside', fakeAsync(() => {
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
const select = fixture.debugElement.nativeElement.querySelector('mat-select');

fixture.detectChanges();
fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click();
fixture.detectChanges();
flush();

expect(document.activeElement).toBe(select, 'Expected trigger to be focused.');

select.blur(); // Blur manually since the programmatic click might not do it.
(overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement).click();
fixture.detectChanges();
flush();

expect(document.activeElement).not.toBe(select, 'Expected trigger not to be focused.');
}));

it('should update the data binding before emitting the change event', fakeAsync(() => {
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
const instance = fixture.componentInstance;
Expand Down
9 changes: 5 additions & 4 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
];

/** Whether the select is focused. */
focused = false;
focused: boolean = false;

/** A name for this control that can be used by `mat-form-field`. */
controlType = 'mat-select';
Expand Down Expand Up @@ -548,7 +548,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
this._panelOpen = false;
this._changeDetectorRef.markForCheck();
this._onTouched();
this.focus();
}
}

Expand Down Expand Up @@ -715,8 +714,9 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
* "blur" to the panel when it opens, causing a false positive.
*/
_onBlur() {
this.focused = false;

if (!this.disabled && !this.panelOpen) {
this.focused = false;
this._onTouched();
this._changeDetectorRef.markForCheck();
this.stateChanges.next();
Expand Down Expand Up @@ -844,8 +844,9 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
).subscribe(event => {
this._onSelect(event.source);

if (!this.multiple) {
if (!this.multiple && this._panelOpen) {
this.close();
this.focus();
}
});

Expand Down

0 comments on commit 960b7cf

Please sign in to comment.