]> BookStack Code Mirror - bookstack/commitdiff
Fixed click issue with tag suggestions in safari
authorDan Brown <redacted>
Fri, 7 Apr 2023 16:47:46 +0000 (17:47 +0100)
committerDan Brown <redacted>
Fri, 7 Apr 2023 16:50:57 +0000 (17:50 +0100)
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
app/Http/Controllers/TagController.php
resources/js/components/auto-suggest.js

index cece30de003b3a48b0d72f2d0e30b28e7dfa828e..13d1d957e962f21237e80451fdcffe7cfa448175 100644 (file)
@@ -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) {
index 6c2876043c4fa78e24c959bf1ca00442183d5b58..00aaf2b7899f775b9db43915b2b50659f7f3ba93 100644 (file)
@@ -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
+    ) {
     }
 
     /**
index b4e6c5957879b759c7e9a358fcd5b528c7673b22..89a507b9094208df3dcb6fbc056130212da0dca7 100644 (file)
@@ -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));
         }
     }
Morty Proxy This is a proxified and sanitized view of the page, visit original site.