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

Use Webview to manage cluster settings #193

Merged
merged 1 commit into from
Jul 6, 2021

Conversation

angelozerr
Copy link
Collaborator

Use Webview to manage cluster settings

Fixes #88

Signed-off-by: azerr azerr@redhat.com

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 6 times, most recently from db41b46 to 7b3b2e3 Compare May 26, 2021 09:51
@fbricon
Copy link
Collaborator

fbricon commented May 26, 2021

Screenshot 2021-05-26 at 12 05 54

  • Page title should be "Edit Kafka cluster" or (New Kafka cluster)
  • Header on top should be removed
  • Cluster block should move up
  • labels and text fields should be aligned and having the same width
  • Authentication "None" should have a label
  • Hide Back/Next, Finish -> Save

@fbricon fbricon mentioned this pull request Jun 2, 2021
@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 2 times, most recently from ba2e91a to 2290928 Compare June 8, 2021 08:42
@angelozerr
Copy link
Collaborator Author

Please note now I'm consuming 0.2.5 vscode-wizard which has been published (which fixes some bugs that you mentionned)

Page title should be "Edit Kafka cluster" or (New Kafka cluster)

Fixed (for the moment I manage only Edit)

Header on top should be removed

Fixed by 0.2.5

Cluster block should move up

Sorry I don't understand what you mean?

labels and text fields should be aligned and having the same width

Fixed, but there is a problem with select that I have reported in redhat-developer/vscode-wizard#33

Authentication "None" should have a label

Fixed

Hide Back/Next, Finish -> Save

TODO

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from 2290928 to 885b734 Compare June 8, 2021 08:45
@angelozerr
Copy link
Collaborator Author

angelozerr commented Jun 8, 2021

Here the current result:

image

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 3 times, most recently from a7e945a to 9f80da9 Compare June 8, 2021 09:08
@angelozerr
Copy link
Collaborator Author

angelozerr commented Jun 8, 2021

I pushed a new commit, I have started to declare the UI for SSL:

image

Please note:

@rgrunber
Copy link

rgrunber commented Jun 8, 2021

I'd probably use the full name of the term.
Ca -> Certificate Authority (CA)
Cert -> Certificate

I don't think the SSL checkbox should exist. The underlying field, cluster.ssl should simply be set to true when the other 3 fields are valid/non-empty.

@fbricon
Copy link
Collaborator

fbricon commented Jun 8, 2021

@rgrunber no you can set SSL=true without specifying certs

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from 9f80da9 to 68b666d Compare June 9, 2021 07:35
@angelozerr
Copy link
Collaborator Author

I'd probably use the full name of the term.
Ca -> Certificate Authority (CA)
Cert -> Certificate

Fixed

I don't think the SSL checkbox should exist. The underlying field, cluster.ssl should simply be set to true when the other 3 fields are valid/non-empty.

As @fbricon said we can enable SSL without certificates

@angelozerr
Copy link
Collaborator Author

angelozerr commented Jun 9, 2021

Just for your information I created a PR redhat-developer/vscode-wizard#37 to improve UI, here the result with light:

image

and with dark

image

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 4 times, most recently from bd6fa41 to fb9b8e9 Compare June 21, 2021 15:02
@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from fb9b8e9 to fea8ce4 Compare June 23, 2021 02:53
@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 2 times, most recently from 989c967 to 33d1b63 Compare July 1, 2021 10:04
@angelozerr angelozerr marked this pull request as ready for review July 1, 2021 10:22
@fbricon
Copy link
Collaborator

fbricon commented Jul 1, 2021

certificate/key files should allow ".pem" and other file extensions (see https://kafka.js.org/docs/configuration#ssl and https://www.ssl.com/guide/pem-der-crt-and-cer-x-509-encodings-and-conversions/)

@angelozerr
Copy link
Collaborator Author

New Cluster(s) title should be "Add New Cluster(s)"

fixed

cluster provider: remove the "none" option, have "configure manually" preselected

fixed

cluster provider combo should not crop selected provider

it's an issue for vscode-wizard, see redhat-developer/vscode-wizard#58

If vscode-rhoas is installed, you select the Red Hat provider, click finish, the wizard stays opened. It should close. If you click finish a 2nd time, it closes.

That's strange, I cannot reproduce on Windows OS?

@fbricon
Copy link
Collaborator

fbricon commented Jul 1, 2021

When renaming a cluster, the selected cluster in the status bar is not renamed

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from edb90c9 to daed248 Compare July 2, 2021 09:20
@angelozerr
Copy link
Collaborator Author

When selecting SASL/*, the SSL box is not automatically checked, rendering the cluster unable to connect.

I cannot do that with vscode-wizard, but I validate that:

image

When creating a new cluster, the wizard is stil opened after saving

I cannot reproduce it -(

If you click save again, after checking the ssl box, a "Cluster name 'foo' already exists." error is shown. The wizard should be closed, or at the very least should enter edit mode.

fixed

When editing an existing cluster:

change name
save
check SSL => cluster name already exists

Same error than below, fixed

certificate/key files should allow ".pem" and other file extensions (see https://kafka.js.org/docs/configuration#ssl and https://www.ssl.com/guide/pem-der-crt-and-cer-x-509-encodings-and-conversions/)

I add other extensions according to https://www.tutorialsteacher.com/https/ssl-certificate-format

Here the filters now:

image

I allow . because I fear to forget some file extensions.

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from daed248 to b35e046 Compare July 2, 2021 11:11
@angelozerr
Copy link
Collaborator Author

When renaming a cluster, the selected cluster in the status bar is not renamed

fixed

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 3 times, most recently from e320fb2 to eefcfe2 Compare July 2, 2021 15:10
@angelozerr
Copy link
Collaborator Author

angelozerr commented Jul 2, 2021

@fbricon everything should be fixed exept:

@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 6 times, most recently from fc3ef88 to a2b7da9 Compare July 6, 2021 10:03
@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch 2 times, most recently from 55611de to b5bfab5 Compare July 6, 2021 14:32
src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Outdated Show resolved Hide resolved
@angelozerr angelozerr force-pushed the cluster-webview-wizard-page branch from b5bfab5 to d2d54e4 Compare July 6, 2021 15:56
Fixes jlandersen#88

Signed-off-by: azerr <azerr@redhat.com>
@fbricon fbricon merged commit 889a606 into jlandersen:master Jul 6, 2021
@fbricon
Copy link
Collaborator

fbricon commented Jul 6, 2021

Nice work @angelozerr! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Webview to manage cluster settings
3 participants