Skip to content

Commit

Permalink
fix(material/autocomplete): requireSelection sometimes not clearing v…
Browse files Browse the repository at this point in the history
…alue when editing after selection (#28628)

Fixes that if the user has `requireSelection` enabled, selects a value and then deletes a character while the input still has a value, the selection wasn't being cleared as expected.

Fixes #28432.
  • Loading branch information
crisbeto committed Feb 22, 2024
1 parent 616d44c commit 216ae0c
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 19 deletions.
58 changes: 39 additions & 19 deletions src/material/autocomplete/autocomplete-trigger.ts
Expand Up @@ -144,6 +144,9 @@ export class MatAutocompleteTrigger
/** Value of the input element when the panel was attached (even if there are no options). */
private _valueOnAttach: string | number | null;

/** Value on the previous keydown event. */
private _valueOnLastKeydown: string | null;

/** Strategy that is used to position the panel. */
private _positionStrategy: FlexibleConnectedPositionStrategy;

Expand Down Expand Up @@ -285,13 +288,7 @@ export class MatAutocompleteTrigger

/** Opens the autocomplete suggestion panel. */
openPanel(): void {
this._attachOverlay();
this._floatLabel();
// Add aria-owns attribute when the autocomplete becomes visible.
if (this._trackedModal) {
const panelId = this.autocomplete.id;
addAriaReferencedId(this._trackedModal, 'aria-owns', panelId);
}
this._openPanelInternal();
}

/** Closes the autocomplete suggestion panel. */
Expand Down Expand Up @@ -461,6 +458,8 @@ export class MatAutocompleteTrigger
event.preventDefault();
}

this._valueOnLastKeydown = this._element.nativeElement.value;

if (this.activeOption && keyCode === ENTER && this.panelOpen && !hasModifier) {
this.activeOption._selectViaInteraction();
this._resetActiveItem();
Expand All @@ -472,15 +471,15 @@ export class MatAutocompleteTrigger
if (keyCode === TAB || (isArrowKey && !hasModifier && this.panelOpen)) {
this.autocomplete._keyManager.onKeydown(event);
} else if (isArrowKey && this._canOpen()) {
this.openPanel();
this._openPanelInternal(this._valueOnLastKeydown);
}

if (isArrowKey || this.autocomplete._keyManager.activeItem !== prevActiveItem) {
this._scrollToOption(this.autocomplete._keyManager.activeItemIndex || 0);

if (this.autocomplete.autoSelectActiveOption && this.activeOption) {
if (!this._pendingAutoselectedOption) {
this._valueBeforeAutoSelection = this._element.nativeElement.value;
this._valueBeforeAutoSelection = this._valueOnLastKeydown;
}

this._pendingAutoselectedOption = this.activeOption;
Expand Down Expand Up @@ -523,7 +522,7 @@ export class MatAutocompleteTrigger
const selectedOption = this.autocomplete.options?.find(option => option.selected);

if (selectedOption) {
const display = this.autocomplete.displayWith?.(selectedOption) ?? selectedOption.value;
const display = this._getDisplayValue(selectedOption.value);

if (value !== display) {
selectedOption.deselect(false);
Expand All @@ -532,7 +531,14 @@ export class MatAutocompleteTrigger
}

if (this._canOpen() && this._document.activeElement === event.target) {
this.openPanel();
// When the `input` event fires, the input's value will have already changed. This means
// that if we take the `this._element.nativeElement.value` directly, it'll be one keystroke
// behind. This can be a problem when the user selects a value, changes a character while
// the input still has focus and then clicks away (see #28432). To work around it, we
// capture the value in `keydown` so we can use it here.
const valueOnAttach = this._valueOnLastKeydown ?? this._element.nativeElement.value;
this._valueOnLastKeydown = null;
this._openPanelInternal(valueOnAttach);
}
}
}
Expand All @@ -542,14 +548,14 @@ export class MatAutocompleteTrigger
this._canOpenOnNextFocus = true;
} else if (this._canOpen()) {
this._previousValue = this._element.nativeElement.value;
this._attachOverlay();
this._attachOverlay(this._previousValue);
this._floatLabel(true);
}
}

_handleClick(): void {
if (this._canOpen() && !this.panelOpen) {
this.openPanel();
this._openPanelInternal();
}
}

Expand Down Expand Up @@ -657,11 +663,14 @@ export class MatAutocompleteTrigger
}
}

/** Given a value, returns the string that should be shown within the input. */
private _getDisplayValue<T>(value: T): T | string {
const autocomplete = this.autocomplete;
return autocomplete && autocomplete.displayWith ? autocomplete.displayWith(value) : value;
}

private _assignOptionValue(value: any): void {
const toDisplay =
this.autocomplete && this.autocomplete.displayWith
? this.autocomplete.displayWith(value)
: value;
const toDisplay = this._getDisplayValue(value);

if (value == null) {
this._clearPreviousSelectedOption(null, false);
Expand Down Expand Up @@ -733,7 +742,17 @@ export class MatAutocompleteTrigger
});
}

private _attachOverlay(): void {
private _openPanelInternal(valueOnAttach = this._element.nativeElement.value) {
this._attachOverlay(valueOnAttach);
this._floatLabel();
// Add aria-owns attribute when the autocomplete becomes visible.
if (this._trackedModal) {
const panelId = this.autocomplete.id;
addAriaReferencedId(this._trackedModal, 'aria-owns', panelId);
}
}

private _attachOverlay(valueOnAttach: string): void {
if (!this.autocomplete && (typeof ngDevMode === 'undefined' || ngDevMode)) {
throw getMatAutocompleteMissingPanelError();
}
Expand All @@ -759,7 +778,8 @@ export class MatAutocompleteTrigger

if (overlayRef && !overlayRef.hasAttached()) {
overlayRef.attach(this._portal);
this._valueOnAttach = this._element.nativeElement.value;
this._valueOnAttach = valueOnAttach;
this._valueOnLastKeydown = null;
this._closingActionsSubscription = this._subscribeToClosingActions();
}

Expand Down
42 changes: 42 additions & 0 deletions src/material/autocomplete/autocomplete.spec.ts
Expand Up @@ -2780,6 +2780,48 @@ describe('MDC-based MatAutocomplete', () => {
expect(spy).not.toHaveBeenCalled();
subscription.unsubscribe();
}));

it('should clear the value if requireSelection is enabled and the user edits the input before clicking away', fakeAsync(() => {
const input = fixture.nativeElement.querySelector('input');
const {stateCtrl, trigger} = fixture.componentInstance;
fixture.componentInstance.requireSelection = true;
fixture.detectChanges();
tick();

// Simulate opening the input and clicking the first option.
trigger.openPanel();
fixture.detectChanges();
zone.simulateZoneExit();
(overlayContainerElement.querySelector('mat-option') as HTMLElement).click();
tick();
fixture.detectChanges();

expect(trigger.panelOpen).toBe(false);
expect(input.value).toBe('Alabama');
expect(stateCtrl.value).toEqual({code: 'AL', name: 'Alabama'});

// Simulate pressing backspace while focus is still on the input.
dispatchFakeEvent(input, 'keydown');
input.value = 'Alabam';
fixture.detectChanges();
dispatchFakeEvent(input, 'input');
fixture.detectChanges();
zone.simulateZoneExit();

expect(trigger.panelOpen).toBe(true);
expect(input.value).toBe('Alabam');
expect(stateCtrl.value).toEqual({code: 'AL', name: 'Alabama'});

// Simulate clicking away.
input.blur();
dispatchFakeEvent(document, 'click');
fixture.detectChanges();
tick();

expect(trigger.panelOpen).toBe(false);
expect(input.value).toBe('');
expect(stateCtrl.value).toBe(null);
}));
});

describe('panel closing', () => {
Expand Down

0 comments on commit 216ae0c

Please sign in to comment.