Skip to content

fix(select): don't blur label when trigger is blurred while the panel is opened #11537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2018
Merged

fix(select): don't blur label when trigger is blurred while the panel is opened #11537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/lib/select/select.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,26 @@ describe('MatSelect', () => {
expect(document.querySelectorAll('.cdk-overlay-container mat-option').length)
.toBeGreaterThan(0, 'Expected at least one option to be rendered.');
}));

it('should not consider itself as blurred if the trigger loses focus while the ' +
'panel is still open', fakeAsync(() => {
const selectElement = fixture.nativeElement.querySelector('.mat-select');
const selectInstance = fixture.componentInstance.select;

dispatchFakeEvent(selectElement, 'focus');
fixture.detectChanges();

expect(selectInstance.focused).toBe(true, 'Expected select to be focused.');

selectInstance.open();
fixture.detectChanges();
flush();
dispatchFakeEvent(selectElement, 'blur');
fixture.detectChanges();

expect(selectInstance.focused).toBe(true, 'Expected select element to remain focused.');
}));

});

describe('selection logic', () => {
Expand Down
16 changes: 13 additions & 3 deletions src/lib/select/select.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,17 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
private _disableOptionCentering: boolean = false;

/** Whether the select is focused. */
focused: boolean = false;
get focused(): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A setter needs to be defined here as well.

With this change, the API is changed as focused becomes readonly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn’t add a setter, because the MatFormFieldControl interface calls for the property to be readonly. That being said, it doesn’t look like TS actually enforces it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm okay, Lets add the setter as deprecated and then we can move it to readonly later

return this._focused || this._panelOpen;
}
/**
* @deprecated Setter to be removed as this property is intended to be readonly.
* @deletion-target 8.0.0
*/
set focused(value: boolean) {
this._focused = value;
}
private _focused = false;

/** A name for this control that can be used by `mat-form-field`. */
controlType = 'mat-select';
Expand Down Expand Up @@ -701,7 +711,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,

_onFocus() {
if (!this.disabled) {
this.focused = true;
this._focused = true;
this.stateChanges.next();
}
}
Expand All @@ -711,7 +721,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
* "blur" to the panel when it opens, causing a false positive.
*/
_onBlur() {
this.focused = false;
this._focused = false;

if (!this.disabled && !this.panelOpen) {
this._onTouched();
Expand Down