From 6a3269f9c20c13a8f45dc45fec701b0a207476bd Mon Sep 17 00:00:00 2001
From: ben-basten <45583362+ben-basten@users.noreply.github.com>
Date: Sun, 17 Mar 2024 09:00:22 -0400
Subject: [PATCH] fix: replace crypto.randomUUID, fix border UI bug, simpler
 focus handling

---
 .../shared-components/change-date.svelte      |  8 ++++-
 .../shared-components/combobox.svelte         | 33 +++++++------------
 .../search-bar/search-camera-section.svelte   | 10 +++---
 .../search-bar/search-location-section.svelte | 15 +++++----
 .../settings/setting-combobox.svelte          |  2 ++
 .../user-settings-page/app-settings.svelte    |  1 +
 web/src/lib/utils/shortcut.ts                 |  7 ++--
 7 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/web/src/lib/components/shared-components/change-date.svelte b/web/src/lib/components/shared-components/change-date.svelte
index 2f906b897aee7..e0d51d76c0fcb 100644
--- a/web/src/lib/components/shared-components/change-date.svelte
+++ b/web/src/lib/components/shared-components/change-date.svelte
@@ -83,7 +83,13 @@
         />
       </div>
       <div class="flex flex-col w-full mt-2">
-        <Combobox bind:selectedOption label="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 53eeda4c2e893..b2f2c1db08e8f 100644
--- a/web/src/lib/components/shared-components/combobox.svelte
+++ b/web/src/lib/components/shared-components/combobox.svelte
@@ -19,6 +19,10 @@
   import { shortcuts } from '$lib/utils/shortcut';
   import { onMount, onDestroy } from 'svelte';
 
+  /**
+   * Unique identifier for the combobox.
+   */
+  export let id: string;
   export let label: string;
   export let hideLabel = false;
   export let options: ComboBoxOption[] = [];
@@ -37,8 +41,8 @@
   let selectedIndex: number | undefined;
   let comboboxRef: HTMLElement;
   let optionRefs: HTMLElement[] = [];
-  const inputId = `combobox-${crypto.randomUUID()}`;
-  const listboxId = `listbox-${crypto.randomUUID()}`;
+  const inputId = `combobox-${id}`;
+  const listboxId = `listbox-${id}`;
 
   $: filteredOptions = options.filter((option) => option.label.toLowerCase().includes(searchQuery.toLowerCase()));
 
@@ -75,20 +79,6 @@
     selectedIndex = undefined;
   };
 
-  const moveFocus = (event: KeyboardEvent) => {
-    // move focus to the next focusable element
-    const focusableElements = document.querySelectorAll(
-      'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])',
-    ) as NodeListOf<HTMLElement>;
-    const focusableElementIndex = Array.prototype.indexOf.call(focusableElements, event.target);
-    // focus next element
-    if (event.shiftKey) {
-      focusableElements[focusableElementIndex - 1]?.focus();
-    } else {
-      focusableElements[focusableElementIndex + 1]?.focus();
-    }
-  };
-
   const incrementSelectedIndex = async (increment: number) => {
     if (filteredOptions.length === 0) {
       selectedIndex = 0;
@@ -161,16 +151,16 @@
       use:shortcuts={[
         {
           shortcut: { key: 'Tab' },
-          onShortcut: (event) => {
+          preventDefault: false,
+          onShortcut: () => {
             deactivate();
-            moveFocus(event);
           },
         },
         {
           shortcut: { key: 'Tab', shift: true },
-          onShortcut: (event) => {
+          preventDefault: false,
+          onShortcut: () => {
             deactivate();
-            moveFocus(event);
           },
         },
         {
@@ -230,7 +220,8 @@
     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 r unded-b-lg border border-t-0 border-gray-300 dark:border-gray-900 rounded-b-xl z-10"
+    class="absolute text-left text-sm w-full max-h-64 overflow-y-auto bg-white dark:bg-gray-800 r unded-b-lg border-t-0 border-gray-300 dark:border-gray-900 rounded-b-xl z-10"
+    class:border={isOpen}
     tabindex="-1"
   >
     {#if isOpen}
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 b147e1f102d08..7acac54d8ed94 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
@@ -41,21 +41,23 @@
   <div class="grid grid-cols-[repeat(auto-fit,minmax(10rem,1fr))] gap-5 mt-1">
     <div class="w-full">
       <Combobox
+        id="camera-make"
         label="Make"
-        options={toComboBoxOptions(makes)}
-        selectedOption={makeFilter ? { label: makeFilter, value: makeFilter } : undefined}
         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">
       <Combobox
+        id="camera-model"
         label="Model"
-        options={toComboBoxOptions(models)}
-        selectedOption={modelFilter ? { label: modelFilter, value: modelFilter } : undefined}
         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 3b7bda6b2896f..fdaabe0f75785 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
@@ -63,31 +63,34 @@
   <div class="grid grid-cols-[repeat(auto-fit,minmax(10rem,1fr))] gap-5 mt-1">
     <div class="w-full">
       <Combobox
+        id="location-country"
         label="Country"
-        options={toComboBoxOptions(countries)}
-        selectedOption={filters.country ? { label: filters.country, value: filters.country } : undefined}
         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">
       <Combobox
+        id="location-state"
         label="State"
-        options={toComboBoxOptions(states)}
-        selectedOption={filters.state ? { label: filters.state, value: filters.state } : undefined}
         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">
       <Combobox
+        id="location-city"
         label="City"
-        options={toComboBoxOptions(cities)}
-        selectedOption={filters.city ? { label: filters.city, value: filters.city } : undefined}
         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 f6c1aa81392c8..ee396935c6102 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,7 @@
   </div>
   <div class="flex items-center">
     <Combobox
+      {id}
       label={title}
       hideLabel={true}
       {selectedOption}
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 1d14e5776c5c8..fc7771ee589db 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()}
diff --git a/web/src/lib/utils/shortcut.ts b/web/src/lib/utils/shortcut.ts
index 3d80c2c802447..8de1592eda477 100644
--- a/web/src/lib/utils/shortcut.ts
+++ b/web/src/lib/utils/shortcut.ts
@@ -10,6 +10,7 @@ export type Shortcut = {
 
 export type ShortcutOptions<T = HTMLElement> = {
   shortcut: Shortcut;
+  preventDefault?: boolean;
   ignoreInputFields?: boolean;
   onShortcut: (event: KeyboardEvent & { currentTarget: T }) => unknown;
 };
@@ -53,13 +54,15 @@ export const shortcuts = <T extends HTMLElement>(
   function onKeydown(event: KeyboardEvent) {
     const ignoreShortcut = shouldIgnoreShortcut(event);
 
-    for (const { shortcut, onShortcut, ignoreInputFields = true } of options) {
+    for (const { shortcut, onShortcut, ignoreInputFields = true, preventDefault = true } of options) {
       if (ignoreInputFields && ignoreShortcut) {
         continue;
       }
 
       if (matchesShortcut(event, shortcut)) {
-        event.preventDefault();
+        if (preventDefault) {
+          event.preventDefault();
+        }
         onShortcut(event as KeyboardEvent & { currentTarget: T });
         return;
       }