From 4e8dd9281fadb2b5e65b7ab754957eb6c1ea1449 Mon Sep 17 00:00:00 2001
From: lugnicca <lugnicca@gmail.com>
Date: Fri, 22 Aug 2025 03:12:58 +0200
Subject: [PATCH] refactor: simplify event handling and fix test setup in
 ModelCombobox

---
 web-app/src/containers/ModelCombobox.tsx      | 12 ++----
 .../__tests__/ModelCombobox.test.tsx          | 37 +++++++++++--------
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/web-app/src/containers/ModelCombobox.tsx b/web-app/src/containers/ModelCombobox.tsx
index 80fe0157c..ae1af0900 100644
--- a/web-app/src/containers/ModelCombobox.tsx
+++ b/web-app/src/containers/ModelCombobox.tsx
@@ -61,7 +61,7 @@ const ErrorSection = ({ error, onRefresh, t }: { error: string; onRefresh?: () =
           size="sm"
           onClick={(e) => {
             e.stopPropagation()
-            onRefresh?.()
+            onRefresh()
           }}
           className="h-6 w-6 p-0 no-underline hover:bg-main-view-fg/10 text-main-view-fg"
           aria-label="Refresh models"
@@ -171,16 +171,10 @@ function useKeyboardNavigation(
     switch (e.key) {
       case 'ArrowDown':
         e.preventDefault()
-        if (keyRepeatTimeoutRef.current) {
-          clearTimeout(keyRepeatTimeoutRef.current)
-        }
         setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev < filteredModels.length - 1 ? prev + 1 : 0))
         break
       case 'ArrowUp':
         e.preventDefault()
-        if (keyRepeatTimeoutRef.current) {
-          clearTimeout(keyRepeatTimeoutRef.current)
-        }
         setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev > 0 ? prev - 1 : filteredModels.length - 1))
         break
       case 'Enter':
@@ -195,9 +189,11 @@ function useKeyboardNavigation(
         setHighlightedIndex(-1)
         break
       case 'PageUp':
+        e.preventDefault()
         setHighlightedIndex(0)
         break
       case 'PageDown':
+        e.preventDefault()
         setHighlightedIndex(filteredModels.length - 1)
         break
     }
@@ -394,8 +390,6 @@ export function ModelCombobox({
             }}
             data-dropdown="model-combobox"
             onPointerDown={(e) => e.stopPropagation()}
-            onClick={(e) => e.stopPropagation()}
-            onMouseDown={(e) => e.stopPropagation()}
             onWheel={(e) => e.stopPropagation()}
           >
             {/* Error state */}
diff --git a/web-app/src/containers/__tests__/ModelCombobox.test.tsx b/web-app/src/containers/__tests__/ModelCombobox.test.tsx
index 29496a6aa..1b91c3712 100644
--- a/web-app/src/containers/__tests__/ModelCombobox.test.tsx
+++ b/web-app/src/containers/__tests__/ModelCombobox.test.tsx
@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { describe, it, expect, vi, beforeEach, beforeAll, afterAll } from 'vitest'
 import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import '@testing-library/jest-dom/vitest'
@@ -28,10 +28,11 @@ describe('ModelCombobox', () => {
     models: ['gpt-3.5-turbo', 'gpt-4', 'claude-3-haiku'],
   }
 
-  beforeEach(() => {
-    vi.clearAllMocks()
+  let bcrSpy: ReturnType<typeof vi.spyOn>
+  let scrollSpy: ReturnType<typeof vi.spyOn>
 
-    Element.prototype.getBoundingClientRect = vi.fn(() => ({
+  beforeAll(() => {
+    const mockRect = {
       width: 300,
       height: 40,
       top: 100,
@@ -41,9 +42,22 @@ describe('ModelCombobox', () => {
       x: 50,
       y: 100,
       toJSON: () => {},
-    }))
-    
-    Element.prototype.scrollIntoView = vi.fn()
+    } as unknown as DOMRect
+
+    bcrSpy = vi
+      .spyOn(Element.prototype as any, 'getBoundingClientRect')
+      .mockReturnValue(mockRect)
+
+    Element.prototype.scrollIntoView = () => {}
+  })
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  afterAll(() => {
+    bcrSpy?.mockRestore()
+    scrollSpy?.mockRestore()
   })
 
   it('renders input field with default placeholder', () => {
@@ -369,15 +383,6 @@ describe('ModelCombobox', () => {
     expect(localMockOnChange).toHaveBeenCalledWith('gpt')
   })
 
-  it('updates input value when typing', () => {
-    render(<ModelCombobox {...defaultProps} />)
-
-    const input = screen.getByRole('textbox')
-    fireEvent.change(input, { target: { value: 'gpt-4' } })
-
-    expect(input).toHaveValue('gpt-4')
-  })
-
   it('displays error message in dropdown', async () => {
     const user = userEvent.setup()
     render(<ModelCombobox {...defaultProps} error="Network connection failed" />)