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: display-edit dev build errors #106
Conversation
…ev build errors" This reverts commit 2ea88b6. # Conflicts: # projects/dsp-ui/src/lib/viewer/operations/display-edit/display-edit.component.ts
<dsp-uri-value class="parent-value-component" #displayVal *ngSwitchCase="constants.UriValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-uri-value> | ||
<dsp-decimal-value class="parent-value-component" #displayVal *ngSwitchCase="constants.DecimalValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-decimal-value> | ||
<dsp-color-value class="parent-value-component" #displayVal *ngSwitchCase="constants.ColorValue" [mode]="mode" [displayValue]="$any(displayValue)"></dsp-color-value> | ||
<dsp-interval-value class="parent-value-component" #displayVal *ngSwitchCase="constants.IntervalValue" [mode]="mode" [displayValue]="$any(displayVal)"></dsp-interval-value> |
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.
$any(displayVal) ;-)
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.
Thanks! Fixed in 2f6f721
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.
#displayVal
is actually a variable, this is why this compiled. But since it is wrapped with $any()
, no compatibility check was performed.
https://angular.io/guide/template-syntax#template-reference-variables-var
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.
I wondered why the unit tests passed and checked the test in display-edit.component.spec
:
it('should choose the apt component for an interval value in the template', () => {
testHostComponent.assignValue('http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval');
testHostFixture.detectChanges();
expect(testHostComponent.displayEditValueComponent.displayValueComponent instanceof TestIntervalValueComponent).toBe(true);
expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined();
expect(testHostComponent.displayEditValueComponent.displayValueComponent.mode).toEqual('read');
});
expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined();
should actually check for an instance of a ReadIntervalValue
. That would make sure that the template correctly passed the ReadIntervalValue
to the ReadIntervalValueComponent
. The same applies to all other value types.
Shall we make a DSP task for this or do it in this PR?
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.
in case of $any(displayVal)
a reference to TestIntervalValueComponent
was passed instead of the ReadIn tervalValue
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.
I wondered why the unit tests passed and checked the test in
display-edit.component.spec
:it('should choose the apt component for an interval value in the template', () => { testHostComponent.assignValue('http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval'); testHostFixture.detectChanges(); expect(testHostComponent.displayEditValueComponent.displayValueComponent instanceof TestIntervalValueComponent).toBe(true); expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined(); expect(testHostComponent.displayEditValueComponent.displayValueComponent.mode).toEqual('read'); });
expect(testHostComponent.displayEditValueComponent.displayValueComponent.displayValue).not.toBeUndefined();
should actually check for an instance of aReadIntervalValue
. That would make sure that the template correctly passed theReadIntervalValue
to theReadIntervalValueComponent
. The same applies to all other value types.Shall we make a DSP task for this or do it in this PR?
Good find! Since not much has changed in this PR in the end, I'm okay making the changes in this PR
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.
… appropriate readvalue. Also added some missing values
I have problems tracking this branch. SOmehow it doesn't like "FIX/" (capitalized). |
<dsp-time-value class="parent-value-component" #displayVal *ngSwitchCase="constants.TimeValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-time-value> | ||
<dsp-geoname-value class="parent-value-component" #displayVal *ngSwitchCase="constants.GeonameValue" [mode]="mode" [displayValue]=$any(displayValue)></dsp-geoname-value> | ||
<dsp-link-value class="parent-value-component" #displayVal *ngSwitchCase="constants.LinkValue" [mode]="mode" [displayValue]=$any(displayValue) | ||
<!-- display value is cast as 'any' because the compiler cannot infer the value type of the child component --> |
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.
"because the compiler cannot infer the value type of the child component" -> because the compiler cannot infer the value type expected by the child component?
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.
yeah, that's clearer. Fixed in 2a89cfb
weird, the other fix branches were named the same way |
somehow my git could not track them correctly |
I'll switch to lowercase from now on :) |
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.
Good to go! Thanks.
closes https://dasch.myjetbrains.com/youtrack/issue/DSP-389