From 54f88d76b7bb5361d5613d2d5a6fd46c21119a98 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 28 Apr 2018 10:11:27 -0400 Subject: [PATCH] fix(select): unable to toggle multi select option after using the mouse Handles the following use case: 1. Clicking on a multi-select to open it. 2. Toggling an option with the mouse. 3. Going to another option using the keyboard. 4. Trying to toggle that option with the keyboard. The issue comes from the fact that usually we manage keyboard events by keeping focus on the select trigger and managing the selection using `aria-activedescendant`. In this case, when the user clicks on a option, focus is moved to it which causes the select's keyboard handling to clash with the one that comes from `mat-option`, causing it to select and then deselect the option quickly. These changes restore focus back to the select trigger when an option is toggled. --- src/lib/select/select.spec.ts | 23 +++++++++++++++++++++++ src/lib/select/select.ts | 8 +++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index cce69647396b..c7de42141bf1 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -702,6 +702,29 @@ describe('MatSelect', () => { expect(host.getAttribute('aria-activedescendant')).toBe(options[0].id); })); + it('should restore focus to the trigger after selecting an option in multi-select mode', + fakeAsync(() => { + fixture.destroy(); + + const multiFixture = TestBed.createComponent(MultiSelect); + const instance = multiFixture.componentInstance; + + multiFixture.detectChanges(); + select = multiFixture.debugElement.query(By.css('mat-select')).nativeElement; + instance.select.open(); + multiFixture.detectChanges(); + + // Ensure that the select isn't focused to begin with. + select.blur(); + expect(document.activeElement).not.toBe(select, 'Expected trigger not to be focused.'); + + const option = overlayContainerElement.querySelector('mat-option')! as HTMLElement; + option.click(); + multiFixture.detectChanges(); + + expect(document.activeElement).toBe(select, 'Expected trigger to be focused.'); + })); + }); describe('for options', () => { diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 2396991650e6..a37141ebfd2e 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -826,7 +826,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, .withVerticalOrientation() .withHorizontalOrientation(this._isRtl() ? 'rtl' : 'ltr'); - this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => this.close()); + this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => this.close()); this._keyManager.change.pipe(takeUntil(this._destroy)).subscribe(() => { if (this._panelOpen && this.panel) { this._scrollActiveOptionIntoView(); @@ -874,6 +874,12 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, wasSelected ? option.deselect() : option.select(); this._keyManager.setActiveItem(option); this._sortValues(); + + // In case the user select the option with their mouse, we + // want to restore focus back to the trigger, in order to + // prevent the select keyboard controls from clashing with + // the ones from `mat-option`. + this.focus(); } else { this._clearSelection(option.value == null ? undefined : option);