From d3fff154d4b432cd142ce997215cd1ad3eb921d2 Mon Sep 17 00:00:00 2001 From: Faisal Amir Date: Tue, 23 Sep 2025 20:17:51 +0700 Subject: [PATCH 1/2] fix: download management ui and double refresh model --- web-app/src/containers/DownloadManegement.tsx | 29 +- web-app/src/containers/LeftPanel.tsx | 24 +- web-app/src/containers/ModelCombobox.tsx | 294 +++++++++++------- 3 files changed, 202 insertions(+), 145 deletions(-) diff --git a/web-app/src/containers/DownloadManegement.tsx b/web-app/src/containers/DownloadManegement.tsx index f91a943d3..92bb3ee85 100644 --- a/web-app/src/containers/DownloadManegement.tsx +++ b/web-app/src/containers/DownloadManegement.tsx @@ -400,20 +400,23 @@ export function DownloadManagement() { className="text-main-view-fg/70 cursor-pointer" title="Cancel download" onClick={() => { - serviceHub.models().abortDownload(download.name).then(() => { - toast.info( - t('common:toast.downloadCancelled.title'), - { - id: 'cancel-download', - description: t( - 'common:toast.downloadCancelled.description' - ), + serviceHub + .models() + .abortDownload(download.name) + .then(() => { + toast.info( + t('common:toast.downloadCancelled.title'), + { + id: 'cancel-download', + description: t( + 'common:toast.downloadCancelled.description' + ), + } + ) + if (downloadProcesses.length === 0) { + setIsPopoverOpen(false) } - ) - if (downloadProcesses.length === 0) { - setIsPopoverOpen(false) - } - }) + }) }} /> diff --git a/web-app/src/containers/LeftPanel.tsx b/web-app/src/containers/LeftPanel.tsx index da596dd4a..1ad0ef560 100644 --- a/web-app/src/containers/LeftPanel.tsx +++ b/web-app/src/containers/LeftPanel.tsx @@ -35,7 +35,7 @@ import { toast } from 'sonner' import { DownloadManagement } from '@/containers/DownloadManegement' import { useSmallScreen } from '@/hooks/useMediaQuery' import { useClickOutside } from '@/hooks/useClickOutside' -import { useDownloadStore } from '@/hooks/useDownloadStore' + import { DeleteAllThreadsDialog } from '@/containers/dialogs' const mainMenus = [ @@ -122,7 +122,7 @@ const LeftPanel = () => { ) { if (currentIsSmallScreen && open) { setLeftPanel(false) - } else if(!open) { + } else if (!open) { setLeftPanel(true) } prevScreenSizeRef.current = currentIsSmallScreen @@ -179,8 +179,6 @@ const LeftPanel = () => { } }, [isSmallScreen, open]) - const { downloads, localDownloadingModels } = useDownloadStore() - return ( <> {/* Backdrop overlay for small screens */} @@ -262,15 +260,8 @@ const LeftPanel = () => { )} -
-
0 || localDownloadingModels.size > 0 - ? 'h-[calc(100%-200px)]' - : 'h-[calc(100%-140px)]' - )} - > +
+
{IS_MACOS && (
{ - +
@@ -468,8 +461,9 @@ const LeftPanel = () => { ) })} -
+ +
diff --git a/web-app/src/containers/ModelCombobox.tsx b/web-app/src/containers/ModelCombobox.tsx index ea5b3d670..5ed8ed14d 100644 --- a/web-app/src/containers/ModelCombobox.tsx +++ b/web-app/src/containers/ModelCombobox.tsx @@ -7,8 +7,15 @@ import { cn } from '@/lib/utils' import { useTranslation } from '@/i18n/react-i18next-compat' // Hook for the dropdown position -function useDropdownPosition(open: boolean, containerRef: React.RefObject) { - const [dropdownPosition, setDropdownPosition] = useState({ top: 0, left: 0, width: 0 }) +function useDropdownPosition( + open: boolean, + containerRef: React.RefObject +) { + const [dropdownPosition, setDropdownPosition] = useState({ + top: 0, + left: 0, + width: 0, + }) const updateDropdownPosition = useCallback(() => { if (containerRef.current) { @@ -51,10 +58,18 @@ function useDropdownPosition(open: boolean, containerRef: React.RefObject string }) => ( +const ErrorSection = ({ + error, + t, +}: { + error: string + t: (key: string) => string +}) => (
- {t('common:failedToLoadModels')} + + {t('common:failedToLoadModels')} +
{error}
@@ -67,12 +82,20 @@ const LoadingSection = ({ t }: { t: (key: string) => string }) => (
) -const EmptySection = ({ inputValue, t }: { inputValue: string; t: (key: string, options?: Record) => string }) => ( +const EmptySection = ({ + inputValue, + t, +}: { + inputValue: string + t: (key: string, options?: Record) => string +}) => (
{inputValue.trim() ? ( - {t('common:noModelsFoundFor', { searchValue: inputValue })} + + {t('common:noModelsFoundFor', { searchValue: inputValue })} + ) : ( {t('common:noModels')} )} @@ -86,7 +109,7 @@ const ModelsList = ({ value, highlightedIndex, onModelSelect, - onHighlight + onHighlight, }: { filteredModels: string[] value: string @@ -127,67 +150,95 @@ function useKeyboardNavigation( onModelSelect: (model: string) => void, dropdownRef: React.RefObject ) { - // Scroll to the highlighted element useEffect(() => { if (highlightedIndex >= 0 && dropdownRef.current) { requestAnimationFrame(() => { - const modelElements = dropdownRef.current?.querySelectorAll('[data-model]') - const highlightedElement = modelElements?.[highlightedIndex] as HTMLElement + const modelElements = + dropdownRef.current?.querySelectorAll('[data-model]') + const highlightedElement = modelElements?.[ + highlightedIndex + ] as HTMLElement if (highlightedElement) { highlightedElement.scrollIntoView({ block: 'nearest', - behavior: 'auto' + behavior: 'auto', }) } }) } }, [highlightedIndex, dropdownRef]) - const handleKeyDown = useCallback((e: React.KeyboardEvent) => { - // Open the dropdown with the arrows if closed - if (!open && (e.key === 'ArrowDown' || e.key === 'ArrowUp')) { - if (models.length > 0) { - e.preventDefault() - setOpen(true) - setHighlightedIndex(0) - } - return - } - - if (!open) return - - switch (e.key) { - case 'ArrowDown': - e.preventDefault() - setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev < filteredModels.length - 1 ? prev + 1 : 0)) - break - case 'ArrowUp': - e.preventDefault() - setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev > 0 ? prev - 1 : filteredModels.length - 1)) - break - case 'Enter': - e.preventDefault() - if (highlightedIndex >= 0 && highlightedIndex < filteredModels.length) { - onModelSelect(filteredModels[highlightedIndex]) + const handleKeyDown = useCallback( + (e: React.KeyboardEvent) => { + // Open the dropdown with the arrows if closed + if (!open && (e.key === 'ArrowDown' || e.key === 'ArrowUp')) { + if (models.length > 0) { + e.preventDefault() + setOpen(true) + setHighlightedIndex(0) } - break - case 'Escape': - e.preventDefault() - e.stopPropagation() - setOpen(false) - setHighlightedIndex(-1) - break - case 'PageUp': - e.preventDefault() - setHighlightedIndex(0) - break - case 'PageDown': - e.preventDefault() - setHighlightedIndex(filteredModels.length - 1) - break - } - }, [open, setOpen, models.length, filteredModels, highlightedIndex, setHighlightedIndex, onModelSelect]) + return + } + + if (!open) return + + switch (e.key) { + case 'ArrowDown': + e.preventDefault() + setHighlightedIndex((prev: number) => + filteredModels.length === 0 + ? 0 + : prev < filteredModels.length - 1 + ? prev + 1 + : 0 + ) + break + case 'ArrowUp': + e.preventDefault() + setHighlightedIndex((prev: number) => + filteredModels.length === 0 + ? 0 + : prev > 0 + ? prev - 1 + : filteredModels.length - 1 + ) + break + case 'Enter': + e.preventDefault() + if ( + highlightedIndex >= 0 && + highlightedIndex < filteredModels.length + ) { + onModelSelect(filteredModels[highlightedIndex]) + } + break + case 'Escape': + e.preventDefault() + e.stopPropagation() + setOpen(false) + setHighlightedIndex(-1) + break + case 'PageUp': + e.preventDefault() + setHighlightedIndex(0) + break + case 'PageDown': + e.preventDefault() + setHighlightedIndex(filteredModels.length - 1) + break + } + }, + [ + open, + setOpen, + models.length, + filteredModels, + highlightedIndex, + setHighlightedIndex, + onModelSelect, + ] + ) return { handleKeyDown } } @@ -266,13 +317,18 @@ export function ModelCombobox({ } const events = ['mousedown', 'touchstart'] - events.forEach(eventType => { - document.addEventListener(eventType, handleClickOutside, { capture: true, passive: true }) + events.forEach((eventType) => { + document.addEventListener(eventType, handleClickOutside, { + capture: true, + passive: true, + }) }) return () => { - events.forEach(eventType => { - document.removeEventListener(eventType, handleClickOutside, { capture: true }) + events.forEach((eventType) => { + document.removeEventListener(eventType, handleClickOutside, { + capture: true, + }) }) } }, [open]) @@ -286,26 +342,32 @@ export function ModelCombobox({ }, []) // Handler for the input change - const handleInputChange = useCallback((newValue: string) => { - setInputValue(newValue) - onChange(newValue) + const handleInputChange = useCallback( + (newValue: string) => { + setInputValue(newValue) + onChange(newValue) - // Open the dropdown if the user types and there are models - if (newValue.trim() && models.length > 0) { - setOpen(true) - } else { - setOpen(false) - } - }, [onChange, models.length]) + // Open the dropdown if the user types and there are models + if (newValue.trim() && models.length > 0) { + setOpen(true) + } else { + setOpen(false) + } + }, + [onChange, models.length] + ) // Handler for the model selection - const handleModelSelect = useCallback((model: string) => { - setInputValue(model) - onChange(model) - setOpen(false) - setHighlightedIndex(-1) - inputRef.current?.focus() - }, [onChange]) + const handleModelSelect = useCallback( + (model: string) => { + setInputValue(model) + onChange(model) + setOpen(false) + setHighlightedIndex(-1) + inputRef.current?.focus() + }, + [onChange] + ) // Hook for the keyboard navigation const { handleKeyDown } = useKeyboardNavigation( @@ -376,54 +438,52 @@ export function ModelCombobox({ onClick={handleDropdownToggle} className="h-6 w-6 p-0 no-underline hover:bg-main-view-fg/10" > - {loading ? ( - - ) : ( - - )} +
{/* Custom dropdown rendered as portal */} - {open && dropdownPosition.width > 0 && createPortal( -
e.stopPropagation()} - onWheel={(e) => e.stopPropagation()} - > - {/* Error state */} - {error && } + {open && + dropdownPosition.width > 0 && + createPortal( +
e.stopPropagation()} + onWheel={(e) => e.stopPropagation()} + > + {/* Error state */} + {error && } - {/* Loading state */} - {loading && } + {/* Loading state */} + {loading && } - {/* Models list */} - {!loading && !error && ( - filteredModels.length === 0 ? ( - - ) : ( - - ) - )} -
, - document.body - )} + {/* Models list */} + {!loading && + !error && + (filteredModels.length === 0 ? ( + + ) : ( + + ))} +
, + document.body + )}
) From 3a5580c725df6ac548baa64f9a46db995f1498d2 Mon Sep 17 00:00:00 2001 From: Faisal Amir Date: Tue, 23 Sep 2025 20:25:05 +0700 Subject: [PATCH 2/2] chore: update test case --- .../__tests__/ModelCombobox.test.tsx | 125 +++++++++++------- 1 file changed, 79 insertions(+), 46 deletions(-) diff --git a/web-app/src/containers/__tests__/ModelCombobox.test.tsx b/web-app/src/containers/__tests__/ModelCombobox.test.tsx index 38f9b97c8..1c0815549 100644 --- a/web-app/src/containers/__tests__/ModelCombobox.test.tsx +++ b/web-app/src/containers/__tests__/ModelCombobox.test.tsx @@ -1,4 +1,12 @@ -import { describe, it, expect, vi, beforeEach, beforeAll, afterAll } 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' @@ -11,7 +19,8 @@ vi.mock('@/i18n/react-i18next-compat', () => ({ t: (key: string, options?: Record) => { if (key === 'common:failedToLoadModels') return 'Failed to load models' if (key === 'common:loading') return 'Loading' - if (key === 'common:noModelsFoundFor') return `No models found for "${options?.searchValue}"` + if (key === 'common:noModelsFoundFor') + return `No models found for "${options?.searchValue}"` if (key === 'common:noModels') return 'No models available' return key }, @@ -21,7 +30,7 @@ vi.mock('@/i18n/react-i18next-compat', () => ({ describe('ModelCombobox', () => { const mockOnChange = vi.fn() const mockOnRefresh = vi.fn() - + const defaultProps = { value: '', onChange: mockOnChange, @@ -64,7 +73,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByRole('textbox') expect(input).toBeInTheDocument() expect(input).toHaveAttribute('placeholder', 'Type or select a model...') @@ -74,7 +83,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByRole('textbox') expect(input).toHaveAttribute('placeholder', 'Choose a model') }) @@ -83,7 +92,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const button = screen.getByRole('button') expect(button).toBeInTheDocument() }) @@ -92,7 +101,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByDisplayValue('gpt-4') expect(input).toBeInTheDocument() }) @@ -110,7 +119,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByRole('textbox') const button = screen.getByRole('button') @@ -118,27 +127,19 @@ describe('ModelCombobox', () => { expect(button).toBeDisabled() }) - it('shows loading spinner in trigger button', () => { - act(() => { - render() - }) - - const button = screen.getByRole('button') - const spinner = button.querySelector('.animate-spin') - expect(spinner).toBeInTheDocument() - }) - it('shows loading section when dropdown is opened during loading', async () => { const user = userEvent.setup() render() - + // Click input to trigger dropdown opening const input = screen.getByRole('textbox') await user.click(input) - + // Wait for dropdown to appear and check loading section await waitFor(() => { - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() expect(screen.getByText('Loading')).toBeInTheDocument() }) @@ -179,7 +180,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByRole('textbox') expect(input).toBeInTheDocument() }) @@ -188,7 +189,7 @@ describe('ModelCombobox', () => { act(() => { render() }) - + const input = screen.getByRole('textbox') expect(input).toBeInTheDocument() }) @@ -259,7 +260,7 @@ describe('ModelCombobox', () => { /> ) }) - + const input = screen.getByRole('textbox') expect(input).toBeInTheDocument() expect(input).toBeDisabled() @@ -273,7 +274,9 @@ describe('ModelCombobox', () => { await user.click(button) await waitFor(() => { - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() }) }) @@ -287,7 +290,9 @@ describe('ModelCombobox', () => { expect(input).toHaveFocus() await waitFor(() => { - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() }) }) @@ -313,9 +318,11 @@ describe('ModelCombobox', () => { await waitFor(() => { // Dropdown should be open - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() - + // Should show GPT models expect(screen.getByText('gpt-3.5-turbo')).toBeInTheDocument() expect(screen.getByText('gpt-4')).toBeInTheDocument() @@ -344,10 +351,14 @@ describe('ModelCombobox', () => { await waitFor(() => { // Dropdown should be open - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() // Should show empty state message - expect(screen.getByText('No models found for "nonexistent"')).toBeInTheDocument() + expect( + screen.getByText('No models found for "nonexistent"') + ).toBeInTheDocument() }) }) @@ -358,12 +369,12 @@ describe('ModelCombobox', () => { const input = screen.getByRole('textbox') await user.click(input) - + await waitFor(() => { const modelOption = screen.getByText('gpt-4') expect(modelOption).toBeInTheDocument() }) - + const modelOption = screen.getByText('gpt-4') await user.click(modelOption) @@ -385,7 +396,9 @@ describe('ModelCombobox', () => { it('displays error message in dropdown', async () => { const user = userEvent.setup() - render() + render( + + ) const input = screen.getByRole('textbox') // Click input to open dropdown @@ -393,7 +406,9 @@ describe('ModelCombobox', () => { await waitFor(() => { // Dropdown should be open - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() // Error messages should be displayed expect(screen.getByText('Failed to load models')).toBeInTheDocument() @@ -404,7 +419,13 @@ describe('ModelCombobox', () => { it('calls onRefresh when refresh button is clicked', async () => { const user = userEvent.setup() const localMockOnRefresh = vi.fn() - render() + render( + + ) const input = screen.getByRole('textbox') // Click input to open dropdown @@ -412,13 +433,19 @@ describe('ModelCombobox', () => { await waitFor(() => { // Dropdown should be open with error section - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() - const refreshButton = document.querySelector('[aria-label="Refresh models"]') + const refreshButton = document.querySelector( + '[aria-label="Refresh models"]' + ) expect(refreshButton).toBeInTheDocument() }) - const refreshButton = document.querySelector('[aria-label="Refresh models"]') + const refreshButton = document.querySelector( + '[aria-label="Refresh models"]' + ) if (refreshButton) { await user.click(refreshButton) expect(localMockOnRefresh).toHaveBeenCalledTimes(1) @@ -435,7 +462,9 @@ describe('ModelCombobox', () => { expect(input).toHaveFocus() await waitFor(() => { - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() }) }) @@ -446,16 +475,18 @@ describe('ModelCombobox', () => { const input = screen.getByRole('textbox') input.focus() - + // ArrowDown should open dropdown await user.keyboard('{ArrowDown}') - + await waitFor(() => { // Dropdown should be open - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() }) - + // Navigate to second item await user.keyboard('{ArrowDown}') @@ -474,13 +505,15 @@ describe('ModelCombobox', () => { const input = screen.getByRole('textbox') // Type 'gpt' to open dropdown and filter models await user.type(input, 'gpt') - + await waitFor(() => { // Dropdown should be open with filtered models - const dropdown = document.querySelector('[data-dropdown="model-combobox"]') + const dropdown = document.querySelector( + '[data-dropdown="model-combobox"]' + ) expect(dropdown).toBeInTheDocument() }) - + // Navigate to highlight first model and select it await user.keyboard('{ArrowDown}') await user.keyboard('{Enter}')