-
Notifications
You must be signed in to change notification settings - Fork 39
feature/finished/IIA-2521: Set Default Values for Property Fields in Edit Panel for Concept FQN #621
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
feature/finished/IIA-2521: Set Default Values for Property Fields in Edit Panel for Concept FQN #621
Conversation
…Edit Panel for Concept FQN
.../src/main/java/dev/ikm/komet/kview/mvvm/view/properties/AddFullyQualifiedNameController.java
Outdated
Show resolved
Hide resolved
|
@carldea did what you suggested and fixed another bug. So now besides fixing the ticket this PR is fixing:
Thanks! |
|
|
||
| // Set UI to default values | ||
| caseSignificanceComboBox.setValue(TinkarTerm.DESCRIPTION_NOT_CASE_SENSITIVE); | ||
| statusComboBox.setValue(Entity.getFast(State.ACTIVE.nid())); |
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.
Can you use the State.ACTIVE instead of calling getFast()?
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.
State is an enum. We can only get the nid and so we’ll need to do getFast
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 statusComboBox items should contain all states as a State objects(enum). setValue(ACTIVE)
We should look at how we did this for the new StampAddController.java as it relates to the following:
- converter
- buttoncell
- cellfactory
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.
@carldea
I was able to do the refactoring of the StampAddController and created the utility class like we discussed in the dev sync (also took the oportunity to correct some minor issues with the old code).
Regarding the FQN Controller and changing the combobox to use State instances, what I was afraid of ended up happening, unfortunately... when we change the ComboBox to have a collection of State objects rather than what we had, this change ends up trickling down to a bunch of other classes.
Let me explain, the AddFullyQualifiedNameController uses DescrNameViewModel which expects a ConceptEntity for the state. The DescrNameViewModel is then used in other name controller, edit FQN controller, edit other name controller, pattern controller, ConceptViewModel.
In conclusion if we change it to State, unfortunately we'll need to change in a lot of other places because it's all connected... With any change it will also bring with it the possibility of breaking Pattern and Concept editing/creation...
We might possibly want to create another ticket for this effort (using State enum for all these classes)?
carldea
left a comment
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.
Looks good!
Fixes: https://ikmdev.atlassian.net/browse/IIA-2521
This ticket also ended up fixing: