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
DSP-652 FIX: Boolean add button #182
Conversation
…currently has a value. If not, show the add button.
<div *ngIf="prop.guiDef.propertyIndex !== 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext' && | ||
addButtonIsVisible && | ||
prop.guiDef.cardinality !== 1 && | ||
(prop.guiDef.cardinality !== 1 || prop.guiDef.cardinality === 1 && prop.values.length === 0 ) && |
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.
@tobiasschweizer this ngIf is quite unmanageable but I don't currently have a way to do it in the TS file.
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.
could you use the cardinality util provided by dsp-js?
if not, could we somehow add what you need to it?
…ty of 0-1 and does not contain a value
…late to the class
*/ | ||
addValueIsAllowed(prop: PropertyInfoValues): boolean { | ||
// check if cardinality allows for a value to be added | ||
if (prop.guiDef.cardinality !== 1 || |
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.
@tobiasschweizer How can I get an instance of a ResourceClassDefinition
to use for CardinalityUtil.createdValueForPropertyAllowed
? I think that's what I'd want to use instead of this logic.
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.
the definition should be contained in the ReadResource
's entityInfo.classes
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.
entityInfo.classes
has a type of ResourceClassDefinitionWithPropertyDefinition
which is an extension of ResourceClassDefinition
but I'm getting an error when I try to pass it in.
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.
there are some methods to get only certain defs
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.
but I think these work only for property defs
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.
What kind of error do you get? Can you use type assertions to make it work?
Otherwise we can look at it on Monday.
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.
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 will try this myself and get back to you later this morning.
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.
@mdelez Could you try this CardinalityUtil.createValueForPropertyAllowed(prop.propDef.id, prop.values.length, this.parentResource.entityInfo.classes[this.parentResource.type]);
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.
That was the missing part :) Thanks!!
addButtonIsVisible && | ||
prop.guiDef.cardinality !== 1 && | ||
propID !== prop.propDef.id"> | ||
<div *ngIf="addValueIsAllowed(prop) && prop.guiDef.propertyIndex !== 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext'"> |
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.
Why does the condition contain a property IRI from the anything test project?
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.
ah I get it: but then this should be more generic than 'http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext'
, you could use the gui ele
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 think we should integrate the XML / CKeditor functionality asap, so this becomes unnecessary
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 agree, I'll make the small change to use the gui element
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.
fixed in 94ab6a8
…hould hide the add button for a Rich Text Value
// cardinality allows for a value to be added | ||
// value component does not already have an add value form open | ||
// user has write permissions | ||
if (isAllowed && this.propID !== prop.propDef.id && this.addButtonIsVisible) { |
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.
What does this do? this.propID !== prop.propDef.id
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.
"value component does not already have an add value form open"?
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.
this.propID
is a string that is assigned a prop.propDef.id
of the corresponding property when the add button is clicked. This logic is used to hide the add button when the add value form for that property is already open so that the user can't keep opening new add value forms. Without this logic, the user would always see the add button under the property if the cardinality allows for it.
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, I get it now :-)
// cardinality allows for a value to be added | ||
// value component does not already have an add value form open | ||
// user has write permissions | ||
if (isAllowed && this.propID !== prop.propDef.id && this.addButtonIsVisible) { |
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 can just return the whole expression, no need for an if
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.
noob code fixed in 40cb19e
* @param prop the resource property | ||
*/ | ||
addValueIsAllowed(prop: PropertyInfoValues): boolean { | ||
// temporary until CkEditor is integrated |
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.
Did we already create a DSP task for that?
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 don't believe so
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 found this in my drafts. I guess I forgot to click 'create'
https://dasch.myjetbrains.com/youtrack/issue/DSP-682
closes https://dasch.myjetbrains.com/youtrack/issue/DSP-652
uses js-lib rc.11 and dsp-api rc.15