pr feedback

web/src/lib/components/elements/buttons/button.svelte

@@ -26,6 +26,11 @@
   export let fullwidth = false;
   export let border = false;
   export let title: string | undefined = '';
+  export const focus = () => {
+    ref.focus;
+  };
+
+  let ref: HTMLButtonElement;
 
   const colorClasses: Record<Color, string> = {
     primary:
@@ -55,6 +60,7 @@
 </script>
 
 <button
+  bind:this={ref}
   {type}
   {disabled}
   {title}

web/src/lib/components/faces-page/merge-suggestion-modal.svelte

@@ -8,19 +8,46 @@
   import { mdiArrowLeft, mdiClose, mdiMerge } from '@mdi/js';
   import Icon from '$lib/components/elements/icon.svelte';
 
+  import { assetViewingStore } from '$lib/stores/asset-viewing.store';
+
+  export let personMerge1: PersonResponseDto;
+  export let personMerge2: PersonResponseDto;
+  export let potentialMergePeople: PersonResponseDto[];
+
+  let { isViewing: showAssetViewer } = assetViewingStore;
+  let choosePersonToMerge = false;
+  let changeFocus = false;
+  let focusButtonNo: () => void;
+  let focusButtonYes: () => void;
+
+  const title = personMerge2.name;
+
   const dispatch = createEventDispatcher<{
     reject: void;
     confirm: [PersonResponseDto, PersonResponseDto];
     close: void;
   }>();
 
-  export let personMerge1: PersonResponseDto;
+  const handleKeyboardPress = (event: KeyboardEvent) => {
-  export let personMerge2: PersonResponseDto;
+    if (!$showAssetViewer) {
-  export let potentialMergePeople: PersonResponseDto[];
+      switch (event.key) {
+        case 'Tab':
+          event.preventDefault();
 
-  let choosePersonToMerge = false;
+          if (changeFocus) {
+            focusButtonYes();
+          } else {
+            focusButtonNo();
+          }
 
-  const title = personMerge2.name;
+          changeFocus = !changeFocus;
+          return;
+        case 'Escape':
+          dispatch('close');
+          return;
+      }
+    }
+  };
 
   const changePersonToMerge = (newperson: PersonResponseDto) => {
     const index = potentialMergePeople.indexOf(newperson);
@@ -29,6 +56,8 @@
   };
 </script>
 
+<svelte:document on:keypress={handleKeyboardPress} />
+
 <FullScreenModal on:clickOutside={() => dispatch('close')}>
   <div class="flex h-full w-full place-content-center place-items-center overflow-hidden">
     <div
@@ -114,8 +143,10 @@
         <p class="text-sm text-gray-500 dark:text-gray-300">They will be merged together</p>
       </div>
       <div class="mt-8 flex w-full gap-4 px-4 pb-4">
-        <Button color="gray" fullwidth on:click={() => dispatch('reject')}>No</Button>
+        <Button bind:focus={focusButtonNo} color="gray" fullwidth on:click={() => dispatch('reject')}>No</Button>
-        <Button fullwidth on:click={() => dispatch('confirm', [personMerge1, personMerge2])}>Yes</Button>
+        <Button bind:focus={focusButtonYes} fullwidth on:click={() => dispatch('confirm', [personMerge1, personMerge2])}
+          >Yes</Button
+        >
       </div>
     </div>
   </div>

web/src/routes/(user)/people/[personId]/+page.svelte

@@ -86,25 +86,10 @@
    **/
   let searchWord: string;
   let isSearchingPeople = false;
-
+  let focusedElements: (HTMLButtonElement | null)[] = Array(20)
-  const searchPeople = async () => {
+    .fill(null)
-    if ((people.length < 20 && name.startsWith(searchWord)) || name === '') {
+    .map(() => null);
-      return;
+  let indexFocus: number | null = null;
-    }
-    const timeout = setTimeout(() => (isSearchingPeople = true), 100);
-    try {
-      const { data } = await api.searchApi.searchPerson({ name });
-      people = data;
-      searchWord = name;
-    } catch (error) {
-      people = [];
-      handleError(error, "Can't search people");
-    } finally {
-      clearTimeout(timeout);
-    }
-
-    isSearchingPeople = false;
-  };
 
   $: isAllArchive = Array.from($selectedAssets).every((asset) => asset.isArchived);
   $: isAllFavorite = Array.from($selectedAssets).every((asset) => asset.isFavorite);
@@ -114,6 +99,7 @@
   $: {
     if (people) {
       suggestedPeople = !name ? [] : searchNameLocal(name, people, 5, data.person.id);
+      indexFocus = null;
     }
   }
 
@@ -127,8 +113,69 @@
       viewMode = ViewMode.MERGE_FACES;
     }
   });
+
+  const handleKeyboardPress = (event: KeyboardEvent) => {
+    if (suggestedPeople.length === 0) {
+      return;
+    }
+    if (!$showAssetViewer) {
+      event.stopPropagation();
+      switch (event.key) {
+        case 'Tab':
+        case 'ArrowDown':
+          event.preventDefault();
+          if (indexFocus === null) {
+            indexFocus = 0;
+          } else if (indexFocus === suggestedPeople.length - 1) {
+            indexFocus = 0;
+          } else {
+            indexFocus++;
+          }
+          focusedElements[indexFocus]?.focus();
+          return;
+        case 'ArrowUp':
+          if (indexFocus === null) {
+            indexFocus = 0;
+            return;
+          }
+          if (indexFocus === 0) {
+            indexFocus = suggestedPeople.length - 1;
+          } else {
+            indexFocus--;
+          }
+          focusedElements[indexFocus]?.focus();
+
+          return;
+        case 'Enter':
+          if (indexFocus !== null) {
+            handleSuggestPeople(suggestedPeople[indexFocus]);
+          }
+      }
+    }
+  };
+
+  const searchPeople = async () => {
+    if ((people.length < 20 && name.startsWith(searchWord)) || name === '') {
+      return;
+    }
+    const timeout = setTimeout(() => (isSearchingPeople = true), 100);
+    try {
+      const { data } = await api.searchApi.searchPerson({ name });
+      indexFocus = null;
+      people = data;
+      searchWord = name;
+    } catch (error) {
+      people = [];
+      handleError(error, "Can't search people");
+    } finally {
+      clearTimeout(timeout);
+    }
+
+    isSearchingPeople = false;
+  };
+
   const handleEscape = () => {
-    if ($showAssetViewer) {
+    if ($showAssetViewer || viewMode === ViewMode.SUGGEST_MERGE) {
       return;
     }
     if ($isMultiSelectState) {
@@ -333,6 +380,8 @@
   };
 </script>
 
+<svelte:document on:keydown={handleKeyboardPress} />
+
 {#if viewMode === ViewMode.SUGGEST_MERGE}
   <MergeSuggestionModal
     {personMerge1}
@@ -468,24 +517,24 @@
                 </div>
               {:else}
                 {#each suggestedPeople as person, index (person.id)}
-                  <div
+                  <button
-                    class="flex border-t border-x border-gray-400 dark:border-immich-dark-gray h-14 place-items-center bg-gray-200 p-2 dark:bg-gray-700 hover:bg-gray-300 hover:dark:bg-[#232932] {index ===
+                    bind:this={focusedElements[index]}
+                    class="flex w-full border-t border-gray-400 dark:border-immich-dark-gray h-14 place-items-center bg-gray-200 p-2 dark:bg-gray-700 hover:bg-gray-300 hover:dark:bg-[#232932] focus:bg-gray-300 focus:dark:bg-[#232932] {index ===
                     suggestedPeople.length - 1
                       ? 'rounded-b-lg border-b'
                       : ''}"
+                    on:click={() => handleSuggestPeople(person)}
                   >
-                    <button class="flex w-full place-items-center" on:click={() => handleSuggestPeople(person)}>
+                    <ImageThumbnail
-                      <ImageThumbnail
+                      circle
-                        circle
+                      shadow
-                        shadow
+                      url={api.getPeopleThumbnailUrl(person.id)}
-                        url={api.getPeopleThumbnailUrl(person.id)}
+                      altText={person.name}
-                        altText={person.name}
+                      widthStyle="2rem"
-                        widthStyle="2rem"
+                      heightStyle="2rem"
-                        heightStyle="2rem"
+                    />
-                      />
+                    <p class="ml-4 text-gray-700 dark:text-gray-100">{person.name}</p>
-                      <p class="ml-4 text-gray-700 dark:text-gray-100">{person.name}</p>
+                  </button>
-                    </button>
-                  </div>
                 {/each}
               {/if}
             </div>