From c6d24085176b9da263ed0d3ad659267cc802ce8d Mon Sep 17 00:00:00 2001
From: Ben Basten <45583362+ben-basten@users.noreply.github.com>
Date: Tue, 19 Mar 2024 12:56:41 +0000
Subject: [PATCH] feat(web): combobox accessibility improvements (#8007)

* bump skip link z index, to prevent overlap with the search box

* combobox refactor initial commit

* pull label into the combobox component

* feat(web): combobox accessibility improvements

* fix: replace crypto.randomUUID, fix border UI bug, simpler focus handling (#2)

* fix: handle changes in the selected option

* fix: better escape key handling in search bar

* fix: remove broken tailwind classes

Co-authored-by: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com>

* fix: remove custom "outclick" handler logic

* fix: use focusout instead of custom key handlers to detect focus change

* fix: move escape key handling to the window

Also add escape key handling to the input box, to make sure that the "recent searches" dropdown gets closed too.

* fix: better input event handling

Co-authored-by: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com>

* fix: highlighting selected dropdown element

---------

Co-authored-by: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com>
---
 .../elements/buttons/skip-link.svelte         |   2 +-
 .../shared-components/change-date.svelte      |   9 +-
 .../shared-components/combobox.svelte         | 219 +++++++++++++-----
 .../search-bar/search-bar.svelte              |  18 +-
 .../search-bar/search-camera-section.svelte   |  16 +-
 .../search-bar/search-location-section.svelte |  24 +-
 .../settings/setting-combobox.svelte          |   4 +
 .../user-settings-page/app-settings.svelte    |   1 +
 8 files changed, 214 insertions(+), 79 deletions(-)

diff --git a/web/src/lib/components/elements/buttons/skip-link.svelte b/web/src/lib/components/elements/buttons/skip-link.svelte
index 457661d65f..8a304469d7 100644
--- a/web/src/lib/components/elements/buttons/skip-link.svelte
+++ b/web/src/lib/components/elements/buttons/skip-link.svelte
@@ -14,7 +14,7 @@
   };
 </script>
 
-<div class="absolute top-2 left-2 transition-transform {isFocused ? 'translate-y-0' : '-translate-y-10 sr-only'}">
+<div class="absolute z-50 top-2 left-2 transition-transform {isFocused ? 'translate-y-0' : '-translate-y-10 sr-only'}">
   <Button
     size={'sm'}
     rounded={false}
diff --git a/web/src/lib/components/shared-components/change-date.svelte b/web/src/lib/components/shared-components/change-date.svelte
index 51d6611b5a..e0d51d76c0 100644
--- a/web/src/lib/components/shared-components/change-date.svelte
+++ b/web/src/lib/components/shared-components/change-date.svelte
@@ -83,8 +83,13 @@
         />
       </div>
       <div class="flex flex-col w-full mt-2">
-        <label for="timezone">Timezone</label>
-        <Combobox bind:selectedOption id="timezone" options={timezones} placeholder="Search timezone..." />
+        <Combobox
+          bind:selectedOption
+          id="settings-timezone"
+          label="Timezone"
+          options={timezones}
+          placeholder="Search timezone..."
+        />
       </div>
     </div>
   </ConfirmDialogue>
diff --git a/web/src/lib/components/shared-components/combobox.svelte b/web/src/lib/components/shared-components/combobox.svelte
index c69460640b..19fd73d25f 100644
--- a/web/src/lib/components/shared-components/combobox.svelte
+++ b/web/src/lib/components/shared-components/combobox.svelte
@@ -11,48 +11,93 @@
 
 <script lang="ts">
   import { fly } from 'svelte/transition';
-
   import Icon from '$lib/components/elements/icon.svelte';
-  import { clickOutside } from '$lib/utils/click-outside';
   import { mdiMagnify, mdiUnfoldMoreHorizontal, mdiClose } from '@mdi/js';
-  import { createEventDispatcher } from 'svelte';
+  import { createEventDispatcher, tick } from 'svelte';
   import IconButton from '../elements/buttons/icon-button.svelte';
+  import type { FormEventHandler } from 'svelte/elements';
+  import { shortcuts } from '$lib/utils/shortcut';
+  import { clickOutside } from '$lib/utils/click-outside';
 
-  export let id: string | undefined = undefined;
+  /**
+   * Unique identifier for the combobox.
+   */
+  export let id: string;
+  export let label: string;
+  export let hideLabel = false;
   export let options: ComboBoxOption[] = [];
   export let selectedOption: ComboBoxOption | undefined;
   export let placeholder = '';
 
+  /**
+   * Indicates whether or not the dropdown autocomplete list should be visible.
+   */
   let isOpen = false;
-  let inputFocused = false;
+  /**
+   * Keeps track of whether the combobox is actively being used.
+   */
+  let isActive = false;
   let searchQuery = selectedOption?.label || '';
+  let selectedIndex: number | undefined;
+  let optionRefs: HTMLElement[] = [];
+  const inputId = `combobox-${id}`;
+  const listboxId = `listbox-${id}`;
 
   $: filteredOptions = options.filter((option) => option.label.toLowerCase().includes(searchQuery.toLowerCase()));
 
+  $: {
+    searchQuery = selectedOption ? selectedOption.label : '';
+  }
+
   const dispatch = createEventDispatcher<{
     select: ComboBoxOption | undefined;
     click: void;
   }>();
 
-  const handleClick = () => {
-    searchQuery = '';
+  const activate = () => {
+    isActive = true;
+    openDropdown();
+  };
+
+  const deactivate = () => {
+    searchQuery = selectedOption ? selectedOption.label : '';
+    isActive = false;
+    closeDropdown();
+  };
+
+  const openDropdown = () => {
     isOpen = true;
-    inputFocused = true;
-    dispatch('click');
   };
 
-  let handleOutClick = () => {
-    // In rare cases it's possible for the input to still have focus and
-    // outclick to fire.
-    if (!inputFocused) {
-      isOpen = false;
-    }
-  };
-
-  let handleSelect = (option: ComboBoxOption) => {
-    selectedOption = option;
-    dispatch('select', option);
+  const closeDropdown = () => {
     isOpen = false;
+    selectedIndex = undefined;
+  };
+
+  const incrementSelectedIndex = async (increment: number) => {
+    if (filteredOptions.length === 0) {
+      selectedIndex = 0;
+    } else if (selectedIndex === undefined) {
+      selectedIndex = increment === 1 ? 0 : filteredOptions.length - 1;
+    } else {
+      selectedIndex = (selectedIndex + increment + filteredOptions.length) % filteredOptions.length;
+    }
+    await tick();
+    optionRefs[selectedIndex]?.scrollIntoView({ block: 'nearest' });
+  };
+
+  const onInput: FormEventHandler<HTMLInputElement> = (event) => {
+    openDropdown();
+    searchQuery = event.currentTarget.value;
+    selectedIndex = undefined;
+    optionRefs[0]?.scrollIntoView({ block: 'nearest' });
+  };
+
+  let onSelect = (option: ComboBoxOption) => {
+    selectedOption = option;
+    searchQuery = option.label;
+    dispatch('select', option);
+    closeDropdown();
   };
 
   const onClear = () => {
@@ -62,30 +107,80 @@
   };
 </script>
 
-<div class="relative w-full dark:text-gray-300 text-gray-700 text-base" use:clickOutside on:outclick={handleOutClick}>
+<label class="text-sm text-black dark:text-white" class:sr-only={hideLabel} for={inputId}>{label}</label>
+<div
+  class="relative w-full dark:text-gray-300 text-gray-700 text-base"
+  use:clickOutside={{ onOutclick: deactivate }}
+  on:focusout={(e) => {
+    if (e.relatedTarget instanceof Node && !e.currentTarget.contains(e.relatedTarget)) {
+      deactivate();
+    }
+  }}
+>
   <div>
-    {#if isOpen}
+    {#if isActive}
       <div class="absolute inset-y-0 left-0 flex items-center pl-3">
         <div class="dark:text-immich-dark-fg/75">
-          <Icon path={mdiMagnify} />
+          <Icon path={mdiMagnify} ariaHidden={true} />
         </div>
       </div>
     {/if}
 
     <input
-      {id}
       {placeholder}
-      role="combobox"
+      aria-activedescendant={selectedIndex || selectedIndex === 0 ? `${listboxId}-${selectedIndex}` : ''}
+      aria-autocomplete="list"
+      aria-controls={listboxId}
       aria-expanded={isOpen}
-      aria-controls={id}
-      class="immich-form-input text-sm text-left w-full !pr-12 transition-all"
-      class:!pl-8={isOpen}
+      autocomplete="off"
+      class:!pl-8={isActive}
       class:!rounded-b-none={isOpen}
-      class:cursor-pointer={!isOpen}
-      value={isOpen ? '' : selectedOption?.label || ''}
-      on:input={(e) => (searchQuery = e.currentTarget.value)}
-      on:focus={handleClick}
-      on:blur={() => (inputFocused = false)}
+      class:cursor-pointer={!isActive}
+      class="immich-form-input text-sm text-left w-full !pr-12 transition-all"
+      id={inputId}
+      on:click={activate}
+      on:focus={activate}
+      on:input={onInput}
+      role="combobox"
+      type="text"
+      value={searchQuery}
+      use:shortcuts={[
+        {
+          shortcut: { key: 'ArrowUp' },
+          onShortcut: () => {
+            openDropdown();
+            void incrementSelectedIndex(-1);
+          },
+        },
+        {
+          shortcut: { key: 'ArrowDown' },
+          onShortcut: () => {
+            openDropdown();
+            void incrementSelectedIndex(1);
+          },
+        },
+        {
+          shortcut: { key: 'ArrowDown', alt: true },
+          onShortcut: () => {
+            openDropdown();
+          },
+        },
+        {
+          shortcut: { key: 'Enter' },
+          onShortcut: () => {
+            if (selectedIndex !== undefined && filteredOptions.length > 0) {
+              onSelect(filteredOptions[selectedIndex]);
+            }
+            closeDropdown();
+          },
+        },
+        {
+          shortcut: { key: 'Escape' },
+          onShortcut: () => {
+            closeDropdown();
+          },
+        },
+      ]}
     />
 
     <div
@@ -95,37 +190,51 @@
     >
       {#if selectedOption}
         <IconButton color="transparent-gray" on:click={onClear} title="Clear value">
-          <Icon path={mdiClose} />
+          <Icon path={mdiClose} ariaLabel="Clear value" />
         </IconButton>
       {:else if !isOpen}
-        <Icon path={mdiUnfoldMoreHorizontal} />
+        <Icon path={mdiUnfoldMoreHorizontal} ariaHidden={true} />
       {/if}
     </div>
   </div>
 
-  {#if isOpen}
-    <div
-      role="listbox"
-      transition:fly={{ duration: 250 }}
-      class="absolute text-left text-sm w-full max-h-64 overflow-y-auto bg-white dark:bg-gray-800 rounded-b-lg border border-t-0 border-gray-300 dark:border-gray-900 z-10"
-    >
+  <ul
+    role="listbox"
+    id={listboxId}
+    transition:fly={{ duration: 250 }}
+    class="absolute text-left text-sm w-full max-h-64 overflow-y-auto bg-white dark:bg-gray-800 border-t-0 border-gray-300 dark:border-gray-900 rounded-b-xl z-10"
+    class:border={isOpen}
+    tabindex="-1"
+  >
+    {#if isOpen}
       {#if filteredOptions.length === 0}
-        <div class="px-4 py-2 font-medium">No results</div>
-      {/if}
-      {#each filteredOptions as option (option.label)}
-        {@const selected = option.label === selectedOption?.label}
-        <button
-          type="button"
+        <!-- svelte-ignore a11y-click-events-have-key-events -->
+        <li
+          role="option"
+          aria-selected={selectedIndex === 0}
+          aria-disabled={true}
+          class:bg-gray-100={selectedIndex === 0}
+          class:dark:bg-gray-700={selectedIndex === 0}
+          class="text-left w-full px-4 py-2 hover:bg-gray-100 dark:hover:bg-gray-700 cursor-default"
+          id={`${listboxId}-${0}`}
+          on:click={() => closeDropdown()}
+        >
+          No results
+        </li>
+      {/if}
+      {#each filteredOptions as option, index (option.label)}
+        <!-- svelte-ignore a11y-click-events-have-key-events -->
+        <li
+          aria-selected={index === selectedIndex}
+          bind:this={optionRefs[index]}
+          class="text-left w-full px-4 py-2 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all cursor-pointer aria-selected:bg-gray-100 aria-selected:dark:bg-gray-700"
+          id={`${listboxId}-${index}`}
+          on:click={() => onSelect(option)}
           role="option"
-          aria-selected={selected}
-          class="text-left w-full px-4 py-2 hover:bg-gray-100 dark:hover:bg-gray-700 transition-all"
-          class:bg-gray-300={selected}
-          class:dark:bg-gray-600={selected}
-          on:click={() => handleSelect(option)}
         >
           {option.label}
-        </button>
+        </li>
       {/each}
-    </div>
-  {/if}
+    {/if}
+  </ul>
 </div>
diff --git a/web/src/lib/components/shared-components/search-bar/search-bar.svelte b/web/src/lib/components/shared-components/search-bar/search-bar.svelte
index 041181f71b..8afa56df71 100644
--- a/web/src/lib/components/shared-components/search-bar/search-bar.svelte
+++ b/web/src/lib/components/shared-components/search-bar/search-bar.svelte
@@ -11,6 +11,7 @@
   import type { MetadataSearchDto, SmartSearchDto } from '@immich/sdk';
   import { getMetadataSearchQuery } from '$lib/utils/metadata-search';
   import { handlePromiseError } from '$lib/utils';
+  import { shortcut } from '$lib/utils/shortcut';
 
   export let value = '';
   export let grayTheme: boolean;
@@ -84,7 +85,16 @@
   };
 </script>
 
-<div class="w-full relative" use:clickOutside on:outclick={onFocusOut} on:escape={onFocusOut}>
+<svelte:window
+  use:shortcut={{
+    shortcut: { key: 'Escape' },
+    onShortcut: () => {
+      onFocusOut();
+    },
+  }}
+/>
+
+<div class="w-full relative" use:clickOutside={{ onOutclick: onFocusOut }}>
   <form
     draggable="false"
     autocomplete="off"
@@ -118,6 +128,12 @@
         bind:this={input}
         on:click={onFocusIn}
         disabled={showFilter}
+        use:shortcut={{
+          shortcut: { key: 'Escape' },
+          onShortcut: () => {
+            onFocusOut();
+          },
+        }}
       />
 
       <div class="absolute inset-y-0 {showClearIcon ? 'right-14' : 'right-5'} flex items-center pl-6 transition-all">
diff --git a/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte b/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte
index 6f377faffb..7acac54d8e 100644
--- a/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte
+++ b/web/src/lib/components/shared-components/search-bar/search-camera-section.svelte
@@ -40,24 +40,24 @@
 
   <div class="grid grid-cols-[repeat(auto-fit,minmax(10rem,1fr))] gap-5 mt-1">
     <div class="w-full">
-      <label class="text-sm text-black dark:text-white" for="search-camera-make">Make</label>
       <Combobox
-        id="search-camera-make"
-        options={toComboBoxOptions(makes)}
-        selectedOption={makeFilter ? { label: makeFilter, value: makeFilter } : undefined}
+        id="camera-make"
+        label="Make"
         on:select={({ detail }) => (filters.make = detail?.value)}
+        options={toComboBoxOptions(makes)}
         placeholder="Search camera make..."
+        selectedOption={makeFilter ? { label: makeFilter, value: makeFilter } : undefined}
       />
     </div>
 
     <div class="w-full">
-      <label class="text-sm text-black dark:text-white" for="search-camera-model">Model</label>
       <Combobox
-        id="search-camera-model"
-        options={toComboBoxOptions(models)}
-        selectedOption={modelFilter ? { label: modelFilter, value: modelFilter } : undefined}
+        id="camera-model"
+        label="Model"
         on:select={({ detail }) => (filters.model = detail?.value)}
+        options={toComboBoxOptions(models)}
         placeholder="Search camera model..."
+        selectedOption={modelFilter ? { label: modelFilter, value: modelFilter } : undefined}
       />
     </div>
   </div>
diff --git a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte
index ac412c5138..fdaabe0f75 100644
--- a/web/src/lib/components/shared-components/search-bar/search-location-section.svelte
+++ b/web/src/lib/components/shared-components/search-bar/search-location-section.svelte
@@ -62,35 +62,35 @@
 
   <div class="grid grid-cols-[repeat(auto-fit,minmax(10rem,1fr))] gap-5 mt-1">
     <div class="w-full">
-      <label class="text-sm text-black dark:text-white" for="search-place-country">Country</label>
       <Combobox
-        id="search-place-country"
-        options={toComboBoxOptions(countries)}
-        selectedOption={filters.country ? { label: filters.country, value: filters.country } : undefined}
+        id="location-country"
+        label="Country"
         on:select={({ detail }) => (filters.country = detail?.value)}
+        options={toComboBoxOptions(countries)}
         placeholder="Search country..."
+        selectedOption={filters.country ? { label: filters.country, value: filters.country } : undefined}
       />
     </div>
 
     <div class="w-full">
-      <label class="text-sm text-black dark:text-white" for="search-place-state">State</label>
       <Combobox
-        id="search-place-state"
-        options={toComboBoxOptions(states)}
-        selectedOption={filters.state ? { label: filters.state, value: filters.state } : undefined}
+        id="location-state"
+        label="State"
         on:select={({ detail }) => (filters.state = detail?.value)}
+        options={toComboBoxOptions(states)}
         placeholder="Search state..."
+        selectedOption={filters.state ? { label: filters.state, value: filters.state } : undefined}
       />
     </div>
 
     <div class="w-full">
-      <label class="text-sm text-black dark:text-white" for="search-place-city">City</label>
       <Combobox
-        id="search-place-city"
-        options={toComboBoxOptions(cities)}
-        selectedOption={filters.city ? { label: filters.city, value: filters.city } : undefined}
+        id="location-city"
+        label="City"
         on:select={({ detail }) => (filters.city = detail?.value)}
+        options={toComboBoxOptions(cities)}
         placeholder="Search city..."
+        selectedOption={filters.city ? { label: filters.city, value: filters.city } : undefined}
       />
     </div>
   </div>
diff --git a/web/src/lib/components/shared-components/settings/setting-combobox.svelte b/web/src/lib/components/shared-components/settings/setting-combobox.svelte
index 7f3dc1906e..ee396935c6 100644
--- a/web/src/lib/components/shared-components/settings/setting-combobox.svelte
+++ b/web/src/lib/components/shared-components/settings/setting-combobox.svelte
@@ -3,6 +3,7 @@
   import { fly } from 'svelte/transition';
   import Combobox, { type ComboBoxOption } from '$lib/components/shared-components/combobox.svelte';
 
+  export let id: string;
   export let title: string;
   export let comboboxPlaceholder: string;
   export let subtitle = '';
@@ -32,6 +33,9 @@
   </div>
   <div class="flex items-center">
     <Combobox
+      {id}
+      label={title}
+      hideLabel={true}
       {selectedOption}
       {options}
       placeholder={comboboxPlaceholder}
diff --git a/web/src/lib/components/user-settings-page/app-settings.svelte b/web/src/lib/components/user-settings-page/app-settings.svelte
index 1d14e5776c..fc7771ee58 100644
--- a/web/src/lib/components/user-settings-page/app-settings.svelte
+++ b/web/src/lib/components/user-settings-page/app-settings.svelte
@@ -86,6 +86,7 @@
       {#if $locale !== undefined}
         <div class="ml-4">
           <SettingCombobox
+            id="custom-locale"
             comboboxPlaceholder="Searching locales..."
             {selectedOption}
             options={getAllLanguages()}