fix(ui): flickering of Typeahead when selecting images (podman-desktop#18277)

* fix(ui): flickering of Typeahead when selecting images

Signed-off-by: Tomáš Bordák <tbordak@redhat.com>

* fix(ui): catch rejected promise + use searchFunction() only for the last request (podman-desktop#18277)
Signed-off-by: Tomáš Bordák <tbordak@redhat.com>

* fix(ui): reset highlightIndex after Typehead component is closed + correct and add unit tests
Signed-off-by: Tomáš Bordák <tbordak@redhat.com>

* fix(ui): address coderabbit review for CreateContainerFromExistingImage.svelte
Signed-off-by: Tomáš Bordák <tbordak@redhat.com>

---------

Signed-off-by: Tomáš Bordák <tbordak@redhat.com>

Author: 0xKalasi

packages/renderer/src/lib/container/CreateContainerFromExistingImage.svelte

@@ -45,6 +45,7 @@ let values: TypeaheadItem[] = $state([]);
 
 let imageToPull: string = $state('');
 let sortResults: ((a: string, b: string) => number) | undefined = $state();
+let searchRequestId = 0;
 
 let providerConnections = $derived(
   $providerInfos
@@ -188,10 +189,10 @@ function validateImageName(image: string): void {
 
 // allTags is defined if last search was a query to search tags of an image
 let allTags: string[] | undefined = undefined;
-async function searchImages(value: string): Promise<string[]> {
+async function searchImages(value: string): Promise<{ images: string[]; tags: string[] | undefined }> {
   if (value.includes(':')) {
     if (allTags !== undefined) {
-      return allTags.filter(i => i.startsWith(value));
+      return { images: allTags.filter(i => i.startsWith(value)), tags: allTags };
     }
     const parts = value.split(':');
     const originalImage = parts[0];
@@ -200,12 +201,11 @@ async function searchImages(value: string): Promise<string[]> {
       image = image.slice(DOCKER_PREFIX_WITH_SLASH.length);
     }
     const tags = await window.listImageTagsInRegistry({ image });
-    allTags = tags.map(t => `${originalImage}:${t}`);
-    return allTags.filter(i => i.startsWith(value));
+    const computedTags = tags.map(t => `${originalImage}:${t}`);
+    return { images: computedTags.filter(i => i.startsWith(value)), tags: computedTags };
   }
-  allTags = undefined;
   if (value === undefined || value.trim() === '') {
-    return [];
+    return { images: [], tags: undefined };
   }
   const options: ImageSearchOptions = {
     query: '',
@@ -218,12 +218,11 @@ async function searchImages(value: string): Promise<string[]> {
     options.registry = registry;
     options.query = rest.join('/');
   }
-  let result: string[];
   const searchResult = await window.searchImageInRegistry(options);
-  result = searchResult.map(r => {
+  const result = searchResult.map(r => {
     return [options.registry, r.name].join('/');
   });
-  return result;
+  return { images: result, tags: undefined };
 }
 
 async function searchLocalImages(value: string): Promise<string[]> {
@@ -236,8 +235,7 @@ async function searchLocalImages(value: string): Promise<string[]> {
     }
     return [];
   });
-  matchingLocalImages = localImagesNames.flat().filter(image => image !== '' && image.includes(value));
-  return matchingLocalImages;
+  return localImagesNames.flat().filter(image => image !== '' && image.includes(value));
 }
 
 let latestTagMessage: string | undefined = $state();
@@ -315,9 +313,16 @@ async function searchFunction(value: string): Promise<void> {
   // do not search for images if no connection is selected
   if (!selectedProviderConnection) return;
 
+  const requestId = ++searchRequestId;
   value = value.trim();
   const localImagesValues = await searchLocalImages(value);
-  const remoteImagesValues = await searchImages(value);
+  const remoteSearchResult = await searchImages(value);
+
+  if (requestId !== searchRequestId) return;
+
+  matchingLocalImages = localImagesValues;
+  allTags = remoteSearchResult.tags;
+
   sortResults = (a: string, b: string): number => {
     const dockerIoValue = `docker.io/${value}`;
     const aStartsWithValue = a.startsWith(value) || a.startsWith(dockerIoValue);
@@ -332,8 +337,8 @@ async function searchFunction(value: string): Promise<void> {
   };
 
   values = [
-    ...localImagesValues.map(value => ({ value: value, group: 'Local Images' })),
-    ...remoteImagesValues.map(value => ({ value: value, group: 'Registry Images' })),
+    ...localImagesValues.map(v => ({ value: v, group: 'Local Images' })),
+    ...remoteSearchResult.images.map(v => ({ value: v, group: 'Registry Images' })),
   ];
 }
 
@@ -359,6 +364,11 @@ function onContainerConnectionChange(): void {
   matchingLocalImages = [];
   imageToPull = '';
 }
+
+// trigger search on mount to populate the typeahead with local images
+onMount(() => {
+  searchFunction('').catch(() => {});
+});
 </script>
 
 <EngineFormPage title="Select an image">

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

@@ -415,11 +415,11 @@ test('should include heading based on given order and searchFunctions order', as
   });
 });
 
-test('list opens on focus', async () => {
+test('list opens on focus without triggering search again', async () => {
   let searchResult: TypeaheadItem[] = [];
-  const searchFunction = async (): Promise<void> => {
+  const searchFunction = vi.fn(async (): Promise<void> => {
     searchResult = ['text1', 'text2', 'text3', 'text4'].map(value => ({ value: value }));
-  };
+  });
 
   const { rerender } = render(Typeahead, {
     onInputChange: searchFunction,
@@ -429,17 +429,67 @@ test('list opens on focus', async () => {
 
   const input = screen.getByRole('textbox');
   await userEvent.click(input);
+  await userEvent.keyboard('text');
 
   await waitFor(() => expect(searchResult.length > 0).toBeTruthy());
   await rerender({ resultItems: searchResult });
 
+  const callCountAfterTyping = searchFunction.mock.calls.length;
+
+  // click away and then select the input with tab to focus it
+  await userEvent.click(document.body);
+  await userEvent.tab();
+
   await waitFor(() => {
     const list = screen.getByRole('row');
     const items = within(list).getAllByRole('button');
     expect(items.length).toBe(4);
     expect(items[0].textContent).toBe('text1');
-    expect(items[1].textContent).toBe('text2');
-    expect(items[2].textContent).toBe('text3');
-    expect(items[3].textContent).toBe('text4');
+  });
+
+  expect(searchFunction.mock.calls.length).toBe(callCountAfterTyping);
+});
+
+test('highlightIndex resets when reopening list via focus', async () => {
+  let searchResult: TypeaheadItem[] = [];
+  const searchFunction = async (s: string): Promise<void> => {
+    searchResult = s ? [{ value: s + '01' }, { value: s + '02' }, { value: s + '03' }] : [];
+  };
+  const { rerender } = render(Typeahead, {
+    onInputChange: searchFunction,
+    resultItems: searchResult,
+    delay: 10,
+  });
+
+  const input = screen.getByRole('textbox');
+  await userEvent.type(input, 'term');
+  await waitFor(() => expect(searchResult.length > 0).toBeTruthy());
+  await rerender({ resultItems: searchResult });
+
+  await waitFor(() => {
+    const items = within(screen.getByRole('row')).getAllByRole('button');
+    expect(items.length).toBe(3);
+  });
+
+  // navigate down to select an item
+  await userEvent.keyboard('[ArrowDown]');
+  await userEvent.keyboard('[ArrowDown]');
+
+  await waitFor(async () => {
+    await tick();
+    const items = within(screen.getByRole('row')).getAllByRole('button');
+    assertItemSelected(items, 1);
+  });
+
+  // close by clicking away, then reopen by focusing
+  await userEvent.click(document.body);
+  await waitFor(() => assertIsListVisible(false));
+
+  await userEvent.tab();
+
+  await waitFor(async () => {
+    await tick();
+    const items = within(screen.getByRole('row')).getAllByRole('button');
+    assertItemSelected(items, -1);
   });
 });

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

@@ -146,7 +146,6 @@ function onUpKey(e: KeyboardEvent): void {
       value = items[highlightIndex];
       makeVisible();
     } else if (highlightIndex === 0) {
-      highlightIndex = -1;
       value = userValue;
       close();
     }
@@ -231,6 +230,7 @@ function open(): void {
 }
 
 function close(): void {
+  highlightIndex = -1;
   opened = false;
 }
 
@@ -277,7 +277,7 @@ function onWindowClick(e: Event): void {
     name={name}
     oninput={onInput}
     onkeydown={onKeyDown}
-    onfocus={processInput}
+    onfocus={open}
     use:requestFocus />
   {#if loading}
     <Spinner size="1em" />