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

Provide back button navigation in new cluster wizard #60

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

angelozerr
Copy link
Collaborator

Provide back button navigation in new cluster wizard

Fixes #21

Signed-off-by: azerr [email protected]

@angelozerr
Copy link
Collaborator Author

angelozerr commented Jan 4, 2021

This PR improves the two wizards:

  • create cluster
  • add topic

with:

  • back button
  • validation
  • default value

for add topic, if you execute add topic from command palette it provides list of cluster to pick it at first.

Here a demo with cluster wizard:

AddClusterDemo

Here a demo with topic wizard:

AddTopicDemo

I need to improve this PR:

@angelozerr
Copy link
Collaborator Author

@jlandersen my PR fails on windows only when tests are executed, I don't understand why although I have not written some tests
https://github.com/jlandersen/vscode-kafka/pull/60/checks?check_run_id=1644491781

Have you some idea?

@angelozerr angelozerr force-pushed the button-navigation branch 2 times, most recently from ad935ae to 5d9dd84 Compare January 4, 2021 17:06
@angelozerr angelozerr marked this pull request as ready for review January 4, 2021 17:07
@fbricon
Copy link
Collaborator

fbricon commented Jan 4, 2021

Use simple messages like "X is required"

Check for blank values: "X can not be blank"

@angelozerr
Copy link
Collaborator Author

Use simple messages like "X is required"

Check for blank values: "X can not be blank"

Done, I updated my PR and now we have a validators.ts which provides commons validator for required filed, positive number etc which follows the same pattern message.

src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Show resolved Hide resolved
src/wizards/multiStepInput.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/validators.ts Outdated Show resolved Hide resolved
src/wizards/clusters.ts Outdated Show resolved Hide resolved
@fbricon
Copy link
Collaborator

fbricon commented Jan 6, 2021

Topic name validation is wrong. _ should be accepted (as explained in the error message)

Screenshot 2021-01-06 at 11 49 00

src/wizards/validators.ts Outdated Show resolved Hide resolved
@fbricon fbricon merged commit b1c8c0e into jlandersen:master Jan 6, 2021
@fbricon
Copy link
Collaborator

fbricon commented Jan 6, 2021

Thanks @angelozerr. This makes the wizard workflows much better. Now on to #64 and #65 to improve validation

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.

Provide back button navigation in new cluster wizard
2 participants