DROOLS-5067: Add Bound Fact Property checkmark #1314
Conversation
|
jenkins retest this please |
|
Thanks @jomarko , please do not forget to fix it also for /expression see: https://issues.redhat.com/secure/attachment/12465472/ExpressionTick.webm |
|
@jomarko @dupliaka I'm not totally convinced regarding this fix. I mean, when I implemented that tick, its scope was: the user manually selected that instance / property from test tools dock, which is different than highlight the instance / property in scope. With this change, the tick and the blue highlight will have the same meaning, and probably this is not the scope of that tick. |
|
@yesamer Ok, I see your point, I was not part of UX planning in the original jira, however still think this change makes more consistent behavior. For sure just my opinion, I propose to ask on weekly meeting where is broader audience. What do you think? |
|
@dupliaka ok, will incorporate also for expressions according to results of the discussion about this approach. |
|
@dupliaka your requirement from #1314 (comment) should be incorporated now. |
|
jenkins execute full downstream build |
| if (!listGroupItemView.isShown()) { | ||
| onToggleRowExpansion(listGroupItemView, false); | ||
| final ListGroupItemView instanceListGroupItemView = listGroupItemViewMap.get(factName); | ||
| if (!instanceListGroupItemView.isShown()) { |
There was a problem hiding this comment.
@jomarko There is a particular reason about this rename?
There was a problem hiding this comment.
From my point of view listGroupItemView just says what the variable is instance of, while instanceListGroupItemView tries to provide more information, it is an ListGroupItemView representing an Instance , however if you think my rename is inapproprtiate I can revert it. Or if you have different proposal for name.
There was a problem hiding this comment.
@jomarko Thank you, fine for me.
|
@jomarko We have 2 failing tests, can you please check them? |
| @Mock | ||
| private FieldItemPresenter fieldItemPresenterMock; | ||
| @Spy | ||
| private FieldItemPresenter fieldItemPresenterSpy; |
There was a problem hiding this comment.
@jomarko @Spy must be initialized, this is the reason of failing tests. (NPE is thown)
|
jenkins execute full downstream build |
1 similar comment
|
jenkins execute full downstream build |
|
jenkins execute full downstream build |
|
@dupliaka just friendly reminder, if you have a moment to review. |
|
jenkins execute full downstream build |
|
jenkins execute full downstream build |
|
@dupliaka done, please have a look |
|
@yesamer the code has changed slightly since your last review, not sure if you want to re-review or we can merge. |
|
jenkins please retest this |
|
@jomarko You can proceed. |
|
@yesamer thank you, once green will ask for merge. |
|
SonarCloud Quality Gate failed.
|
|
@kiegroup/gatekeepers could we please merge? |
|
@jomarko SonarCloud is reporting zero coverage.. but I see you have written tests; so merging. |
|
@manstis Unfortunetely test coverage is currently broken :/ |
For more details see: https://issues.redhat.com/browse/DROOLS-5067