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

KOGITO-2674: [DMN Designer] Multiple DRDs support - Context menu component #3357

Merged
merged 23 commits into from
Jul 22, 2020

Conversation

vpellegrino
Copy link
Contributor

Please refer to: https://issues.redhat.com/browse/KOGITO-2674

Task description: Need a context menu for DRD actions

Proposed solution: Providing a generic context menu widget

There you can find an utilization example of this widget.
The result is the follow:
context_menu_demo

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @vpellegrino, great stuff!

I personally, would add an extra show method like this (to avoid the API users from remembering that they need to reset the menu before showing it):

dRDContextMenu.show(self -> {
  self.setHeaderMenu("DRD ACTIONS", "fa fa-share-alt");
  self.addTextMenuItem("Create", true, () -> DomGlobal.console.log("A"));
  self.addTextMenuItem("Add to", true, () -> DomGlobal.console.log("B"));
  self.addTextMenuItem("Remove", true, () -> DomGlobal.console.log("C"));
});

Let me know wdyt :-)

@karreiro karreiro self-requested a review July 14, 2020 17:41
Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Ohh.. I've noticed another minor thing ;-)

@karreiro karreiro self-requested a review July 14, 2020 17:45
@vpellegrino
Copy link
Contributor Author

@karreiro I updated PR including your suggestions. Thanks for the feedback!

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thanks, @vpellegrino ;-)

@vpellegrino
Copy link
Contributor Author

Jenkins please retest this

@vpellegrino
Copy link
Contributor Author

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

83.1% 83.1% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_202) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

For the code smell, the Span element was already present in the constructor. I have only added a further parameter: the Icon. TBH, I would not change it, in order to be sure to not introduce regressions.

@karreiro
Copy link
Contributor

Jenkins please retest this.

@vpellegrino
Copy link
Contributor Author

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

91.5% 91.5% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_202) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

The reason quality gate is failing:
0.0% Condition Coverage on New Code (is less than 80%)

Please let me know if there is any action pending from my side: coverage is now 91.5%

Copy link

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

@vpellegrino I haven't checked the code yet and kogito-channels.

I checked on business central using a DecisionNavigatorView you shared. It works pretty well on chrome. However I got and NullPointer and freezing screen on firefox [1].

I am concerned its caused because I touched code manually to have entry point for showing your new menu. I had idea if we can not use experimental API (documented here https://github.com/kiegroup/kie-docs/pull/1131/files) that displays feature implemented as experimental just if business central is started with -Dappformer.experimental.features=true. It would eliminate any mistake done by me when touching code locally. Not saying it is doable but at least worth to assume as I expect we will iterate over this feature longer time.

[1]
Screenshot from 2020-07-20 15-31-43

@vpellegrino
Copy link
Contributor Author

vpellegrino commented Jul 20, 2020

@vpellegrino I haven't checked the code yet and kogito-channels.

I checked on business central using a DecisionNavigatorView you shared. It works pretty well on chrome. However I got and NullPointer and freezing screen on firefox [1].

I am concerned its caused because I touched code manually to have entry point for showing your new menu. I had idea if we can not use experimental API (documented here https://github.com/kiegroup/kie-docs/pull/1131/files) that displays feature implemented as experimental just if business central is started with -Dappformer.experimental.features=true. It would eliminate any mistake done by me when touching code locally. Not saying it is doable but at least worth to assume as I expect we will iterate over this feature longer time.

[1]
Screenshot from 2020-07-20 15-31-43

@jomarko thanks for your quick feedback.
It turns out that under firefox, event.path is not defined (https://www.thetopsites.net/article/53508096.shtml), so I have used composedPath() method instead. I tested it locally and works fine!

@vpellegrino
Copy link
Contributor Author

@vpellegrino vpellegrino requested a review from jomarko July 21, 2020 12:54
Copy link

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

I manually retried firefox and VS code, seems working nice. Thank you @vpellegrino

Copy link
Contributor

@danielzhe danielzhe left a comment

Choose a reason for hiding this comment

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

Good job!

@sonarcloud
Copy link

sonarcloud bot commented Jul 21, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

91.9% 91.9% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_202) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@vpellegrino
Copy link
Contributor Author

@kiegroup/gatekeepers can you please merge?

@karreiro karreiro merged commit b928a5c into kiegroup:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants