Skip to content

Commit 3941bf2

Browse files
committed
fix(select): don't restore focus unless an option was selected
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.
1 parent f95f832 commit 3941bf2

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

src/lib/select/select.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,6 +2421,25 @@ describe('MatSelect', () => {
24212421
expect(document.activeElement).toBe(select, 'Expected trigger to be focused.');
24222422
}));
24232423

2424+
it('should not restore focus to the host element when clicking outside', fakeAsync(() => {
2425+
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
2426+
const select = fixture.debugElement.nativeElement.querySelector('mat-select');
2427+
2428+
fixture.detectChanges();
2429+
fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement.click();
2430+
fixture.detectChanges();
2431+
flush();
2432+
2433+
expect(document.activeElement).toBe(select, 'Expected trigger to be focused.');
2434+
2435+
select.blur(); // Blur manually since the programmatic click might not do it.
2436+
(overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement).click();
2437+
fixture.detectChanges();
2438+
flush();
2439+
2440+
expect(document.activeElement).not.toBe(select, 'Expected trigger not to be focused.');
2441+
}));
2442+
24242443
it('should update the data binding before emitting the change event', fakeAsync(() => {
24252444
const fixture = TestBed.createComponent(BasicSelectWithoutForms);
24262445
const instance = fixture.componentInstance;

src/lib/select/select.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,12 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
297297
},
298298
];
299299

300+
private _hasFocus = false;
301+
300302
/** Whether the select is focused. */
301-
focused = false;
303+
get focused() {
304+
return this._hasFocus || this._panelOpen;
305+
}
302306

303307
/** A name for this control that can be used by `mat-form-field`. */
304308
controlType = 'mat-select';
@@ -547,7 +551,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
547551
if (this._panelOpen) {
548552
this._panelOpen = false;
549553
this._changeDetectorRef.markForCheck();
550-
this.focus();
551554
}
552555
}
553556

@@ -696,7 +699,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
696699

697700
_onFocus() {
698701
if (!this.disabled) {
699-
this.focused = true;
702+
this._hasFocus = true;
700703
this.stateChanges.next();
701704
}
702705
}
@@ -706,8 +709,9 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
706709
* "blur" to the panel when it opens, causing a false positive.
707710
*/
708711
_onBlur() {
712+
this._hasFocus = false;
713+
709714
if (!this.disabled && !this.panelOpen) {
710-
this.focused = false;
711715
this._onTouched();
712716
this._changeDetectorRef.markForCheck();
713717
this.stateChanges.next();
@@ -835,8 +839,9 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
835839
).subscribe(event => {
836840
this._onSelect(event.source);
837841

838-
if (!this.multiple) {
842+
if (!this.multiple && this._panelOpen) {
839843
this.close();
844+
this.focus();
840845
}
841846
});
842847

0 commit comments

Comments
 (0)