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

kie-issues#1498: Add "Generate form code for human interactions" command in the BPMN VS Code extension #2660

Merged
merged 23 commits into from
Oct 29, 2024

Conversation

ljmotta
Copy link
Contributor

@ljmotta ljmotta commented Oct 11, 2024

Closes: apache/incubator-kie-issues#1498

Description

  • Add BPMN Editor: Generate form code for human interactions from jBPM project command.
  • Add BPMN Editor prefix to the other BPMN Editor VS Code commads.
  • Rename inputSanizationUtil to removeInvalidVarChars.
  • Rename sanatizedId to idWithoutInvalidVarChars.
  • Make theme idWithoutInvalidVarChars optional.
  • Improve form-code-generator FormAsset interface.

@ljmotta ljmotta self-assigned this Oct 11, 2024
@ljmotta ljmotta added the pr: wip PR is still under development label Oct 11, 2024
@ljmotta ljmotta added pr: waiting-for-ci CI is still running or broken. and removed pr: wip PR is still under development labels Oct 14, 2024
@ljmotta ljmotta marked this pull request as ready for review October 14, 2024 11:08
Copy link
Contributor

@tiagobento tiagobento left a comment

Choose a reason for hiding this comment

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

Few comments inline... and also I think we need to add this command to our development VS Code Extension...

@@ -65,11 +69,15 @@
"dark": "./static/svg-icon-dark.png",
"light": "./static/svg-icon-light.png"
},
"title": "Get BPMN Editor Preview SVG"
"title": "BPMN Editor: Get BPMN Editor Preview SVG"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"title": "BPMN Editor: Get BPMN Editor Preview SVG"
"title": "BPMN Editor: Get preview SVG"

?

Comment on lines 53 to 55
context.subscriptions.push(
vscode.commands.registerCommand("extension.kie.generateFormCode", async (args: any) => generateFormsCommand())
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
context.subscriptions.push(
vscode.commands.registerCommand("extension.kie.generateFormCode", async (args: any) => generateFormsCommand())
);
context.subscriptions.push(
vscode.commands.registerCommand("extension.apache.kie.bpmnEditor.generateFormCode", async (args: any) => generateFormsCommand())
);

?

Copy link
Contributor

@tiagobento tiagobento left a comment

Choose a reason for hiding this comment

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

"Invalid var chars" seems like a very clear name, but I'm not sure in what context "var" falls into. Is it a BPMN variable? Or a TypeScript variable? Maybe we could be a little bit extra there, if that's the goal?

@ljmotta
Copy link
Contributor Author

ljmotta commented Oct 14, 2024

@tiagobento It's related to TypeScript variables... I'll update accordingly.

@ljmotta ljmotta force-pushed the kie-issues-1498-b branch 3 times, most recently from 586fc96 to f51b8ea Compare October 17, 2024 01:19
@tiagobento tiagobento removed the pr: waiting-for-ci CI is still running or broken. label Oct 22, 2024
Copy link
Contributor

@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 think the only feedback I have is about the step of selecting jBPM project path

Comment on lines +100 to +112
"title": "Kogito Editors: Save Preview SVG"
},
{
"command": "extension.kogito.runTest",
"title": "Run"
"title": "Kogito Editors: Run"
},
{
"command": "extension.kogito.silentlyGenerateSvg",
"title": "Generate SVG without any notification"
"title": "Kogito Editors: Generate SVG without any notification"
},
{
"command": "extension.apache.kie.kogitoEditors.generateFormCode",
"title": "Kogito Editors: Generate form code for human interactions from jBPM project"
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about word Kogito as we are in kie-editors-dev-vscode-extension package, not a kogito-editors-dev-vscode-extension

@@ -144,7 +149,7 @@
},
"customEditors": [
{
"displayName": "KIE Kogito Editors",
"displayName": "Apache KIE Kogito Editors",
Copy link
Contributor

Choose a reason for hiding this comment

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

similar comment

Comment on lines 38 to 45
// Select project path
const projectUri = await vscode.window.showOpenDialog({
canSelectFiles: false,
canSelectFolders: true,
canSelectMany: false,
openLabel: "Select Project Folder",
defaultUri: defaultPath ? vscode.Uri.file(defaultPath) : undefined,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with added dialogues/prompts/popups, not sure what is the terminology in VSCode is only with the first one. There is no additionl linformation in the step of selecting path to the jBPM project. See the second screenshot below.:

01

Screenshot 2024-10-22 150300

02

Screenshot 2024-10-22 150309

it seems openLabel: "Select Project Folder", config is not used? In the documentation I see alternative title ? https://code.visualstudio.com/api/references/vscode-api#OpenDialogOptions

Using the generate command for the first time I was confused what should I do in the step 02.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Changing to title fixed the problem, adding the label in this step:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

perfect

@ljmotta
Copy link
Contributor Author

ljmotta commented Oct 28, 2024

@jomarko Thanks for your review. The name "Kogito Editors" is due to the current extension name. It's not the intent of this PR to change the extension name, I've only added the "Apache" as it's a must. @tiagobento do we create a new issue to tackle the Kogito name, or I just remove and call it "Apache KIE Editors"?

Copy link
Contributor

@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.

Thank you for changes, I understand the substring "Kogito Editors" must be discussed further with others, so I am approving this PR and I will accept any decision that will be taken by the team around this.

@tiagobento tiagobento merged commit 9a8d8c9 into apache:main Oct 29, 2024
14 checks passed
ricardozanini pushed a commit to ricardozanini/kogito-tooling that referenced this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Generate form code for human interactions" command in the BPMN VS Code extension
3 participants