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: allow editing of build containerfile #4471

Merged
merged 1 commit into from Oct 24, 2023
Merged

fix: allow editing of build containerfile #4471

merged 1 commit into from Oct 24, 2023

Conversation

deboer-tim
Copy link
Collaborator

What does this PR do?

Changes the build image form from a readonly text field that you click on to browser, to a more common editable text field with a Browse button. Likewise the build directory was hidden until you entered a containerfile before, now it is always visible and uses the same text field + browse button.

This change makes the form use a more common pattern, and allows e2e tests to more easily enter text in either field.

The drawback to this approach is that there is no more validation than before - the build button enables whenever there is text in both fields. This happens automatically when you use the Browse button like before, but if you edit or copy/paste and have a typo you won't find out until you click Build. We could add more validation in the future, but IMHO the better usability/form layout is enough of a benefit to make this change in the meantime.

Screenshot/screencast of this PR

Screenshot 2023-10-23 at 9 52 25 AM

What issues does this PR fix or reference?

Fixes #3824.

How to test this PR?

Just go to Images > Build Image.

Changes the build image form from a readonly text field that you click on to
browser, to a more common editable text field with a Browse button. Likewise
the build directory was hidden until you entered a containerfile before, now
it is always visible and uses the same text field + browse button.

This change makes the form use a more common pattern, and allows e2e tests
to more easily enter text in either field.

The drawback to this approach is that there is no more validation than
before - the build button enables whenever there is text in both fields.
This happens automatically when you use the Browse button like before,
but if you edit or copy/paste and have a typo you won't find out until you
click Build. We could add more validation in the future, but IMHO the
better usability/form layout is enough of a benefit to make this change in
the meantime.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim deboer-tim requested review from benoitf and a team as code owners October 23, 2023 13:53
@deboer-tim deboer-tim requested review from dgolovin and feloy and removed request for a team October 23, 2023 13:53
@benoitf
Copy link
Collaborator

benoitf commented Oct 23, 2023

hello, I'm fine with the change but I think we need to postpone the merge to be after the release

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.

probably need form component (not asked by this PR) but we do have several ways to submit files in the code base

@deboer-tim deboer-tim merged commit 09583b7 into main Oct 24, 2023
9 checks passed
@deboer-tim deboer-tim deleted the build-task branch October 24, 2023 15:02
@podman-desktop-bot podman-desktop-bot added this to the 1.6.0 milestone Oct 24, 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
3 participants