Skip to content
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.

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

More robust autocomplete #1860

Merged
merged 14 commits into from
May 3, 2023
Merged
2 changes: 1 addition & 1 deletion e2e/integration/LinkingChildToSchool.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("Scenario: Linking a child to a school - E2E test", function () {

// choose the school to add
cy.contains("mat-form-field", "School")
.find("[matInput]")
.find("[matInput]:visible")
.type("E2E School{enter}");

// save school in child profile
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,40 @@
<!--Search-->
<input
[hidden]="!focused"
#inputElement
[formControl]="autocompleteForm"
matInput
style="text-overflow: ellipsis"
[matAutocomplete]="autoSuggestions"
(focusin)="onFocusIn()"
(focusout)="onFocusOut($event)"
/>

<!--Display-->
<input
[hidden]="focused"
[disabled]="_disabled"
matInput
style="text-overflow: ellipsis"
(focusin)="showAutocomplete()"
(focusout)="showAutocomplete()"
[value]="displayText"
/>

<mat-autocomplete
[disableRipple]="true"
#autoSuggestions="matAutocomplete"
(optionSelected)="select($event.option.value)"
autoActiveFirstOption
[hideSingleSelectionIndicator]="multi"
>
<mat-option
*ngFor="let item of autocompleteSuggestedOptions | async"
[value]="item"
>
<mat-checkbox *ngIf="multi" [checked]="item.selected"></mat-checkbox>
<ng-container *ngIf="!templateRef; else useTemplate">{{ item.asString }}</ng-container>
<ng-container *ngIf="!templateRef; else useTemplate">{{
item.asString
}}</ng-container>
<ng-template
#useTemplate
[ngTemplateOutlet]="templateRef"
Expand All @@ -28,7 +45,12 @@
*ngIf="createOption && showAddOption && inputElement.value"
[value]="inputElement.value"
>
<em i18n="Label for adding an option in a dropdown|e.g. Add option My new Option">Add
option</em> {{ inputElement.value }}
<em
i18n="
Label for adding an option in a dropdown|e.g. Add option My new Option
"
>Add option</em
>
{{ inputElement.value }}
</mat-option>
</mat-autocomplete>
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { NoopAnimationsModule } from "@angular/platform-browser/animations";
import { MatDialogModule } from "@angular/material/dialog";
import { TestbedHarnessEnvironment } from "@angular/cdk/testing/testbed";
import { HarnessLoader } from "@angular/cdk/testing";
import { MatInputHarness } from "@angular/material/input/testing";
import { MatAutocompleteHarness } from "@angular/material/autocomplete/testing";
import {
FormControl,
Expand All @@ -24,6 +23,8 @@ import {
NgForm,
Validators,
} from "@angular/forms";
import { genders } from "../../../child-dev-project/children/model/genders";
import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service";

describe("BasicAutocompleteComponent", () => {
let component: BasicAutocompleteComponent<any, any>;
Expand Down Expand Up @@ -92,9 +93,7 @@ describe("BasicAutocompleteComponent", () => {
component.ngOnChanges({ value: true, options: true, valueMapper: true });
fixture.detectChanges();

expect(component.autocompleteForm).toHaveValue("First Child");
const inputElement = await loader.getHarness(MatInputHarness);
await expectAsync(inputElement.getValue()).toBeResolvedTo("First Child");
expect(component.displayText).toBe("First Child");
});

it("should have the correct entity selected when it's name is entered", () => {
Expand All @@ -108,7 +107,7 @@ describe("BasicAutocompleteComponent", () => {
expect(component.value).toBe(child1.getId());
});

it("should reset if nothing has been selected", fakeAsync(() => {
it("should reset if leaving empty autocomplete", fakeAsync(() => {
const first = Child.create("First");
const second = Child.create("Second");
component.options = [first, second];
Expand All @@ -117,7 +116,7 @@ describe("BasicAutocompleteComponent", () => {
component.select({ asValue: first.getId() } as any);
expect(component.value).toBe(first.getId());

component.autocompleteForm.setValue("Non existent");
component.autocompleteForm.setValue("");
component.onFocusOut({} as any);
tick(200);

Expand Down Expand Up @@ -145,25 +144,30 @@ describe("BasicAutocompleteComponent", () => {
expect(options).toHaveSize(3);

await options[2].click();
// When browser is not in foreground, this doesn't happen automatically
component.autocomplete.openPanel();
fixture.detectChanges();
await options[1].click();

expect(component.value).toEqual([0, 2]);
});

it("should clear the input when focusing in multi select mode", fakeAsync(() => {
it("should switch the input when focusing in multi select mode", fakeAsync(() => {
component.multi = true;
component.options = ["some", "values", "and", "other", "options"];
component.value = ["some", "values"];
component.ngOnChanges({ value: true, options: true });
expect(component.autocompleteForm).toHaveValue("some, values");
expect(component.displayText).toBe("some, values");

component.onFocusIn();
component.showAutocomplete();
expect(component.autocompleteForm).toHaveValue("");
expect(component.focused).toBeTrue();

component.onFocusOut({} as any);
tick(200);

expect(component.autocompleteForm).toHaveValue("some, values");
expect(component.displayText).toBe("some, values");
expect(component.focused).toBeFalse();
}));

it("should update the error state if the form is invalid", () => {
Expand All @@ -178,4 +182,45 @@ describe("BasicAutocompleteComponent", () => {

expect(component.errorState).toBeTrue();
});

it("should create new option", fakeAsync(() => {
const newOption = "new option";
const confirmationSpy = spyOn(
TestBed.inject<ConfirmationDialogService>(ConfirmationDialogService),
"getConfirmation"
);
component.createOption = (id) => ({ id: id, label: id });
const createOptionEventSpy = spyOn(
component,
"createOption"
).and.callThrough();
component.options = genders;
const initialValue = genders[0].id;
component.value = initialValue;
component.valueMapper = (o) => o.id;

component.ngOnChanges({ value: true, options: true, valueMapper: true });

component.showAutocomplete();
component.autocompleteForm.setValue(newOption);

// decline confirmation for new option
confirmationSpy.and.resolveTo(false);
component.select(newOption);

tick();
expect(confirmationSpy).toHaveBeenCalled();
expect(createOptionEventSpy).not.toHaveBeenCalled();
expect(component.value).toEqual(initialValue);

// confirm new option
confirmationSpy.calls.reset();
confirmationSpy.and.resolveTo(true);
component.select(newOption);

tick();
expect(confirmationSpy).toHaveBeenCalled();
expect(createOptionEventSpy).toHaveBeenCalledWith(newOption);
expect(component.value).toEqual(newOption);
}));
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@ import {
MatAutocompleteModule,
MatAutocompleteTrigger,
} from "@angular/material/autocomplete";
import { concat, of, skip } from "rxjs";
import { MatCheckboxModule } from "@angular/material/checkbox";
import { distinctUntilChanged, filter, map, startWith } from "rxjs/operators";
import {
distinctUntilChanged,
filter,
map,
skip,
startWith,
} from "rxjs/operators";
import { ConfirmationDialogService } from "../../confirmation-dialog/confirmation-dialog.service";
import { ErrorStateMatcher } from "@angular/material/core";
import { CustomFormControlDirective } from "./custom-form-control.directive";
import { coerceBooleanProperty } from "@angular/cdk/coercion";
import { concat, of } from "rxjs";

interface SelectableOption<O, V> {
initial: O;
Expand Down Expand Up @@ -82,7 +88,14 @@ export class BasicAutocompleteComponent<O, V = O>
startWith([] as SelectableOption<O, V>[])
);
showAddOption = false;
private delayedBlur: any;

get displayText() {
Copy link
Member

Choose a reason for hiding this comment

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

isn't this going against #1087 and we should rather set a variable instead of a getter?

const values: V[] = Array.isArray(this.value) ? this.value : [this.value];

return values
.map((v) => this._options.find((o) => o.asValue === v)?.asString)
.join(", ");
}

get disabled(): boolean {
return this._disabled;
Expand Down Expand Up @@ -136,10 +149,25 @@ export class BasicAutocompleteComponent<O, V = O>
}

showAutocomplete() {
this.autocompleteSuggestedOptions = concat(
of(this._options),
this.autocompleteSuggestedOptions.pipe(skip(1))
);
if (this.multi) {
this.autocompleteForm.setValue("");
} else {
// cannot setValue to "" here because the current selection would be lost
this.autocompleteForm.setValue(this.displayText);
this.autocompleteSuggestedOptions = concat(
of(this._options),
this.autocompleteSuggestedOptions.pipe(skip(1))
);
}
setTimeout(() => {
this.inputElement.focus();

// select all text for easy overwriting when typing to search for options
(
this.inputElement._elementRef.nativeElement as HTMLInputElement
).select();
});
this.focus();
}

private updateAutocomplete(inputText: string): SelectableOption<O, V>[] {
Expand All @@ -160,24 +188,9 @@ export class BasicAutocompleteComponent<O, V = O>
this._options.forEach(
(o) => (o.selected = (this.value as V[])?.includes(o.asValue))
);
this.displaySelectedOptions();
} else {
const selected = this._options.find(
({ asValue }) => asValue === this.value
);
this.autocompleteForm.setValue(selected?.asString ?? "");
}
}

private displaySelectedOptions() {
this.autocompleteForm.setValue(
this._options
.filter((o) => o.selected)
.map((o) => o.asString)
.join(", ")
);
}

select(selected: string | SelectableOption<O, V>) {
if (typeof selected === "string") {
this.createNewOption(selected);
Expand All @@ -202,6 +215,10 @@ export class BasicAutocompleteComponent<O, V = O>
const newOption = this.toSelectableOption(this.createOption(option));
this._options.push(newOption);
this.select(newOption);
} else {
// continue editing
this.showAutocomplete();
this.autocompleteForm.setValue(option);
}
}

Expand All @@ -212,10 +229,8 @@ export class BasicAutocompleteComponent<O, V = O>
.filter((o) => o.selected)
.map((o) => o.asValue);
// re-open autocomplete to select next option
this.autocompleteForm.setValue("");
setTimeout(() => this.autocomplete.openPanel(), 100);
this.showAutocomplete();
} else {
this.autocompleteForm.setValue(option.asString);
this.value = option.asValue;
}
}
Expand All @@ -229,49 +244,23 @@ export class BasicAutocompleteComponent<O, V = O>
};
}

onFocusIn() {
clearTimeout(this.delayedBlur);
if (!this.focused) {
if (this.multi) {
this.autocompleteForm.setValue("");
} else {
this.showAutocomplete();
}
this.focus();
}
}

onFocusOut(event: FocusEvent) {
if (
!this.elementRef.nativeElement.contains(event.relatedTarget as Element)
) {
// use short timeout in order for creating an option to work
this.delayedBlur = setTimeout(() => this.notifyFocusOut(), 200);
}
}

private notifyFocusOut() {
if (this.multi) {
this.displaySelectedOptions();
} else {
const inputValue = this.autocompleteForm.value;
const selectedOption = this._options.find(
({ asValue }) => asValue === this._value
);
if (selectedOption?.asString !== inputValue) {
// try to select the option that matches the input string
const matchingOption = this._options.find(
({ asString }) => asString.toLowerCase() === inputValue.toLowerCase()
);
this.select(matchingOption);
if (!this.multi && this.autocompleteForm.value === "") {
this.select(undefined);
}
this.blur();
}
this.blur();
}

onContainerClick(event: MouseEvent) {
if ((event.target as Element).tagName.toLowerCase() != "input") {
this.inputElement.focus();
if (
!this._disabled &&
(event.target as Element).tagName.toLowerCase() != "input"
) {
this.showAutocomplete();
}
}

Expand Down