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.

Sign up for GitHub

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

Jump to bottom

KOGITO-3517: Stunner: InlineEditor Keyboard events are triggering shortcuts in VSCode #3454

Merged
merged 2 commits into from Oct 26, 2020
Merged

KOGITO-3517: Stunner: InlineEditor Keyboard events are triggering shortcuts in VSCode #3454

merged 2 commits into from Oct 26, 2020

Conversation

handreyrc
Copy link
Contributor

@handreyrc handreyrc commented Oct 15, 2020 •
edited

Hey @romartin: Please, could you review this PR?

This PR fixes the following related issues:
JIRA: KOGITO-3517
JIRA: RHPAM-3197

Referenced Pull Requests:

VS Code: plugin

Thanks!

@handreyrc handreyrc changed the title Kogito 3517 KOGITO-3517: Stunner: InlineEditor Keyboard events are triggering shortcuts in VSCode Oct 15, 2020
@handreyrc handreyrc mentioned this pull request Oct 15, 2020
Merged
@handreyrc
Copy link
Contributor Author

Jenkins run fdb

@handreyrc
Copy link
Contributor Author

Jenkins run fdb

@sonarcloud
Copy link

sonarcloud bot commented Oct 19, 2020

Kudos, SonarCloud Quality Gate passed!

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

85.7% 85.7% Coverage
0.0% 0.0% Duplication

@romartin romartin self-requested a review October 21, 2020 13:23
romartin
romartin approved these changes Oct 21, 2020
View reviewed changes
Copy link
Contributor

@romartin romartin left a comment

Choose a reason for hiding this comment

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

Looks good to me @handreyrc ! Thanks!

domhanak
domhanak approved these changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@domhanak domhanak left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Tested with VSCode 1.50.1.
I was able to open shortcuts panel using shift+/ or shift+? and the panel was not opening while I was renaming node name.
Verified in BPMN, DMN, I was not able to test in SCESIM.

domhanak
domhanak requested changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@domhanak domhanak left a comment

Choose a reason for hiding this comment

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

Checking the business central...

@romartin
Copy link
Contributor

good point @domhanak , plz check BC and let us know if it works properly as with kogito... 👍
Thanks!!

domhanak
domhanak reviewed Oct 26, 2020
View reviewed changes
Copy link
Contributor

@domhanak domhanak left a comment

Choose a reason for hiding this comment

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

Looks good from business central point of view.

I was unable to reproduce RHPAM-3197 with these changes.

@handreyrc I noticed a small delay when opening the text editor of node.
What I did:

  1. Open node name editor and click on canvas
  2. Repeat for other nodes in process.
    After 2-3 nodes the text editor for name takes some time to fully show. There is 2-3 seconds.
    Could you please let me know if this is related?

domhanak
domhanak approved these changes Oct 26, 2020
View reviewed changes
Copy link
Contributor

@domhanak domhanak left a comment

Choose a reason for hiding this comment

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

@handreyrc I checked master and the delay when triggering the node name editor is present there also.
Approved.

@romartin romartin merged commit 09f548b into kiegroup:master Oct 26, 2020
@romartin
Copy link
Contributor

@domhanak I noticed some delay when starting the inline edition for a node, but looks it's just due to the backgrand animation itself, and do not prevent from early typing. Is that correct? maybe you don't like the animation guys?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romartin romartin romartin approved these changes

@tiagobento tiagobento tiagobento approved these changes

@domhanak domhanak domhanak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@handreyrc @romartin @tiagobento @domhanak