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.

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

[UI] Make sure form validation displays non-valid fields as red in all forms #7064

Merged
merged 10 commits into from
Sep 5, 2023
Merged

[UI] Make sure form validation displays non-valid fields as red in all forms #7064

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cc7a1b1
Add validation to multi-container component
rm3l Sep 1, 2023
3b3ee9c
Add validation to multi-key-value component
rm3l Sep 1, 2023
25e2d3f
Add validation to multi-text component
rm3l Sep 1, 2023
f3f0e1d
Add validation to select-container component
rm3l Sep 1, 2023
1657316
Add validation to volume-mounts component
rm3l Sep 1, 2023
c3e6cc3
Add error helper message for invalid volume size quantities
rm3l Sep 1, 2023
59276cb
Fix Cypress tests
rm3l Sep 1, 2023
b35c5ca
Generate static UI
rm3l Sep 5, 2023
9177d76
fixup! Add error helper message for invalid volume size quantities
rm3l Sep 5, 2023
7055b6e
Generate static UI
rm3l Sep 5, 2023
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
2 changes: 1 addition & 1 deletion pkg/apiserver-impl/ui/index.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...piserver-impl/ui/main.4d8dc3ef32c88ca3.js → ...piserver-impl/ui/main.2218559ad9edb268.js
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions ui/cypress/e2e/spec.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ describe('devfile editor spec', () => {
cy.getByDataCy('container-env-value-2').type("val3");

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1");
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1", {force: true});
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why, but I had to make Cypress forcibly bypass some checks with force: true. Otherwise, Cypress would complain that the element was covered by another one:

CypressError: Timed out retrying after 4000ms: `cy.type()` failed because this element:

`<input _ngcontent-xcd-c181="" formcontrolname="path" matinput="" class="mat-mdc-input-element ng-tns-c84-53 ng-untouched ng-pristine ng-invalid mat-mdc-form-field-input-control mdc-text-field__input cdk-text-field-autofill-monitored" ng-reflect-name="path" data-cy="volume-mount-path-0" id="mat-input-32" required="" aria-required="true">`

is being covered by another element:

`<span aria-hidden="true" class="mat-mdc-form-field-required-marker mdc-floating-label--required ng-tns-c84-53 ng-star-inserted"></span>`

Fix this problem, or use {force: true} to disable error checking.

cy.getByDataCy('volume-mount-name-0').click().get('mat-option').contains('volume1').click();

cy.getByDataCy('endpoints-add').click();
cy.getByDataCy('endpoint-name-0').type("ep1");
cy.getByDataCy('endpoint-targetPort-0').type("4001");

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2");
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2", {force: true});
cy.getByDataCy('volume-mount-name-1').click().get('mat-option').contains('(New Volume)').click();
cy.getByDataCy('volume-name').type('volume2');
cy.getByDataCy('volume-create').click();
Expand Down Expand Up @@ -134,11 +134,11 @@ describe('devfile editor spec', () => {
cy.getByDataCy('container-source-mapping').type('/mnt/sources');

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1");
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1", {force: true});
cy.getByDataCy('volume-mount-name-0').click().get('mat-option').contains('volume1').click();

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2");
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2", {force: true});
cy.getByDataCy('volume-mount-name-1').click().get('mat-option').contains('(New Volume)').click();
cy.getByDataCy('volume-name').type('volume2');
cy.getByDataCy('volume-create').click();
Expand Down Expand Up @@ -397,11 +397,11 @@ describe('devfile editor spec', () => {
cy.getByDataCy('container-image').type('an-image');

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1");
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1", {force: true});
cy.getByDataCy('volume-mount-name-0').click().get('mat-option').contains('volume1').click();

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2");
cy.getByDataCy('volume-mount-path-1').type("/mnt/vol2", {force: true});
cy.getByDataCy('volume-mount-name-1').click().get('mat-option').contains('(New Volume)').click();
cy.getByDataCy('volume-name').type('volume2');
cy.getByDataCy('volume-create').click();
Expand Down
9 changes: 5 additions & 4 deletions ui/src/app/controls/multi-command/multi-command.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
<h3>{{title}}</h3>
<div class="group">
<span *ngFor="let command of commands; let i=index">
<span *ngFor="let control of form.controls; index as i">
<mat-form-field appearance="fill">
<mat-select [value]="command" (selectionChange)="onCommandChange(i, $event.value)">
<mat-label><span>Command</span></mat-label>
<mat-select [formControl]="control">
<mat-option *ngFor="let commandElement of commandList" [value]="commandElement">{{commandElement}}</mat-option>
</mat-select>
</mat-form-field>
</span>
<button *ngIf="commands.length > 0" mat-icon-button (click)="addCommand()">
<button *ngIf="form.controls.length > 0" mat-icon-button (click)="addCommand('')">
<mat-icon class="tab-icon material-icons-outlined">add</mat-icon>
</button>
<button *ngIf="commands.length == 0" mat-flat-button (click)="addCommand()">{{addLabel}}</button>
<button *ngIf="form.controls.length == 0" mat-flat-button (click)="addCommand('')">{{addLabel}}</button>
</div>
52 changes: 39 additions & 13 deletions ui/src/app/controls/multi-command/multi-command.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import { Component, Input } from '@angular/core';
import { NG_VALUE_ACCESSOR } from '@angular/forms';
import {Component, forwardRef, Input} from '@angular/core';
import {
AbstractControl,
ControlValueAccessor,
FormArray,
FormControl,
FormGroup,
NG_VALIDATORS,
NG_VALUE_ACCESSOR, ValidationErrors, Validator,
Validators
} from '@angular/forms';

@Component({
selector: 'app-multi-command',
Expand All @@ -10,21 +19,32 @@ import { NG_VALUE_ACCESSOR } from '@angular/forms';
provide: NG_VALUE_ACCESSOR,
multi: true,
useExisting: MultiCommandComponent
}
},
{
provide: NG_VALIDATORS,
useExisting: forwardRef(() => MultiCommandComponent),
multi: true,
},
]
})
export class MultiCommandComponent {
export class MultiCommandComponent implements ControlValueAccessor, Validator {

@Input() addLabel: string = "";
@Input() commandList: string[] = [];
@Input() title: string = "";

onChange = (_: string[]) => {};

commands: string[] = [];
form = new FormArray<FormControl>([]);

constructor() {
this.form.valueChanges.subscribe(value => {
this.onChange(value);
});
}

writeValue(value: any) {
this.commands = value;
writeValue(value: string[]) {
value.forEach(v => this.addCommand(v));
}

registerOnChange(onChange: any) {
Expand All @@ -33,13 +53,19 @@ export class MultiCommandComponent {

registerOnTouched(_: any) {}

addCommand() {
this.commands.push("");
this.onChange(this.commands);
newCommand(cmdName : string) {
return new FormControl(cmdName, [Validators.required]);
}

addCommand(cmdName: string) {
this.form.push(this.newCommand(cmdName));
}

onCommandChange(i: number, cmd: string) {
this.commands[i] = cmd;
this.onChange(this.commands);
/* Validator implementation */
validate(control: AbstractControl): ValidationErrors | null {
if (!this.form.valid) {
return {'internal': true};
}
return null;
}
}
26 changes: 14 additions & 12 deletions ui/src/app/controls/multi-key-value/multi-key-value.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<div class="group">
<span *ngFor="let entry of entries; let i=index">
<mat-form-field class="mid-width" appearance="outline">
<mat-label><span>Name</span></mat-label>
<input [attr.data-cy]="dataCyPrefix+'-name-'+i" matInput [value]="entry.name" (change)="onKeyChange(i, $event)" (input)="onKeyChange(i, $event)">
</mat-form-field>
<mat-form-field class="mid-width" appearance="outline">
<mat-label><span>Value</span></mat-label>
<input [attr.data-cy]="dataCyPrefix+'-value-'+i" matInput [value]="entry.value" (change)="onValueChange(i, $event)" (input)="onValueChange(i, $event)">
</mat-form-field>
</span>
<button [attr.data-cy]="dataCyPrefix+'-plus'" *ngIf="entries.length > 0" mat-icon-button (click)="addEntry()">
<div *ngFor="let control of form.controls; index as i">
<ng-container [formGroup]="control">
<mat-form-field class="mid-width" appearance="outline">
<mat-label><span>Name</span></mat-label>
<input [attr.data-cy]="dataCyPrefix+'-name-'+i" matInput formControlName="name">
</mat-form-field>
<mat-form-field class="mid-width" appearance="outline">
<mat-label><span>Value</span></mat-label>
<input [attr.data-cy]="dataCyPrefix+'-value-'+i" matInput formControlName="value">
</mat-form-field>
</ng-container>
</div>
<button [attr.data-cy]="dataCyPrefix+'-plus'" *ngIf="form.controls.length > 0" mat-icon-button (click)="addEntry('', '')">
<mat-icon class="tab-icon material-icons-outlined">add</mat-icon>
</button>
<button [attr.data-cy]="dataCyPrefix+'-add'" *ngIf="entries.length == 0" mat-flat-button (click)="addEntry()">{{addLabel}}</button>
<button [attr.data-cy]="dataCyPrefix+'-add'" *ngIf="form.controls.length == 0" mat-flat-button (click)="addEntry('', '')">{{addLabel}}</button>
</div>
52 changes: 30 additions & 22 deletions ui/src/app/controls/multi-key-value/multi-key-value.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import { Component, Input, forwardRef } from '@angular/core';
import { AbstractControl, ControlValueAccessor, NG_VALIDATORS, NG_VALUE_ACCESSOR, ValidationErrors, Validator, Validators } from '@angular/forms';
import {Component, forwardRef, Input} from '@angular/core';
import {
AbstractControl,
ControlValueAccessor,
FormArray,
FormControl,
FormGroup,
NG_VALIDATORS,
NG_VALUE_ACCESSOR,
ValidationErrors,
Validator,
Validators
} from '@angular/forms';

interface KeyValue {
name: string;
Expand Down Expand Up @@ -28,13 +39,19 @@ export class MultiKeyValueComponent implements ControlValueAccessor, Validator {
@Input() dataCyPrefix: string = "";
@Input() addLabel: string = "";

form = new FormArray<FormGroup>([]);

onChange = (_: KeyValue[]) => {};
onValidatorChange = () => {};

entries: KeyValue[] = [];
constructor() {
this.form.valueChanges.subscribe(value => {
this.onChange(value);
});
}

writeValue(value: KeyValue[]) {
this.entries = value;
value.forEach(v => this.addEntry(v.name, v.value));
}

registerOnChange(onChange: any) {
Expand All @@ -43,30 +60,21 @@ export class MultiKeyValueComponent implements ControlValueAccessor, Validator {

registerOnTouched(_: any) {}

addEntry() {
this.entries.push({name: "", value: ""});
this.onChange(this.entries);
}

onKeyChange(i: number, e: Event) {
const target = e.target as HTMLInputElement;
this.entries[i].name = target.value;
this.onChange(this.entries);
newKeyValueForm(kv: KeyValue): FormGroup {
return new FormGroup({
name: new FormControl(kv.name, [Validators.required]),
value: new FormControl(kv.value, [Validators.required]),
});
}

onValueChange(i: number, e: Event) {
const target = e.target as HTMLInputElement;
this.entries[i].value = target.value;
this.onChange(this.entries);
addEntry(name: string, value: string) {
this.form.push(this.newKeyValueForm({name, value}));
}

/* Validator implementation */
validate(control: AbstractControl): ValidationErrors | null {
for (let i=0; i<this.entries.length; i++) {
const entry = this.entries[i];
if (entry.name == "" || entry.value == "") {
return {'internal': true};
}
if (!this.form.valid) {
return {'internal': true};
}
return null;
}
Expand Down
10 changes: 5 additions & 5 deletions ui/src/app/controls/multi-text/multi-text.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<h3 *ngIf="title">{{title}}</h3>
<div class="group">
<span *ngFor="let text of texts; let i=index">
<span *ngFor="let control of form.controls; index as i">
<mat-form-field class="inline" appearance="outline">
<mat-label><span>{{label}}</span></mat-label>
<input matInput [value]="text" (change)="onTextChange(i, $event)">
</mat-form-field>
<input matInput [formControl]="control">
</mat-form-field>
</span>
<button *ngIf="texts.length > 0" mat-icon-button (click)="addText()">
<button *ngIf="form.controls.length > 0" mat-icon-button (click)="addText('')">
<mat-icon class="tab-icon material-icons-outlined">add</mat-icon>
</button>
<button *ngIf="texts.length == 0" mat-flat-button (click)="addText()">{{addLabel}}</button>
<button *ngIf="form.controls.length == 0" mat-flat-button (click)="addText('')">{{addLabel}}</button>
</div>
57 changes: 40 additions & 17 deletions ui/src/app/controls/multi-text/multi-text.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
import { Component, Input } from '@angular/core';
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
import {Component, forwardRef, Input} from '@angular/core';
import {
AbstractControl,
ControlValueAccessor,
FormArray,
FormControl,
NG_VALIDATORS,
NG_VALUE_ACCESSOR,
ValidationErrors,
Validator,
Validators
} from '@angular/forms';

@Component({
selector: 'app-multi-text',
Expand All @@ -10,24 +20,36 @@ import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
provide: NG_VALUE_ACCESSOR,
multi: true,
useExisting: MultiTextComponent
}
},
{
provide: NG_VALIDATORS,
useExisting: forwardRef(() => MultiTextComponent),
multi: true,
},
]
})
export class MultiTextComponent implements ControlValueAccessor {
export class MultiTextComponent implements ControlValueAccessor, Validator {

@Input() label: string = "";
@Input() addLabel: string = "";
@Input() title: string = "";

onChange = (_: string[]) => {};

texts: string[] = [];
form = new FormArray<FormControl>([]);

writeValue(value: any) {
if (value == null) {
value = [];
}
this.texts = value;
constructor() {
this.form.valueChanges.subscribe(value => {
this.onChange(value);
});
}

newText(text: string): FormControl {
return new FormControl(text, [Validators.required]);
}

writeValue(value: string[]) {
value?.forEach(v => this.addText(v));
}

registerOnChange(onChange: any) {
Expand All @@ -36,14 +58,15 @@ export class MultiTextComponent implements ControlValueAccessor {

registerOnTouched(_: any) {}

addText() {
this.texts.push("");
this.onChange(this.texts);
addText(text: string) {
this.form.push(this.newText(text));
}

onTextChange(i: number, e: Event) {
const target = e.target as HTMLInputElement;
this.texts[i] = target.value;
this.onChange(this.texts);
/* Validator implementation */
validate(control: AbstractControl): ValidationErrors | null {
if (!this.form.valid) {
return {'internal': true};
}
return null;
}
}
2 changes: 1 addition & 1 deletion ui/src/app/controls/select-container/select-container.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<mat-form-field appearance="fill">
<mat-label>{{label}}</mat-label>
<mat-select data-cy="select-container" [value]="container" (selectionChange)="onSelectChange($event.value)">
<mat-select [formControl]="formCtrl" data-cy="select-container" (selectionChange)="onSelectChange($event.value)">
<mat-option *ngFor="let container of containers" [value]="container">{{container}}</mat-option>
<mat-option value="!">(New {{label}})</mat-option>
</mat-select>
Expand Down
Loading