From da08a8744f6fbb07a80a55f2a4320d5c378e8d20 Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Wed, 3 Apr 2024 01:26:29 -0700
Subject: [PATCH] Fix keyup vs down bug

---
 .../global-search.component.html              | 98 +++++++++----------
 .../global-search.component.spec.ts           | 63 ++++++------
 .../global-search/global-search.component.ts  | 73 ++++++++------
 3 files changed, 121 insertions(+), 113 deletions(-)

diff --git a/src-ui/src/app/components/app-frame/global-search/global-search.component.html b/src-ui/src/app/components/app-frame/global-search/global-search.component.html
index 451533fb2..3a1fbd4af 100644
--- a/src-ui/src/app/components/app-frame/global-search/global-search.component.html
+++ b/src-ui/src/app/components/app-frame/global-search/global-search.component.html
@@ -1,65 +1,65 @@
 
 <div ngbDropdown #resultsDropdown="ngbDropdown" (openChange)="onDropdownOpenChange">
-<form class="form-inline position-relative">
-    <i-bs width="1em" height="1em" name="search"></i-bs>
-    <input #searchInput class="form-control form-control-sm" type="text" name="query"
-        autocomplete="off" placeholder="Search" aria-label="Search" i18n-placeholder
-        [(ngModel)]="query" (ngModelChange)="this.queryDebounce.next($event)" (keyup)="searchInputKeyDown($event)" ngbDropdownAnchor>
-    <div class="position-absolute top-50 end-0 translate-middle">
-        <span class="badge d-none d-lg-inline text-muted position-absolute top-50 start-100 translate-middle ms-n4 fw-normal">⌘K</span>
-        @if (loading) {
-            <div class="spinner-border spinner-border-sm text-muted mt-1"></div>
-        }
-    </div>
-
-</form>
-
-<ng-template #resultItemTemplate let-item="item" let-nameProp="nameProp" let-type="type" let-icon="icon" let-date="date">
-    <div #resultItem ngbDropdownItem class="py-2 d-flex align-items-center focus-ring border-0 cursor-pointer" tabindex="-1"
-    (click)="primaryAction(type, item)"
-    (mouseenter)="onItemHover($event)">
-        <i-bs width="1.2em" height="1.2em" name="{{icon}}" class="me-2 text-muted"></i-bs>
-        <div class="text-truncate">
-            {{item[nameProp]}}
-            @if (date) {
-                <small class="small text-muted">{{date | customDate}}</small>
+    <form class="form-inline position-relative">
+        <i-bs width="1em" height="1em" name="search"></i-bs>
+        <input #searchInput class="form-control form-control-sm" type="text" name="query"
+            autocomplete="off" placeholder="Search" aria-label="Search" i18n-placeholder
+            [(ngModel)]="query" (ngModelChange)="this.queryDebounce.next($event)" (keydown)="searchInputKeyDown($event)" ngbDropdownAnchor>
+        <div class="position-absolute top-50 end-0 translate-middle">
+            <span class="badge d-none d-lg-inline text-muted position-absolute top-50 start-100 translate-middle ms-n4 fw-normal">⌘K</span>
+            @if (loading) {
+                <div class="spinner-border spinner-border-sm text-muted mt-1"></div>
             }
         </div>
-        <div class="btn-group ms-auto">
-            <button #primaryButton type="button" class="btn btn-sm btn-outline-primary d-flex"
-            (click)="primaryAction(type, item); $event.stopPropagation()"
-            [disabled]="disablePrimaryButton(type, item)"
-            (mouseenter)="onButtonHover($event)">
-                @if (type === DataType.Document) {
-                    <i-bs width="1em" height="1em" name="pencil"></i-bs>
-                    <span>&nbsp;<ng-container i18n>Open</ng-container></span>
-                } @else if (type === DataType.Workflow || type === DataType.CustomField || type === DataType.Group || type === DataType.User || type === DataType.MailAccount || type === DataType.MailRule) {
-                    <i-bs width="1em" height="1em" name="pencil"></i-bs>
-                    <span>&nbsp;<ng-container i18n>Edit</ng-container></span>
-                } @else {
-                    <i-bs width="1em" height="1em" name="filter"></i-bs>
-                    <span>&nbsp;<ng-container i18n>Filter documents</ng-container></span>
+
+    </form>
+
+    <ng-template #resultItemTemplate let-item="item" let-nameProp="nameProp" let-type="type" let-icon="icon" let-date="date">
+        <div #resultItem ngbDropdownItem class="py-2 d-flex align-items-center focus-ring border-0 cursor-pointer" tabindex="-1"
+        (click)="primaryAction(type, item)"
+        (mouseenter)="onItemHover($event)">
+            <i-bs width="1.2em" height="1.2em" name="{{icon}}" class="me-2 text-muted"></i-bs>
+            <div class="text-truncate">
+                {{item[nameProp]}}
+                @if (date) {
+                    <small class="small text-muted">{{date | customDate}}</small>
                 }
-            </button>
-            @if (type !== DataType.Workflow && type !== DataType.CustomField && type !== DataType.Group && type !== DataType.User && type !== DataType.MailAccount && type !== DataType.MailRule) {
-                <button #secondaryButton type="button" class="btn btn-sm btn-outline-primary d-flex"
-                (click)="secondaryAction(type, item); $event.stopPropagation()"
-                [disabled]="disableSecondaryButton(type, item)"
+            </div>
+            <div class="btn-group ms-auto">
+                <button #primaryButton type="button" class="btn btn-sm btn-outline-primary d-flex"
+                (click)="primaryAction(type, item); $event.stopPropagation()"
+                [disabled]="disablePrimaryButton(type, item)"
                 (mouseenter)="onButtonHover($event)">
                     @if (type === DataType.Document) {
-                        <i-bs width="1em" height="1em" name="download"></i-bs>
-                        <span>&nbsp;<ng-container i18n>Download</ng-container></span>
-                    } @else {
+                        <i-bs width="1em" height="1em" name="pencil"></i-bs>
+                        <span>&nbsp;<ng-container i18n>Open</ng-container></span>
+                    } @else if (type === DataType.Workflow || type === DataType.CustomField || type === DataType.Group || type === DataType.User || type === DataType.MailAccount || type === DataType.MailRule) {
                         <i-bs width="1em" height="1em" name="pencil"></i-bs>
                         <span>&nbsp;<ng-container i18n>Edit</ng-container></span>
+                    } @else {
+                        <i-bs width="1em" height="1em" name="filter"></i-bs>
+                        <span>&nbsp;<ng-container i18n>Filter documents</ng-container></span>
                     }
                 </button>
-            }
+                @if (type !== DataType.Workflow && type !== DataType.CustomField && type !== DataType.Group && type !== DataType.User && type !== DataType.MailAccount && type !== DataType.MailRule) {
+                    <button #secondaryButton type="button" class="btn btn-sm btn-outline-primary d-flex"
+                    (click)="secondaryAction(type, item); $event.stopPropagation()"
+                    [disabled]="disableSecondaryButton(type, item)"
+                    (mouseenter)="onButtonHover($event)">
+                        @if (type === DataType.Document) {
+                            <i-bs width="1em" height="1em" name="download"></i-bs>
+                            <span>&nbsp;<ng-container i18n>Download</ng-container></span>
+                        } @else {
+                            <i-bs width="1em" height="1em" name="pencil"></i-bs>
+                            <span>&nbsp;<ng-container i18n>Edit</ng-container></span>
+                        }
+                    </button>
+                }
+            </div>
         </div>
-    </div>
-</ng-template>
+    </ng-template>
 
-    <div ngbDropdownMenu class="w-100 mh-75 overflow-y-scroll shadow-lg">
+    <div ngbDropdownMenu class="w-100 mh-75 overflow-y-scroll shadow-lg" (keydown)="dropdownKeyDown($event)">
         @if (searchResults?.total === 0) {
             <h6 class="dropdown-header" i18n="@@searchResults.noResults">No results</h6>
         } @else {
diff --git a/src-ui/src/app/components/app-frame/global-search/global-search.component.spec.ts b/src-ui/src/app/components/app-frame/global-search/global-search.component.spec.ts
index eaf5aae87..fed135818 100644
--- a/src-ui/src/app/components/app-frame/global-search/global-search.component.spec.ts
+++ b/src-ui/src/app/components/app-frame/global-search/global-search.component.spec.ts
@@ -169,7 +169,7 @@ describe('GlobalSearchComponent', () => {
       component.primaryButtons.get(1).nativeElement,
       'focus'
     )
-    component.handleKeyboardEvent(
+    component.dropdownKeyDown(
       new KeyboardEvent('keydown', { key: 'ArrowDown' })
     )
     expect(component['currentItemIndex']).toBe(1)
@@ -179,12 +179,12 @@ describe('GlobalSearchComponent', () => {
       component.secondaryButtons.get(1).nativeElement,
       'focus'
     )
-    component.handleKeyboardEvent(
+    component.dropdownKeyDown(
       new KeyboardEvent('keydown', { key: 'ArrowRight' })
     )
     expect(secondaryItemFocusSpy).toHaveBeenCalled()
 
-    component.handleKeyboardEvent(
+    component.dropdownKeyDown(
       new KeyboardEvent('keydown', { key: 'ArrowLeft' })
     )
     expect(firstItemFocusSpy).toHaveBeenCalled()
@@ -193,9 +193,7 @@ describe('GlobalSearchComponent', () => {
       component.primaryButtons.get(0).nativeElement,
       'focus'
     )
-    component.handleKeyboardEvent(
-      new KeyboardEvent('keydown', { key: 'ArrowUp' })
-    )
+    component.dropdownKeyDown(new KeyboardEvent('keydown', { key: 'ArrowUp' }))
     expect(component['currentItemIndex']).toBe(0)
     expect(zeroItemSpy).toHaveBeenCalled()
 
@@ -203,22 +201,42 @@ describe('GlobalSearchComponent', () => {
       component.searchInput.nativeElement,
       'focus'
     )
-    component.handleKeyboardEvent(
-      new KeyboardEvent('keydown', { key: 'ArrowUp' })
-    )
+    component.dropdownKeyDown(new KeyboardEvent('keydown', { key: 'ArrowUp' }))
     expect(component['currentItemIndex']).toBe(-1)
     expect(inputFocusSpy).toHaveBeenCalled()
 
-    component.handleKeyboardEvent(
+    component.dropdownKeyDown(
       new KeyboardEvent('keydown', { key: 'ArrowDown' })
     )
     component['currentItemIndex'] = searchResults.total - 1
     component['setCurrentItem']()
-    component.handleKeyboardEvent(
+    component.dropdownKeyDown(
+      new KeyboardEvent('keydown', { key: 'ArrowDown' })
+    )
+    expect(component['currentItemIndex']).toBe(-1)
+
+    // Search input
+
+    component.searchInputKeyDown(
+      new KeyboardEvent('keydown', { key: 'ArrowUp' })
+    )
+    expect(component['currentItemIndex']).toBe(searchResults.total - 1)
+
+    component.searchInputKeyDown(
       new KeyboardEvent('keydown', { key: 'ArrowDown' })
     )
     expect(component['currentItemIndex']).toBe(0)
-    expect(zeroItemSpy).toHaveBeenCalled()
+
+    component.searchResults = { total: 1 } as any
+    const primaryActionSpy = jest.spyOn(component, 'primaryAction')
+    component.searchInputKeyDown(new KeyboardEvent('keydown', { key: 'Enter' }))
+    expect(primaryActionSpy).toHaveBeenCalled()
+
+    const resetSpy = jest.spyOn(GlobalSearchComponent.prototype as any, 'reset')
+    component.searchInputKeyDown(
+      new KeyboardEvent('keydown', { key: 'Escape' })
+    )
+    expect(resetSpy).toHaveBeenCalled()
   })
 
   it('should search on query debounce', fakeAsync(() => {
@@ -380,27 +398,6 @@ describe('GlobalSearchComponent', () => {
     expect(focusSpy).toHaveBeenCalled()
   })
 
-  it('should handle search input keyboard nav', () => {
-    component.searchResults = searchResults as any
-    component.resultsDropdown.open()
-    fixture.detectChanges()
-    component.searchInputKeyDown(
-      new KeyboardEvent('keydown', { key: 'ArrowDown' })
-    )
-    expect(component['currentItemIndex']).toBe(0)
-
-    component.searchResults = { total: 1 } as any
-    const primaryActionSpy = jest.spyOn(component, 'primaryAction')
-    component.searchInputKeyDown(new KeyboardEvent('keydown', { key: 'Enter' }))
-    expect(primaryActionSpy).toHaveBeenCalled()
-
-    const resetSpy = jest.spyOn(GlobalSearchComponent.prototype as any, 'reset')
-    component.searchInputKeyDown(
-      new KeyboardEvent('keydown', { key: 'Escape' })
-    )
-    expect(resetSpy).toHaveBeenCalled()
-  })
-
   it('should reset on dropdown close', () => {
     const resetSpy = jest.spyOn(GlobalSearchComponent.prototype as any, 'reset')
     component.onDropdownOpenChange(false)
diff --git a/src-ui/src/app/components/app-frame/global-search/global-search.component.ts b/src-ui/src/app/components/app-frame/global-search/global-search.component.ts
index 6550964e6..9ee32c906 100644
--- a/src-ui/src/app/components/app-frame/global-search/global-search.component.ts
+++ b/src-ui/src/app/components/app-frame/global-search/global-search.component.ts
@@ -65,37 +65,6 @@ export class GlobalSearchComponent {
     if (event.key === 'k' && (event.ctrlKey || event.metaKey)) {
       this.searchInput.nativeElement.focus()
     }
-
-    if (
-      this.searchResults &&
-      this.resultsDropdown.isOpen() &&
-      document.activeElement !== this.searchInput.nativeElement
-    ) {
-      if (event.key === 'ArrowDown') {
-        if (this.currentItemIndex < this.searchResults.total - 1) {
-          event.preventDefault()
-          this.currentItemIndex++
-          this.setCurrentItem()
-        } else {
-          event.preventDefault()
-          this.currentItemIndex = 0
-          this.setCurrentItem()
-        }
-      } else if (event.key === 'ArrowUp') {
-        if (this.currentItemIndex > 0) {
-          event.preventDefault()
-          this.currentItemIndex--
-          this.setCurrentItem()
-        } else {
-          this.searchInput.nativeElement.focus()
-          this.currentItemIndex = -1
-        }
-      } else if (event.key === 'ArrowRight') {
-        this.secondaryButtons.get(this.domIndex)?.nativeElement.focus()
-      } else if (event.key === 'ArrowLeft') {
-        this.primaryButtons.get(this.domIndex).nativeElement.focus()
-      }
-    }
   }
 
   constructor(
@@ -277,6 +246,14 @@ export class GlobalSearchComponent {
       event.preventDefault()
       this.currentItemIndex = 0
       this.setCurrentItem()
+    } else if (
+      event.key === 'ArrowUp' &&
+      this.searchResults?.total &&
+      this.resultsDropdown.isOpen()
+    ) {
+      event.preventDefault()
+      this.currentItemIndex = this.searchResults.total - 1
+      this.setCurrentItem()
     } else if (
       event.key === 'Enter' &&
       this.searchResults?.total === 1 &&
@@ -288,6 +265,40 @@ export class GlobalSearchComponent {
     }
   }
 
+  dropdownKeyDown(event: KeyboardEvent) {
+    if (
+      this.searchResults?.total &&
+      this.resultsDropdown.isOpen() &&
+      document.activeElement !== this.searchInput.nativeElement
+    ) {
+      if (event.key === 'ArrowDown') {
+        event.preventDefault()
+        if (this.currentItemIndex < this.searchResults.total - 1) {
+          this.currentItemIndex++
+          this.setCurrentItem()
+        } else {
+          this.searchInput.nativeElement.focus()
+          this.currentItemIndex = -1
+        }
+      } else if (event.key === 'ArrowUp') {
+        event.preventDefault()
+        if (this.currentItemIndex > 0) {
+          this.currentItemIndex--
+          this.setCurrentItem()
+        } else {
+          this.searchInput.nativeElement.focus()
+          this.currentItemIndex = -1
+        }
+      } else if (event.key === 'ArrowRight') {
+        event.preventDefault()
+        this.secondaryButtons.get(this.domIndex)?.nativeElement.focus()
+      } else if (event.key === 'ArrowLeft') {
+        event.preventDefault()
+        this.primaryButtons.get(this.domIndex).nativeElement.focus()
+      }
+    }
+  }
+
   public onDropdownOpenChange(open: boolean) {
     if (!open) {
       this.reset()