-
Notifications
You must be signed in to change notification settings - Fork 6
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(ontology): unsupported property type was displayed wrong (DEV-936) #740
Conversation
// if the property is of type number, but sub property of SeqNum, | ||
// select the correct default prop params | ||
propType = (group.elements.find(i => | ||
i.objectType === property.objectType | ||
i.objectType === property.objectType && i.subPropOf === Constants.SeqNum | ||
)); | ||
} else if (property.objectType === Constants.IntValue && subProp === Constants.SeqNum) { | ||
} else if (property.objectType === Constants.TextValue) { | ||
// if the property is of type text value, we have to check the gui element | ||
// to get the correct default prop params | ||
propType = (group.elements.find(i => | ||
i.objectType === property.objectType && i.subPropOf === Constants.SeqNum | ||
i.guiEle === property.guiElement && i.objectType === property.objectType | ||
)); | ||
} else { | ||
// in all other cases the gui-element resp. the subProp is not relevant | ||
// because the object type is unique |
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.
Small observation: In line 139/140 you put the comment before the if structure and here after the if and else
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 point. It's always the question, where to put the comment. Normally we put it outside the if statement. But in this long list of if
, else if
, else
I thought it makes sense to have it inside.
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.
LGTM
resolves DEV-936