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(ontology): unsupported property type was displayed wrong (DEV-936) #740

Merged
merged 2 commits into from May 16, 2022

Conversation

kilchenmann
Copy link
Contributor

resolves DEV-936

@kilchenmann kilchenmann self-assigned this May 16, 2022
@kilchenmann kilchenmann added the bug A bug fix label May 16, 2022
Comment on lines +147 to +160
// 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
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Collaborator

@Vijeinath Vijeinath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kilchenmann kilchenmann merged commit 87124e9 into main May 16, 2022
@kilchenmann kilchenmann deleted the wip/dev-936-fix-unsupported-prop branch May 16, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants