fix: call fileItem onChange only when there is actually a change (pod…

…man-desktop#9075)

* fix: call fileItem onChange only when it actually changes

Signed-off-by: lstocchi <[email protected]>

* fix: use callback

Signed-off-by: lstocchi <[email protected]>

* fix: extract lambda to function

Signed-off-by: lstocchi <[email protected]>

* fix: handle input

Signed-off-by: lstocchi <[email protected]>

---------

Signed-off-by: lstocchi <[email protected]>

packages/renderer/src/lib/preferences/item-formats/FileItem.spec.ts

@@ -26,6 +26,7 @@ import type { IConfigurationPropertyRecordedSchema } from '../../../../../main/s
 import FileItem from './FileItem.svelte';
 
 const openDialogMock = vi.fn();
+
 beforeAll(() => {
   (window as any).getConfigurationValue = vi.fn();
   (window as any).openDialog = openDialogMock;
@@ -91,3 +92,27 @@ test('Ensure clicking on browse invokes openDialog with corresponding directory
     selectors: ['openDirectory'],
   });
 });
+
+test('Ensure the onChange is called if the fileInput onChange is triggered', async () => {
+  const filename = 'somefile';
+  openDialogMock.mockResolvedValue([filename]);
+
+  const record: IConfigurationPropertyRecordedSchema = {
+    id: 'record',
+    title: 'record',
+    parentId: 'parent.record',
+    description: 'record-description',
+    type: 'string',
+    format: 'file',
+  };
+
+  const onChangeMock = vi.fn().mockResolvedValue('');
+  render(FileItem, { record, value: '', onChange: onChangeMock });
+  const browseButton = screen.getByRole('button');
+  expect(browseButton).toBeInTheDocument();
+  await userEvent.click(browseButton);
+
+  expect(openDialogMock).toHaveBeenCalled();
+
+  expect(onChangeMock).toHaveBeenCalled();
+});

packages/renderer/src/lib/preferences/item-formats/FileItem.svelte

@@ -9,19 +9,16 @@ export let record: IConfigurationPropertyRecordedSchema;
 export let value: string = '';
 export let onChange = async (_id: string, _value: string) => {};
 
-let lastValue: string;
-
 let invalidEntry = false;
 let dialogOptions: OpenDialogOptions = {
   title: `Select ${record.description}`,
   selectors: record.format === 'folder' ? ['openDirectory'] : ['openFile'],
 };
 
-$: if (value !== lastValue) {
+function onChangeFileInput(value: string): void {
   if (record.id) {
     onChange(record.id, value).catch((_: unknown) => (invalidEntry = true));
   }
-  lastValue = value;
 }
 </script>
 
@@ -30,6 +27,7 @@ $: if (value !== lastValue) {
     id="input-standard-{record.id}"
     name={record.id}
     bind:value={value}
+    onChange={onChangeFileInput}
     readonly={record.readonly ?? true}
     placeholder={record.placeholder}
     options={dialogOptions}

packages/renderer/src/lib/ui/FileInput.spec.ts

@@ -42,11 +42,12 @@ test('Expect clicking the button opens file dialog', async () => {
   expect(openDialogMock).toHaveBeenCalled();
 });
 
-test('Expect value to change when selecting via file dialog', async () => {
+test('Expect onChange function called with new value when selecting via file dialog', async () => {
   const filename = 'somefile';
   openDialogMock.mockResolvedValue([filename]);
 
-  render(FileInput, {});
+  const onChangeMock = vi.fn();
+  render(FileInput, { options: { title: 'title' }, onChange: onChangeMock });
 
   const browseButton = screen.getByRole('button');
   expect(browseButton).toBeInTheDocument();
@@ -57,4 +58,45 @@ test('Expect value to change when selecting via file dialog', async () => {
   const input = screen.getByRole('textbox');
   expect(input).toBeInTheDocument();
   expect(input).toHaveValue(filename);
+
+  expect(onChangeMock).toHaveBeenCalledWith(filename);
+});
+
+test('Expect onChange function called if user types', async () => {
+  const filename = 'somefile';
+  const onChangeMock = vi.fn();
+  render(FileInput, { options: { title: 'title' }, onChange: onChangeMock });
+
+  const browseButton = screen.getByRole('button');
+  expect(browseButton).toBeInTheDocument();
+  await userEvent.click(browseButton);
+
+  expect(openDialogMock).toHaveBeenCalled();
+
+  const input = screen.getByRole('textbox');
+  expect(input).toBeInTheDocument();
+
+  await userEvent.type(input, filename);
+
+  expect(onChangeMock).toHaveBeenCalledWith(filename);
+});
+
+test('Expect onChange function called if user paste content', async () => {
+  const filename = 'somefile';
+  const onChangeMock = vi.fn();
+  render(FileInput, { options: { title: 'title' }, onChange: onChangeMock });
+
+  const browseButton = screen.getByRole('button');
+  expect(browseButton).toBeInTheDocument();
+  await userEvent.click(browseButton);
+
+  expect(openDialogMock).toHaveBeenCalled();
+
+  const input = screen.getByRole('textbox');
+  expect(input).toBeInTheDocument();
+
+  await userEvent.click(input);
+  await userEvent.paste(filename);
+
+  expect(onChangeMock).toHaveBeenCalledWith(filename);
 });

packages/renderer/src/lib/ui/FileInput.svelte

@@ -10,13 +10,20 @@ export let value: string | undefined = undefined;
 export let options: OpenDialogOptions;
 export let readonly: boolean = false;
 export let required: boolean = false;
+export let onChange: (value: string) => void = () => {};
 
 async function openDialog() {
   const result = await window.openDialog(options);
   if (result?.[0]) {
     value = result[0];
+    onChange(value);
   }
 }
+
+function onInput(event: Event): void {
+  const inputEvent = event as Event & { target: HTMLInputElement };
+  onChange(inputEvent.target.value);
+}
 </script>
 
 <div class="flex flex-row grow space-x-1.5">
@@ -25,7 +32,7 @@ async function openDialog() {
     name={name}
     class={$$props.class || ''}
     bind:value={value}
-    on:input
+    on:input={onInput}
     on:keypress
     placeholder={placeholder}
     readonly={readonly}