ljmotta commented on code in PR #2158:
URL:
https://github.com/apache/incubator-kie-tools/pull/2158#discussion_r1489443674
##########
packages/dmn-editor/stories/dev/DevWebApp.mdx:
##########
@@ -0,0 +1,26 @@
+{/* prettier-ignore */}
+{/*
+
+- Licensed to the Apache Software Foundation (ASF) under one
+- or more contributor license agreements. See the NOTICE file
+- distributed with this work for additional information
+- regarding copyright ownership. The ASF licenses this file
+- to you under the Apache License, Version 2.0 (the
+- "License"); you may not use this file except in compliance
+- with the License. You may obtain a copy of the License at
+-
+- http://www.apache.org/licenses/LICENSE-2.0
+-
+- Unless required by applicable law or agreed to in writing,
+- software distributed under the License is distributed on an
+- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+- KIND, either express or implied. See the License for the
+- specific language governing permissions and limitations
+- under the License.
+ \*/}
+
+import { Meta, Story, Canvas } from "@storybook/blocks";
+
+# DevWebApp
+
+Some content to put here.
Review Comment:
I don't think we need a documentation file for the Dev Web App, as it's
purpose is to use during development.
##########
.gitignore:
##########
@@ -10,6 +10,10 @@
**/dist-e2e-tests/
**/dist-storybook/
**/e2e-tests-tmp/
+**/test-results/
+**/playwright-report/
+**/blob-report/
+**/playwright/.cache/
Review Comment:
I'm not sure we need those. These files are generate on each run?
##########
packages/dmn-editor/tests/e2e/connections/connectTwoExistingNodes.spec.ts:
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "@playwright/test";
+import { env } from "../../../env";
+
+test.beforeEach(async ({ page, browserName }, testInfo) => {
+ test.skip(browserName === "webkit", "hover() and dragTo() cause troubles on
webkit");
+ await page.goto(
+
`http://localhost:${env.dmnEditor.storybook.port}/iframe.html?args=&id=example-dmndevwebapp--empty-model&viewMode=story`
Review Comment:
We shouldn't use the dev-webapp as a target for our tests, its purpose is to
help us during development, where we can already have a model ready to interact
(e.g. sample, traffic violation, etc). So you need to create a new Story in a
clean state for the tests.
##########
packages/dmn-editor/stories/dev/DevWebApp.stories.ts:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { Meta, StoryObj } from "@storybook/react";
+
+import { DevWebApp, EMPTY_DMN_15 } from "../../dev-webapp/src/DevWebApp";
+import { DEFAULT_DEV_WEBAPP_DMN } from "../../dev-webapp/src/DefaultDmn";
+
+// More on how to set up stories at:
https://storybook.js.org/docs/writing-stories#default-export
+const meta: Meta<typeof DevWebApp> = {
+ title: "Example/DmnDevWebApp",
+ component: DevWebApp,
+ parameters: {},
+ // This component will have an automatically generated Autodocs entry:
https://storybook.js.org/docs/writing-docs/autodocs
+ tags: ["autodocs"],
Review Comment:
This property is auto generating the documentation without the .mdx file. If
you remove it, you'll have a Story without documentation.
##########
packages/dmn-editor/stories/dev/DevWebApp.stories.ts:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { Meta, StoryObj } from "@storybook/react";
+
+import { DevWebApp, EMPTY_DMN_15 } from "../../dev-webapp/src/DevWebApp";
+import { DEFAULT_DEV_WEBAPP_DMN } from "../../dev-webapp/src/DefaultDmn";
+
+// More on how to set up stories at:
https://storybook.js.org/docs/writing-stories#default-export
+const meta: Meta<typeof DevWebApp> = {
+ title: "Example/DmnDevWebApp",
Review Comment:
We could use the same approach of the `boxed-expression-component` and named
it `Dev/Web App`.
##########
packages/dmn-editor/package.json:
##########
@@ -61,30 +67,47 @@
"@babel/core": "^7.16.0",
"@babel/preset-env": "^7.16.0",
"@babel/preset-react": "^7.16.0",
+ "@babel/preset-typescript": "^7.22.5",
"@kie-tools-core/webpack-base": "workspace:*",
"@kie-tools/eslint": "workspace:*",
+ "@kie-tools/playwright-base": "workspace:*",
"@kie-tools/root-env": "workspace:*",
+ "@kie-tools/storybook-base": "workspace:*",
"@kie-tools/tsconfig": "workspace:*",
+ "@playwright/test": "^1.38.1",
+ "@storybook/addon-essentials": "^7.3.2",
+ "@storybook/addon-interactions": "^7.3.2",
+ "@storybook/addon-links": "^7.3.2",
+ "@storybook/addon-onboarding": "^1.0.11",
+ "@storybook/blocks": "^7.3.2",
+ "@storybook/react": "^7.3.2",
+ "@storybook/react-webpack5": "^7.3.2",
+ "@storybook/test": "^7.3.2",
+ "@storybook/testing-library": "^0.2.2",
Review Comment:
I'm not sure we're require all of these. Could you please double check?
##########
packages/dmn-editor/tests/e2e/connections/createNewNodeFromExistingOne.spec.ts:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "@playwright/test";
+import { env } from "../../../env";
+
+test.beforeEach(async ({ page, browserName }, testInfo) => {
+ test.skip(browserName === "webkit", "hover() and dragTo() cause troubles on
webkit");
+ await page.goto(
+
`http://localhost:${env.dmnEditor.storybook.port}/iframe.html?args=&id=example-dmndevwebapp--empty-model&viewMode=story`
+ );
+});
+
+test.describe("Create new node from existing one", () => {
+ test("InputData -> Decision", async ({ page }) => {
Review Comment:
You could have take advantage of nested descriptions to organize the tests,
for example:
test.describe("Input Data", () => {
test("Decision", () => {
})
})
##########
packages/dmn-editor/tests/e2e/connections/betweenTwoExistingNodes.spec.ts:
##########
@@ -0,0 +1,191 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "@playwright/test";
+import { env } from "../../../env";
+
+test.beforeEach(async ({ page }, testInfo) => {
+ await page.goto(
+
`http://localhost:${env.dmnEditor.storybook.port}/iframe.html?args=&id=example-dmndevwebapp--empty-model&viewMode=story`
Review Comment:
We shouldn't use the dev-webapp as a target for our tests, its purpose is to
help us during development, where we can already have a model ready to interact
(e.g. the sample model, traffic violation, etc). The ideal is to create a new
Story in a clean state for the tests.
##########
packages/dmn-editor/tests/e2e/connections/createNewNodeFromExistingOne.spec.ts:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "@playwright/test";
+import { env } from "../../../env";
+
+test.beforeEach(async ({ page, browserName }, testInfo) => {
+ test.skip(browserName === "webkit", "hover() and dragTo() cause troubles on
webkit");
+ await page.goto(
+
`http://localhost:${env.dmnEditor.storybook.port}/iframe.html?args=&id=example-dmndevwebapp--empty-model&viewMode=story`
+ );
+});
+
+test.describe("Create new node from existing one", () => {
Review Comment:
I would change it to "Add connected node", mimicking the functionality name.
I've added more about this on the review comment.
##########
packages/dmn-editor/.babelrc.json:
##########
@@ -0,0 +1,18 @@
+{
+ "sourceType": "unambiguous",
+ "presets": [
+ [
+ "@babel/preset-env",
+ {
+ "targets": {
+ "chrome": 100,
+ "safari": 15,
+ "firefox": 91
+ }
+ }
+ ],
+ "@babel/preset-typescript",
+ "@babel/preset-react"
+ ],
Review Comment:
Yes, I guess it would be good. This is the babel config file used by the
Storybook, right?
##########
packages/dmn-editor/env/index.js:
##########
@@ -28,6 +28,11 @@ module.exports =
composeEnv([require("@kie-tools/root-env/env")], {
port: 3001,
},
},
+ dmnEditor: {
+ storybook: {
+ port: 9900,
+ },
+ },
Review Comment:
We should change to a different port to enable starting both Storybooks at
the same time if we wish.
##########
packages/dmn-editor/stories/dev/DevWebApp.stories.ts:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { Meta, StoryObj } from "@storybook/react";
+
+import { DevWebApp, EMPTY_DMN_15 } from "../../dev-webapp/src/DevWebApp";
+import { DEFAULT_DEV_WEBAPP_DMN } from "../../dev-webapp/src/DefaultDmn";
+
+// More on how to set up stories at:
https://storybook.js.org/docs/writing-stories#default-export
+const meta: Meta<typeof DevWebApp> = {
+ title: "Example/DmnDevWebApp",
+ component: DevWebApp,
+ parameters: {},
+ // This component will have an automatically generated Autodocs entry:
https://storybook.js.org/docs/writing-docs/autodocs
+ tags: ["autodocs"],
+};
+
+export default meta;
+
+type Story = StoryObj<typeof DevWebApp>;
+
+// More on writing stories with args:
https://storybook.js.org/docs/writing-stories/args
+export const GettingStartedModel: Story = {
Review Comment:
For the purpose of the Dev Web App we will need only one story, with the
entire component, and a few samples, like the `boxed-expression-component`. We
could add the traffic-violation, can-drive, etc. It would require to modify the
current header to incorporate such changes.
##########
packages/dmn-editor/dev-webapp/src/DevWebApp.tsx:
##########
@@ -45,7 +43,13 @@ const EMPTY_DMN_15 = () => `<?xml version="1.0"
encoding="UTF-8"?>
name="DMN${generateUuid()}">
</definitions>`;
-export function DevWebApp() {
+interface DevWebAppProps {
+ initialModel: string;
+}
+
+export function DevWebApp(props: DevWebAppProps) {
+ const initialDmnMarshaller = useMemo(() => getMarshaller(props.initialModel,
{ upgradeTo: "latest" }), []);
Review Comment:
The dev webapp will be on the Storybook. This means you remove the current
structure, and move all necessary content from the "dev-webapp" to the
"stories/dev" folder. The start script will start the Storybook application.
##########
packages/dmn-editor/stories/__assets__/.gitkeep:
##########
Review Comment:
Leftover?
##########
packages/dmn-editor/src/diagram/nodes/OutgoingStuffNodePanel.tsx:
##########
@@ -74,6 +74,7 @@ export function OutgoingStuffNodePanel(props: { isVisible:
boolean; nodeTypes: N
type={"source"}
style={handleStyle}
position={RF.Position.Top}
+ title={edgeType}
Review Comment:
I'd say it's not a problem to add this metadata to the edge and node.
##########
packages/dmn-editor/package.json:
##########
@@ -20,11 +20,17 @@
"build:dev": "rimraf dist && pnpm copy:css && tsc -p tsconfig.json",
"build:dev-webapp": "rimraf ./dist-dev && webpack -c
./dev-webapp/webpack.config.js --env prod",
"build:prod": "rimraf dist && pnpm copy:css && pnpm lint && tsc -p
tsconfig.json && pnpm test && pnpm test:it",
+ "build:storybook": "storybook build -o dist-storybook",
"copy:css": "copyfiles -u 1 \"src/**/*.{sass,scss,css}\" dist/",
"lint": "run-script-if --bool \"$(build-env linters.run)\" --then
\"kie-tools--eslint ./src\"",
"start": "webpack serve -c ./dev-webapp/webpack.config.js --host 0.0.0.0
--env dev",
+ "start:new": "run-script-os",
+ "start:new:linux:darwin": "cross-env STORYBOOK_PORT=$(build-env
dmnEditor.storybook.port) pnpm storybook-base --storybookArgs=\"dev
--no-open\"",
+ "start:new:win32": "pnpm powershell \"cross-env STORYBOOK_PORT=$(build-env
dmnEditor.storybook.port) pnpm storybook-base --storybookArgs='dev --no-open'",
"test": "run-script-if --ignore-errors \"$(build-env
tests.ignoreFailures)\" --bool \"$(build-env tests.run)\" --then \"jest
--silent --verbose --passWithNoTests\"",
- "test:it": "echo 'No IT tests to run at this moment.'"
+ "test:e2e": "run-script-if --ignore-errors \"$(build-env
endToEndTests.ignoreFailures)\" --bool \"$(build-env endToEndTests.run)\"
--then \"pnpm rimraf ./dist-e2e\" \"pnpm test:e2e:run\"",
+ "test:e2e:open": "pnpm exec playwright show-report dist-e2e-tests/reports",
+ "test:e2e:run": "pnpm exec playwright test"
Review Comment:
You can remove all the `dev-webapp` related scripts. We will only have the
Storybook scripts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
