feat: modified UI to use patternfly #78

ilan-pinto · 2023-07-20T12:20:49Z

No description provided.

openshift-ci · 2023-07-20T12:20:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ilan-pinto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ilan-pinto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Ilan Pinto <[email protected]>

ilan-pinto · 2023-07-20T12:34:09Z

@IlonaShishov, can you run a quick test on this branch?

using patternfly react Signed-off-by: Ilan Pinto <[email protected]>

TomerFi

@ilan-pinto Very good work, sorry it took me so long to review this, you can take it up with my manager. ;-)
Honestly, very good work. Me being the 'yeke' that I am, most of my comments/change requests are semantics only.

.vscode/launch.json

TomerFi · 2023-09-20T00:51:05Z

.vscode/settings.json

+    "typescript.tsc.autoDetect": "off",
+    "cSpell.words": [
+        "klusterlet"
+    ]


As a former cscpell user myself, on multiple occasions, I clicked the "add to project settings" instead of "add to user settings". :-)

I think I accidently marked line 10 too, but I only meant the cSpell.words array. I think you also deleted "typescript.tsc.autoDetect": "off" when resolving this. Sorry.

src/providers/contextWebProvider.ts

TomerFi · 2023-09-20T00:53:56Z

src/providers/contextWebProvider.ts

 		// get the loader instance
 		let load = loader.Load.getLoader();
-
+		


I think you can configure VSCode to auto-delete stuff like this on-save.

how? what is the problem?

The spaces your IDE allows you to mistakenly add to line endings in files you open. VSCode has an option to auto delete these when you hit save.

webview-ui/src/comp/Klusterlet.tsx

webview-ui/src/comp/Placements.tsx

webview-ui/src/comp/SubscriptionStatuses.tsx

webview-ui/tsconfig.json

TomerFi · 2023-09-20T01:18:20Z

package.json

+		"@patternfly/react-core": "^4.276.8",
+		"@patternfly/react-styles": "^4.92.6",
+		"@patternfly/react-table": "^4.113.0",
+		"@types/d3-shape": "^3.1.1",


I noticed you added the patternfly dependencies to both the root project.json and the webui one. Is this necessary?

to be honest, i am not sure.
only after adding it to both spaces it worked.
but it could that should have added part of in root and rest in other folder

I think they should go only in webui, I also see other dependencies that exist only in webui and not the root, and it works well. Can you please try building while keeping this only in webui?

Signed-off-by: Ilan Pinto <[email protected]> fixed PR comments Signed-off-by: Ilan Pinto <[email protected]>

.gitignore

.vscode/launch.json

TomerFi · 2023-10-23T15:45:55Z

src/providers/contextWebProvider.ts

 		// get the loader instance
 		let load = loader.Load.getLoader();
-
+		


The spaces your IDE allows you to mistakenly add to line endings in files you open. VSCode has an option to auto delete these when you hit save.

TomerFi · 2023-10-23T15:51:14Z

templates/Argo/placement.yaml

I think global should suffice. New ManagedClusters are only members in the default set until they are moved to their own set.

TomerFi · 2023-10-23T15:58:17Z

.vscode/settings.json

@@ -7,5 +7,4 @@
        "out": true // set this to false to include "out" folder in search results
    },
    // Turn off tsc task auto detection since we have the necessary tasks as npm scripts
-    "typescript.tsc.autoDetect": "off"


"typescript.tsc.autoDetect": "off" was here before this PR, see my next comment.

TomerFi · 2023-10-23T16:00:29Z

.vscode/settings.json

+    "typescript.tsc.autoDetect": "off",
+    "cSpell.words": [
+        "klusterlet"
+    ]


I think I accidently marked line 10 too, but I only meant the cSpell.words array. I think you also deleted "typescript.tsc.autoDetect": "off" when resolving this. Sorry.

TomerFi · 2023-10-23T16:10:18Z

test-workspace/.vscode/settings.json

@@ -0,0 +1,3 @@
+{


I don't see you did. Regardless, it's not quite necessary.
We're ok with a test-workspace inside the test folder, we use it for testing and we crafted our .gitignore to ignore just its content.
But you accidentally copied test/test-workspace folder to test-workspace. Simply delete test-workspace from the root (only from the root) and we're good.

TomerFi · 2023-10-23T16:10:56Z

tsconfig.json

@@ -9,14 +9,27 @@
 		"sourceMap": true,
 		"rootDirs": [
 			"src",
-			"test"
+			"test",
+			"styles"


I think we don't, can you try removing it and build?

TomerFi · 2023-10-23T16:15:27Z

package.json

+		"@patternfly/react-core": "^4.276.8",
+		"@patternfly/react-styles": "^4.92.6",
+		"@patternfly/react-table": "^4.113.0",
+		"@types/d3-shape": "^3.1.1",


I think they should go only in webui, I also see other dependencies that exist only in webui and not the root, and it works well. Can you please try building while keeping this only in webui?

openshift-merge-robot · 2024-08-04T16:21:01Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot requested review from vbelouso and zvigrinberg July 20, 2023 12:20

openshift-ci bot added the approved label Jul 20, 2023

sync upstream

bcea5e1

Signed-off-by: Ilan Pinto <[email protected]>

ilan-pinto force-pushed the patternfly-main branch from 635f7ea to 1d79d00 Compare July 20, 2023 12:32

feat(patternfly webview): replaced vscode view with patternfly

4ef114e

using patternfly react Signed-off-by: Ilan Pinto <[email protected]>

ilan-pinto force-pushed the patternfly-main branch from 1d79d00 to 4ef114e Compare July 23, 2023 13:33

ilan-pinto requested review from TomerFi and IlonaShishov and removed request for zvigrinberg and vbelouso July 25, 2023 10:29

ilan-pinto force-pushed the patternfly-main branch 2 times, most recently from faaf134 to 92c5166 Compare July 26, 2023 13:22

TomerFi requested changes Sep 20, 2023

View reviewed changes

openshift-ci bot assigned TomerFi Sep 20, 2023

ilan-pinto force-pushed the patternfly-main branch 3 times, most recently from f42c771 to ac4cc5b Compare October 18, 2023 11:14

feat: klusterlet and dashboard fixes

1852719

Signed-off-by: Ilan Pinto <[email protected]> fixed PR comments Signed-off-by: Ilan Pinto <[email protected]>

ilan-pinto force-pushed the patternfly-main branch from ac4cc5b to 1852719 Compare October 18, 2023 13:01

ilan-pinto requested a review from TomerFi October 18, 2023 13:46

TomerFi requested changes Oct 23, 2023

View reviewed changes

openshift-merge-robot added the needs-rebase label Aug 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: modified UI to use patternfly #78

feat: modified UI to use patternfly #78

ilan-pinto commented Jul 20, 2023

openshift-ci bot commented Jul 20, 2023

ilan-pinto commented Jul 20, 2023

TomerFi left a comment

TomerFi Sep 20, 2023

TomerFi Oct 23, 2023

TomerFi Sep 20, 2023

ilan-pinto Oct 18, 2023

TomerFi Oct 23, 2023

TomerFi Sep 20, 2023

ilan-pinto Oct 18, 2023 •

edited

Loading

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023 •

edited

Loading

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023

TomerFi Oct 23, 2023

openshift-merge-robot commented Aug 4, 2024

		// get the loader instance
		let load = loader.Load.getLoader();

feat: modified UI to use patternfly #78

Are you sure you want to change the base?

feat: modified UI to use patternfly #78

Conversation

ilan-pinto commented Jul 20, 2023

openshift-ci bot commented Jul 20, 2023

ilan-pinto commented Jul 20, 2023

TomerFi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilan-pinto Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomerFi Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-merge-robot commented Aug 4, 2024

ilan-pinto Oct 18, 2023 •

edited

Loading

TomerFi Oct 23, 2023 •

edited

Loading