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

feat: allow to embed existing component to onboarding (#3755) #3763

Merged
merged 4 commits into from Sep 6, 2023

Conversation

lstocchi
Copy link
Contributor

@lstocchi lstocchi commented Sep 5, 2023

What does this PR do?

This PR allows to embed an existing svelte component to the onboarding. So far only the create component is allowed.

Screenshot/screencast of this PR

onboarding_podman_2

What issues does this PR fix or reference?

it fixes #3755

How to test this PR?

  1. run the onboarding without having any podman machine

@lstocchi lstocchi requested review from benoitf and a team as code owners September 5, 2023 10:42
@lstocchi lstocchi requested review from dgolovin and cdrage and removed request for a team September 5, 2023 10:42
Signed-off-by: lstocchi <lstocchi@redhat.com>
});
</script>

{#if embeddedComponent === 'create' && providerInfo && configurationItems}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 100% we need to add documentation to this part as unlike the other micromark features this seems really hidden.

If a provider such as Red Hat Sandbox were to add create to their extension, they wouldn't be able to figure it out through package.json / as well as other "onboarding-related" documentation.

With this PR, we are requiring that the provider has a containerProviderConnection provided.

We also have a kubernetes-specific one as well? So it's two things were misssing documentation for.

I'm fine if we create an issue for this so we can make sure we document the update.

Copy link
Contributor Author

@lstocchi lstocchi Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I still didn't add any doc for the onboarding as i was waiting it to settle down a bit. Or i would have to rewrite it from scratch almost every PR as it changed a lot from the first implementation.
I should be close to have a working onboarding for podman (install -> set some setting -> create/start machine), then i'll work on #3633 which covers all aspects of how to create an onboarding (including the supported embedded components).

Regarding the kubernetes-specific one - not sure if i got what you're saying but the code support the kubernetesProviderConnectionCreation. So if we create a workflow for kind it should work fine.

Copy link
Contributor

@cdrage cdrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments, but otherwise the code works great. Messed around with a bit having it go to the creation page and tried breaking things, but in all worked well! Thanks for implementing being able to get to the creation screen.

Signed-off-by: lstocchi <lstocchi@redhat.com>
hideCloseButton="{true}" />
{:else}
<div aria-label="not supported warning" class="flex min-h-[500px] items-center justify-center">
This provider does not support this mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add the component that is not found. It may help for troubleshooting.

⚠️ This extension does not provide a component of the type "{component}"

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks 👍 thanks for the quick fixes

Signed-off-by: lstocchi <lstocchi@redhat.com>
@lstocchi lstocchi merged commit f8fc3ca into containers:main Sep 6, 2023
8 checks passed
@lstocchi lstocchi deleted the i3755 branch September 6, 2023 15:29
@podman-desktop-bot podman-desktop-bot added this to the 1.4.0 milestone Sep 6, 2023
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.

Add step to create new podman machine in podman onboarding
5 participants