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

fix: move new registry button to header #3245

Merged
merged 2 commits into from Jul 18, 2023

Conversation

deboer-tim
Copy link
Collaborator

What does this PR do?

Fixes the initial/most basic part of issue #3233: adds support for adding actions to the SettingsPage header and moves the Registries page Add registry button from being bottom-right aligned into the header.

Screenshot/screencast of this PR

Screenshot 2023-07-17 at 4 26 25 PM

What issues does this PR fix or reference?

First part of issue #3233.

How to test this PR?

Open Settings > Registries page and confirm the button still works.

Fixes the initial/most basic part of issue containers#3233: adds support for adding actions
to the SettingsPage header and moves the Registries page Add registry button from
being bottom-right aligned into the header.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim deboer-tim requested review from a team and benoitf as code owners July 17, 2023 20:26
@deboer-tim deboer-tim requested review from jeffmaury and lstocchi and removed request for a team July 17, 2023 20:26
@@ -243,6 +243,19 @@ const processPasswordElement = (node: HTMLInputElement, registry: containerDeskt
</script>

<SettingsPage title="Registries">
<svelte:fragment slot="actions">
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need a svelte fragment to not break flow layout or just a div would be good ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A div would work fine, I was just running with the 'don't add an extra html component if it's not necessary'. You'd prefer I switch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a div would be simpler but I don't have a strong opinion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Switched, and turned on auto-merge.

Switch to using a div.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim deboer-tim enabled auto-merge (rebase) July 18, 2023 02:26
@deboer-tim deboer-tim merged commit 9e9d7ee into containers:main Jul 18, 2023
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.3.0 milestone Jul 18, 2023
@deboer-tim deboer-tim deleted the add-registry branch February 5, 2024 22:22
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.

None yet

3 participants