-
Notifications
You must be signed in to change notification settings - Fork 1
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
Read only fields #5
Conversation
Pull Request Test Coverage Report for Build 77
💛 - Coveralls |
<div *ngIf="!editing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> | ||
<div *ngIf="!isEditing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> <small *ngIf="isOptional" style="color: gray">(optional)</small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the 'optional' part be only in the editable div? It will never be used here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right
|
||
get isOptional(): boolean { | ||
return this.editing && !this.field.readOnly && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would avoid testing if it is 'editing' if you remove the 'optional' part from the div in the non-editing state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right
<strong *ngIf="!editing">{{ field.label }}</strong> | ||
<p class="text-muted" *ngIf="!editing">{{ form?.controls[elementId]?.value | valueSuffix: field.unit }}</p> | ||
<div *ngIf="!isEditing" id="{{elementId}}-label"> | ||
<strong>{{ field.label }}</strong> <small *ngIf="isOptional" style="color: gray">(optional)</small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
<div *ngIf="!editing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> | ||
<div *ngIf="!isEditing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> <small *ngIf="isOptional" style="color: gray">(optional)</small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
<div [hidden]="editing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> | ||
<div [hidden]="isEditing" id="{{field.key}}-label"> | ||
<strong>{{ field.label }}</strong> <small *ngIf="isOptional" style="color: gray">(optional)</small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
demo/src/app/home/home.component.ts
Outdated
@@ -44,6 +43,14 @@ export class HomeComponent implements OnInit { | |||
Validators.email | |||
] | |||
}), | |||
new SelectField({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A read-only SelectField doesn't make much sense. But ok, we can review that later
@@ -53,6 +53,10 @@ export class SelectFieldComponent extends BaseDynamicFieldComponent<SelectField> | |||
this.select.registerOnTouched(this._onTouched); | |||
this.viewModel.subscribe((value: any) => { | |||
if (this.field.searchByValueAttributeHandler) { | |||
if (value === undefined || value === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could value be undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If initialize the FormControl with undefined value or call form.setValue with undefined for the key of this control
<div *ngIf="editing" id="{{elementId}}-div" class="form-group" [formGroup]="form" [ngClass]="displayFieldCss()"> | ||
<label for="{{elementId}}">{{field.label}}</label> | ||
<div *ngIf="isEditing" id="{{elementId}}-div" class="form-group" [formGroup]="form" [ngClass]="displayFieldCss()"> | ||
<label for="{{elementId}}">{{field.label}}</label> <small *ngIf="isOptional" style="color: gray">(optional)</small> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRY!
Optional should be a component named <optional-tag>
or something alike.
Add read-only behavior, and display '(optional)' label for not required fields