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-tools-issues#2641: Upgrade Patternfly to v5 #2853

Open
wants to merge 104 commits into
base: main
Choose a base branch
from

Conversation

kumaradityaraj
Copy link
Contributor

@kumaradityaraj kumaradityaraj commented Jan 13, 2025

Closes #1877
Closes #2641

Upgrade Patternfly from v4 to v5 to the full repository following PatternFly's migration.

guide: https://www.patternfly.org/get-started/upgrade/#patternfly-5

Updated packages
@kie-tools-core/editor
@kie-tools-examples/base64png-editor
@kie-tools-examples/uniforms-patternfly
@kie-tools-examples/webapp
@kie-tools/boxed-expression-component
@kie-tools/dashbuilder-component-table
@kie-tools/dashbuilder-component-uniforms
@kie-tools/dashbuilder-component-victory-charts
@kie-tools/dashbuilder-editor
@kie-tools/dashbuilder-viewer
@kie-tools/dashbuilder-viewer-deployment-webapp
@kie-tools/dev-deployment-dmn-form-webapp
@kie-tools/dmn-editor
@kie-tools/dmn-editor-envelope
@kie-tools/dmn-editor-standalone
@kie-tools/dmn-runner
@kie-tools/form
@kie-tools/form-dmn
@kie-tools/import-java-classes-component
@kie-tools/kie-bc-editors
@kie-tools/online-editor
@kie-tools/pmml-editor
@kie-tools/runtime-tools-components
@kie-tools/runtime-tools-management-console-webapp
@kie-tools/runtime-tools-process-dev-ui-webapp
@kie-tools/runtime-tools-process-enveloped-components
@kie-tools/runtime-tools-shared-enveloped-components
@kie-tools/runtime-tools-shared-webapp-components
@kie-tools/runtime-tools-swf-enveloped-components
@kie-tools/runtime-tools-swf-webapp-components
@kie-tools/runtime-tools-task-console-webapp
@kie-tools/scesim-editor
@kie-tools/serverless-logic-web-tools
@kie-tools/serverless-workflow-combined-editor
@kie-tools/serverless-workflow-dev-ui-webapp
@kie-tools/serverless-workflow-text-editor
@kie-tools/sonataflow-management-console-webapp
@kie-tools/text-editor
@kie-tools/uniforms-patternfly
@kie-tools/unitables
@kie-tools/unitables-dmn
@kie-tools/yard-editor
sonataflow-deployment-webapp

See more: https://docs.google.com/spreadsheets/d/1V1LxLuu4Vv6-V8ncOMKH7vXHog5fQiwcnMw-glLq32k/edit?gid=1948850227#gid=1948850227

kumaradityaraj and others added 30 commits October 7, 2024 13:14
@fantonangeli
Copy link
Contributor

@tiagobento @yesamer
We are having a failure on a test related to stunner-editors :

TEST FAILED: testBusinessKnowledgeModelExpressionDecisionTableDefaultOutputRef

Can you please guide or help us regarding this?

@yesamer
Copy link
Contributor

yesamer commented Jan 17, 2025

@kumaradityaraj @fantonangeli
I checked the failing tests. As I feared, this change has an impact on the old GWT Stunner editors, which are PF3-based.
Please notice the following screenshot:

Screenshot 2025-01-17 at 13 03 10

On the left, you can see the version without your changes. On the right, is the version with the PF5 migration.
You may notice there are some minor style changes, I guess those changes may have effects on the failing test, but I still didn't check that.

I guess the reason for this behavior is that in DMN Editor we nested a React component (the table) that is affected by your changes, in a GWT Editor based on PF3.

//cc @tiagobento

@yesamer
Copy link
Contributor

yesamer commented Jan 17, 2025

@kumaradityaraj @fantonangeli To fix the test please:
https://github.com/apache/incubator-kie-tools/blob/main/packages/stunner-editors/kie-wb-common-dmn/kie-wb-common-dmn-webapp-kogito-runtime/src/test/java/org/kie/workbench/common/dmn/showcase/client/selenium/locator/EditorXPathLocator.java#L46

Change
return new EditorXPathLocator("//p[@class='expression-info-name pf-u-text-truncate name']");
to
return new EditorXPathLocator("//p[@class='expression-info-name pf-v5-u-text-truncate name']");

Screenshot 2025-01-17 at 13 23 44

@fantonangeli
Copy link
Contributor

Thank you so much @yesamer.
Yes the pf-v5- prefix for CSS has been introduced with the new PF version.

@tiagobento tiagobento requested a review from jomarko January 17, 2025 13:20
@tiagobento
Copy link
Contributor

@jomarko Adding you as a reviewer here if you're available to take a look at the online-editor, dmn-editor, and dev-deployment-dmn-form-webapp components, as I think you're very familiar with them. Thanks!

@tiagobento
Copy link
Contributor

Hey, a green build! Awesome work @kumaradityaraj and @fantonangeli! Looking forward to hearing back from reviewers once we do some sanity checks in key packages. 🚀

@tiagobento tiagobento changed the title kie-tools-issues#2641: [SonataFlow] Upgrade Patternfly to v5 kie-tools-issues#2641: Upgrade Patternfly to v5 Jan 17, 2025
@fantonangeli fantonangeli linked an issue Jan 17, 2025 that may be closed by this pull request
@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

My review will be provided per multiple comments, I will announce once my last comment was provided so we can somehow better organize the discussion about opened points. Thank you for the PR, it is a huge and important effort!

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

01 Home Screen

Screenshot 2025-01-20 115359

Has these three points, that seems as candidates for a fix:

  1. success default text next to URL input field shouldn't be present, also the scenarios when valid URL is provided success success and invalid URL is provided success error should be fixed
  2. actions = text should be not displayed for the asset
  3. trash icons were previously shown only once the item was hovered, no strong opinion about this, personally I do not mind much if they are displayed always, however previously they were not.

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

02 Connected accounts

image
The actions = text should not be present, also the icons miss the color.

The same issues, text and color are present once user click Add in the dialogue above:
image

Please notice Kubernetes icon is wrong.

If there is no connected account, there seems to be inappropriate dialogue size. Please notice there is scrollbar , however it is not needed, all text is visible. If user scrolls, no additional text is present.

image

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

03 Extended services settings

Please notice (default) text in the screenshot. It should be not there.
image

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

04 No Dev Deployments

I understand some fonts and colors needs to change due to similar change. However, not sure about the boundary, what is acceptable and what not. So just bringing this change, not sure if reaction is needed. Notice icon color and text size.

main

Screenshot 2025-01-20 123233

pull request

Screenshot 2025-01-20 123229

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

05 Model Name text field

Please compare main:
Screenshot 2025-01-20 123636
with pr:
Screenshot 2025-01-20 123642

I think the textbox box should not be visible

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

06 New DMN Editor button

Is not clickable, please compare main:
Screenshot 2025-01-20 133859
with PR:
Screenshot 2025-01-20 133903

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

07 New File Authentication

Has some unexpected text
image

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

08 Back to diagram Icon

Styling should be updated. Please compare main:
Screenshot 2025-01-20 134300

with the PR:
Screenshot 2025-01-20 134251

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

09 Font and Shape Icon position

In the DMN properties panel should be updated. Please compare main:
Screenshot 2025-01-20 134536

and PR:
Screenshot 2025-01-20 134531

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

10 Expression type icon

Is positioned in some strange way, causing not readable as good as it used to be. Please compare main:
Screenshot 2025-01-20 140530

with the PR:
Screenshot 2025-01-20 140442

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

11 New Data Type landing page

❗ BLOCKER ❗ the data type is not editable

  1. hasFilter size different than main and the grey background color is much more used comparing main.

main

Screenshot 2025-01-20 141836

pr

Screenshot 2025-01-20 141846

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

12 Java function parameters help icon

position is wrong
image

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

13 DRG Nodes and External Nodes icon

Size is smaller than other icons
Screenshot 2025-01-20 144801

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

14 Folder Toolbar Icons

Please compare group icon size in main:
Screenshot 2025-01-20 145046

vs PR:
Screenshot 2025-01-20 145040

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

15 Include model styling

is different on main:
Screenshot 2025-01-20 145400

vs in PR:
Screenshot 2025-01-20 145337

also the background for included model cards is different on main:
Screenshot 2025-01-20 145604

vs pr:
Screenshot 2025-01-20 145609

@jomarko
Copy link
Contributor

jomarko commented Jan 20, 2025

16 Create github gist

Has some unexpected content if we compare main:
image

with PR:
Screenshot 2025-01-20 160435

The same issue we see for github repository when we compare main:
image

with PR:
image

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 will stop my review here. I think I tested quite well 2 of three componenets I was asked:

  • online-editor ✔️
  • dmn-editor ✔️
  • dev-deployment-dmn-form-webapp 🚫

I think I could test also runtime-tools-process-dev-ui-webapp however will do maybe in secound round of review.

dev-deployment-dmn-form-webapp

The problem I have for testing dev-deployment-dmn-form-webapp is I never used it as standalone and locally

I made this build:

  • KIE_TOOLS_BUILD__buildContainerImages=true pnpm -F @kie-tools/dev-deployment-kogito-quarkus-blank-app-image... build:prod
  • KIE_TOOLS_BUILD__buildContainerImages=true pnpm -F @kie-tools/dev-deployment-dmn-form-webapp-image... build:prod

Then started:

  • docker run -p 8080:8080 -e DEV_DEPLOYMENT__UPLOAD_SERVICE_API_KEY=dev docker.io/apache/incubator-kie-sandbox-dev-deployment-kogito-quarkus-blank-app:main
  • docker run -d -p 8081:8081 -e DEV_DEPLOYMENT_DMN_FORM_WEBAPP_QUARKUS_APP_ORIGIN=http://localhost:8080 docker.io/apache/incubator-kie-sandbox-dev-deployment-dmn-form-webapp:main

And tried to deploy DMN as:

  • curl -X POST -H "Content-Type: multipart/form-data" -F "myFile=/home/jomarko/kie-tools/examples/process-compact-architecture/src/main/resources/assets.zip" 'http://localhost:8080/upload?apiKey=dev'

However always get some error, I need more guidance how to deploy some dmn on the running images.

[dev-deployment-upload-service] ℹ️  Upload arrived...
[dev-deployment-upload-service] ❌ ERROR: Reading uploaded file failed:
[dev-deployment-upload-service] ❌ ERROR: http: no such file
2025/01/20 15:53:33 http: panic serving 172.17.0.1:50788: runtime error: invalid memory address or nil pointer dereference
goroutine 49 [running]:
net/http.(*conn).serve.func1()
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:1903 +0xbe
panic({0x667c00?, 0x8c0d60?})
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/runtime/panic.go:770 +0x132
main.main.func3({0x727320, 0xc0002ba000}, 0xc0002ac000)
        /home/jomarko/kie-tools/packages/dev-deployment-upload-service/main.go:169 +0x7d5
net/http.HandlerFunc.ServeHTTP(0x8cce50?, {0x727320?, 0xc0002ba000?}, 0x10?)
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:2171 +0x29
net/http.(*ServeMux).ServeHTTP(0x40e745?, {0x727320, 0xc0002ba000}, 0xc0002ac000)
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:2688 +0x1ad
net/http.serverHandler.ServeHTTP({0x7266a0?}, {0x727320?, 0xc0002ba000?}, 0x6?)
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:3142 +0x8e
net/http.(*conn).serve(0xc000296000, {0x727840, 0xc000200210})
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:2044 +0x5e8
created by net/http.(*Server).Serve in goroutine 19
        /nix/store/frc5188kgv3ws0n999c7cy5vi2f8k4jp-go-1.22.9/share/go/src/net/http/server.go:3290 +0x4b4
[dev-deployment-upload-service] ⚠️  Upload arrived, but another arrived earlier. Server should be in the process of gracefully shutting down.

Please let me know once the PR is ready for my another review round. Once more, thank you for this huge effort!

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.

Upgrade PatternFly to v5 in all kie-tools packages [uniforms-patternfly] Upgrade @patternfly to v5.0.0
5 participants