From e722ee4268d64fd3c6699bec5291b0413c4e9099 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 7 Apr 2023 17:47:46 +0100
Subject: [PATCH] Fixed click issue with tag suggestions in safari

Updated selectable elements to be divs instead of buttons since Safari
akwardly does not focus on buttons on click.
Also standardised keyboard handling to our standard nav class.
Also addressed empty tag values showing in results.
For #4139
---
 app/Actions/TagRepo.php                 |  9 ++--
 app/Http/Controllers/TagController.php  |  8 ++-
 resources/js/components/auto-suggest.js | 68 +++++++++----------------
 3 files changed, 30 insertions(+), 55 deletions(-)

diff --git a/app/Actions/TagRepo.php b/app/Actions/TagRepo.php
index cece30de0..13d1d957e 100644
--- a/app/Actions/TagRepo.php
+++ b/app/Actions/TagRepo.php
@@ -11,11 +11,9 @@ use Illuminate\Support\Facades\DB;
 
 class TagRepo
 {
-    protected PermissionApplicator $permissions;
-
-    public function __construct(PermissionApplicator $permissions)
-    {
-        $this->permissions = $permissions;
+    public function __construct(
+        protected PermissionApplicator $permissions
+    ) {
     }
 
     /**
@@ -90,6 +88,7 @@ class TagRepo
     {
         $query = Tag::query()
             ->select('*', DB::raw('count(*) as count'))
+            ->where('value', '!=', '')
             ->groupBy('value');
 
         if ($searchTerm) {
diff --git a/app/Http/Controllers/TagController.php b/app/Http/Controllers/TagController.php
index 6c2876043..00aaf2b78 100644
--- a/app/Http/Controllers/TagController.php
+++ b/app/Http/Controllers/TagController.php
@@ -8,11 +8,9 @@ use Illuminate\Http\Request;
 
 class TagController extends Controller
 {
-    protected TagRepo $tagRepo;
-
-    public function __construct(TagRepo $tagRepo)
-    {
-        $this->tagRepo = $tagRepo;
+    public function __construct(
+        protected TagRepo $tagRepo
+    ) {
     }
 
     /**
diff --git a/resources/js/components/auto-suggest.js b/resources/js/components/auto-suggest.js
index b4e6c5957..89a507b90 100644
--- a/resources/js/components/auto-suggest.js
+++ b/resources/js/components/auto-suggest.js
@@ -1,6 +1,7 @@
 import {escapeHtml} from "../services/util";
 import {onChildEvent} from "../services/dom";
 import {Component} from "./component";
+import {KeyboardNavigationHandler} from "../services/keyboard-navigation";
 
 const ajaxCache = {};
 
@@ -21,26 +22,31 @@ export class AutoSuggest extends Component {
     }
 
     setupListeners() {
+        const navHandler = new KeyboardNavigationHandler(
+            this.list,
+            event => {
+                this.input.focus();
+                setTimeout(() => this.hideSuggestions(), 1);
+            },
+            event => {
+                event.preventDefault();
+                this.selectSuggestion(event.target.textContent);
+            },
+        );
+        navHandler.shareHandlingToEl(this.input);
+
+        onChildEvent(this.list, '.text-item', 'click', (event, el) => {
+            this.selectSuggestion(el.textContent);
+        });
+
         this.input.addEventListener('input', this.requestSuggestions.bind(this));
         this.input.addEventListener('focus', this.requestSuggestions.bind(this));
+        this.input.addEventListener('blur', this.hideSuggestionsIfFocusedLost.bind(this));
         this.input.addEventListener('keydown', event => {
             if (event.key === 'Tab') {
                 this.hideSuggestions();
             }
         });
-
-        this.input.addEventListener('blur', this.hideSuggestionsIfFocusedLost.bind(this));
-        this.container.addEventListener('keydown', this.containerKeyDown.bind(this));
-
-        onChildEvent(this.list, 'button', 'click', (event, el) => {
-            this.selectSuggestion(el.textContent);
-        });
-        onChildEvent(this.list, 'button', 'keydown', (event, el) => {
-            if (event.key === 'Enter') {
-                this.selectSuggestion(el.textContent);
-            }
-        });
-
     }
 
     selectSuggestion(value) {
@@ -52,36 +58,6 @@ export class AutoSuggest extends Component {
         this.hideSuggestions();
     }
 
-    containerKeyDown(event) {
-        if (event.key === 'Enter') event.preventDefault();
-        if (this.list.classList.contains('hidden')) return;
-
-        // Down arrow
-        if (event.key === 'ArrowDown') {
-            this.moveFocus(true);
-            event.preventDefault();
-        }
-        // Up Arrow
-        else if (event.key === 'ArrowUp') {
-            this.moveFocus(false);
-            event.preventDefault();
-        }
-        // Escape key
-        else if (event.key === 'Escape') {
-            this.hideSuggestions();
-            event.preventDefault();
-        }
-    }
-
-    moveFocus(forward = true) {
-        const focusables = Array.from(this.container.querySelectorAll('input,button'));
-        const index = focusables.indexOf(document.activeElement);
-        const newFocus = focusables[index + (forward ? 1 : -1)];
-        if (newFocus) {
-            newFocus.focus()
-        }
-    }
-
     async requestSuggestions() {
         if (Date.now() - this.lastPopulated < 50) {
             return;
@@ -132,9 +108,11 @@ export class AutoSuggest extends Component {
             return this.hideSuggestions();
         }
 
-        this.list.innerHTML = suggestions.map(value => `<li><button type="button" class="text-item">${escapeHtml(value)}</button></li>`).join('');
+        // This used to use <button>s but was changed to div elements since Safari would not focus on buttons
+        // on which causes a range of other complexities related to focus handling.
+        this.list.innerHTML = suggestions.map(value => `<li><div tabindex="-1" class="text-item">${escapeHtml(value)}</div></li>`).join('');
         this.list.style.display = 'block';
-        for (const button of this.list.querySelectorAll('button')) {
+        for (const button of this.list.querySelectorAll('.text-item')) {
             button.addEventListener('blur', this.hideSuggestionsIfFocusedLost.bind(this));
         }
     }