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

Fix bug where required caused save to be disabled even though populated #2091

Merged
merged 4 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 docs/forms.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

![](images/form.png)

You first need to use the `bl-complex-form` componenent. You will need to pass the formGroup as an input and the submit method(Don't forget to autobind the submit)
You first need to use the `bl-complex-form` component. You will need to pass the formGroup as an input and the submit method(Don't forget to autobind the submit)

```html
<bl-complex-form [formGroup]="form" [submit]="submit" [containerRef]="sidebarRef">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe("DurationPickerComponent", () => {
expect(component.time).toBe("invalid-num");
const errors = de.queryAll(By.css(".error"));
expect(errors.length).toBe(1);
expect(errors[0].nativeElement.textContent).toContain("Input should be a valid number");
expect(errors[0].nativeElement.textContent).toContain("Input should be a valid positive number");
expect(testComponent.control.valid).toBe(false, "Testcomponent control should be invalid");
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class DurationPickerComponent implements FormFieldControl<any>,
@Input() public allowUnlimited: boolean = true;
@Input() public defaultDuration: string = "";

@Input() @FlagInput() public required = false;
@Input() @FlagInput() public required: boolean = false;

@Input()
public get disabled(): boolean {
Expand Down Expand Up @@ -138,6 +138,9 @@ export class DurationPickerComponent implements FormFieldControl<any>,
this.changeDetector.markForCheck();
});
}
if (this._propagateChange) {
this._propagateChange(this.value);
}
}

public ngOnChanges(changes) {
Expand All @@ -164,7 +167,12 @@ export class DurationPickerComponent implements FormFieldControl<any>,

public writeValue(value: Duration | string): void {
if (value === null || value === undefined) {
this.value = null;
if (this.defaultDuration) {
this.time = this.defaultDuration;
this.value = this._getDuration();
} else {
this.value = null;
}
} else if (value instanceof Duration) {
this.value = value;
} else {
Expand All @@ -183,12 +191,12 @@ export class DurationPickerComponent implements FormFieldControl<any>,
}

public validate() {
if (this.invalidTimeNumber || this.invalidCustomDuration) {
if (this.invalidTimeNumber || this.invalidCustomDuration || (this.required && !this.time)) {
return {
duration: "Invalid",
};
}
return false;
bgklein marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

public setDescribedByIds(ids: string[]): void {
Expand Down Expand Up @@ -232,7 +240,7 @@ export class DurationPickerComponent implements FormFieldControl<any>,
return this._getCustomDuration(this.time);
default:
const time = Number(this.time);
if (isNaN(time)) {
if (isNaN(time) || time < 0) {
this.invalidTimeNumber = true;
return null;
} else {
Expand All @@ -250,6 +258,7 @@ export class DurationPickerComponent implements FormFieldControl<any>,
*/
private _setTimeAndUnitFromDuration(duration: Duration) {
if (!duration) {
this.time = "";
return;
}
const days = duration.as("day");
Expand Down
5 changes: 4 additions & 1 deletion src/@batch-flask/ui/duration-picker/duration-picker.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
</div>
</div>
<div class="error" *ngIf="invalidTimeNumber">
Input should be a valid number
Input should be a valid positive number
</div>
<div class="error" *ngIf="required && !this.time">
Input is required
</div>
<div class="error" *ngIf="invalidCustomDuration">
This is not a valid ISO 8601 duration <i>(e.g. PT1M, P1D)</i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<bl-error controlName="maxTaskRetryCount" code="validateRange">Retry count values can range from -1 to 100</bl-error>
</bl-form-field>
<bl-form-field>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [required]="true" [allowUnlimited]="false" [defaultDuration]="7"></bl-duration-picker>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [allowUnlimited]="false" [defaultDuration]="7" required></bl-duration-picker>
</bl-form-field>
</div>
<div class="form-element pad-top">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<bl-error controlName="maxTaskRetryCount" code="validateRange">Retry count values can range from -1 to 100</bl-error>
</bl-form-field>
<bl-form-field>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [required]="true" [allowUnlimited]="false" [defaultDuration]="7"></bl-duration-picker>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [allowUnlimited]="false" [defaultDuration]="7" required></bl-duration-picker>
</bl-form-field>
</div>
<div class="form-element pad-top">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<bl-error controlName="maxTaskRetryCount" code="validateRange">Retry count values can range from -1 to 100</bl-error>
</bl-form-field>
<bl-form-field>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [required]="true" [allowUnlimited]="false" [defaultDuration]="7"></bl-duration-picker>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [allowUnlimited]="false" [defaultDuration]="7" required></bl-duration-picker>
</bl-form-field>
</div>
<div class="form-element pad-top">
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/task/action/add/add-task-form.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
</div>
<div class="form-element">
<bl-form-field>
<bl-duration-picker formControlName="retentionTime" label="Retention time" [required]="true" [allowUnlimited]="false" [defaultDuration]="7"></bl-duration-picker>
bgklein marked this conversation as resolved.
Show resolved Hide resolved
<bl-duration-picker formControlName="retentionTime" label="Retention time" [allowUnlimited]="false" [defaultDuration]="7" required></bl-duration-picker>
</bl-form-field>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/app/utils/component-utils/component-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class ComponentUtils {
* Now you navigate back to /pool/pool-2. As pool-4 wasn't present originally it will be removed from the list
*
* @param route Angular Activated Route
* @param view ListView used in the componenent
* @param view ListView used in the component
*/
public static setActiveItem<TEntity extends Record<any>>(route: ActivatedRoute, view: ListView<TEntity, any>) {
route.url.subscribe((url) => {
Expand Down