From 09583b760788bcb678e69d99d89f4c812cc8a8a9 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Mon, 23 Oct 2023 09:51:24 -0400 Subject: [PATCH] fix: allow editing of build containerfile 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 --- .../image/BuildImageFromContainerfile.spec.ts | 14 ++++-- .../image/BuildImageFromContainerfile.svelte | 48 ++++++++++--------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/packages/renderer/src/lib/image/BuildImageFromContainerfile.spec.ts b/packages/renderer/src/lib/image/BuildImageFromContainerfile.spec.ts index c24e22f39432..4aa8888fe7a3 100644 --- a/packages/renderer/src/lib/image/BuildImageFromContainerfile.spec.ts +++ b/packages/renderer/src/lib/image/BuildImageFromContainerfile.spec.ts @@ -88,9 +88,12 @@ test('Expect Build button is enabled', async () => { render(BuildImageFromContainerfile, {}); const containerFilePath = screen.getByRole('textbox', { name: 'Containerfile Path' }); - expect(containerFilePath).toBeInTheDocument(); - await userEvent.click(containerFilePath); + await userEvent.type(containerFilePath, '/somepath/containerfile'); + + const buildFolder = screen.getByRole('textbox', { name: 'Build Context Directory' }); + expect(buildFolder).toBeInTheDocument(); + await userEvent.type(buildFolder, '/somepath'); const buildButton = screen.getByRole('button', { name: 'Build' }); expect(buildButton).toBeInTheDocument(); @@ -102,9 +105,12 @@ test('Expect Done button is enabled once build is done', async () => { render(BuildImageFromContainerfile, {}); const containerFilePath = screen.getByRole('textbox', { name: 'Containerfile Path' }); - expect(containerFilePath).toBeInTheDocument(); - await userEvent.click(containerFilePath); + await userEvent.type(containerFilePath, '/somepath/containerfile'); + + const buildFolder = screen.getByRole('textbox', { name: 'Build Context Directory' }); + expect(buildFolder).toBeInTheDocument(); + await userEvent.type(buildFolder, '/somepath'); const buildButton = screen.getByRole('button', { name: 'Build' }); expect(buildButton).toBeInTheDocument(); diff --git a/packages/renderer/src/lib/image/BuildImageFromContainerfile.svelte b/packages/renderer/src/lib/image/BuildImageFromContainerfile.svelte index cf435edf58a4..76382636f281 100644 --- a/packages/renderer/src/lib/image/BuildImageFromContainerfile.svelte +++ b/packages/renderer/src/lib/image/BuildImageFromContainerfile.svelte @@ -27,7 +27,6 @@ let buildFinished = false; let containerImageName = 'my-custom-image'; let containerFilePath: string; let containerBuildContextDirectory: string; -let hasInvalidFields = true; let buildImageInfo: BuildImageInfo | undefined = undefined; let providers: ProviderInfo[] = []; @@ -36,6 +35,8 @@ let selectedProvider: ProviderContainerConnectionInfo | undefined = undefined; let selectedProviderConnection: ProviderContainerConnectionInfo | undefined = undefined; let logsTerminal: Terminal; +$: hasInvalidFields = !containerFilePath || !containerBuildContextDirectory; + function getTerminalCallback(): BuildImageCallback { return { onStream: function (data: string): void { @@ -117,7 +118,6 @@ async function getContainerfileLocation() { const result = await window.openFileDialog('Select Containerfile to build'); if (!result.canceled && result.filePaths.length === 1) { containerFilePath = result.filePaths[0]; - hasInvalidFields = false; if (!containerBuildContextDirectory) { // select the parent directory of the file as default // eslint-disable-next-line no-useless-escape @@ -146,28 +146,32 @@ async function getContainerBuildContextDirectory() {
-