Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: disabled remote model and show prompt users to use on-device models when offline mode #4229

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions joi/src/hooks/useOnlineStatus/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { useState, useEffect } from 'react'

const useOnlineStatus = () => {
const [isOnline, setIsOnline] = useState(navigator.onLine)

useEffect(() => {
const handleOnline = () => setIsOnline(true)
const handleOffline = () => setIsOnline(false)

window.addEventListener('online', handleOnline)
window.addEventListener('offline', handleOffline)

return () => {
window.removeEventListener('online', handleOnline)
window.removeEventListener('offline', handleOffline)
}
}, [])

return { isOnline }
}

export { useOnlineStatus }
62 changes: 62 additions & 0 deletions joi/src/hooks/useOnlineStatus/useOnlineStatus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { renderHook, act } from '@testing-library/react'
import { useOnlineStatus } from './index'

describe('useOnlineStatus', () => {
beforeEach(() => {
jest.spyOn(window, 'addEventListener')
jest.spyOn(window, 'removeEventListener')
})

afterEach(() => {
jest.restoreAllMocks()
})

it('should initialize with the correct online status', () => {
const { result } = renderHook(() => useOnlineStatus())
expect(result.current.isOnline).toBe(navigator.onLine)
})

it('should set online status to true when the online event is triggered', () => {
const { result } = renderHook(() => useOnlineStatus())

act(() => {
window.dispatchEvent(new Event('online'))
})

expect(result.current.isOnline).toBe(true)
})

it('should set online status to false when the offline event is triggered', () => {
const { result } = renderHook(() => useOnlineStatus())

act(() => {
window.dispatchEvent(new Event('offline'))
})

expect(result.current.isOnline).toBe(false)
})

it('should register event listeners on mount and unregister on unmount', () => {
const { unmount } = renderHook(() => useOnlineStatus())

expect(window.addEventListener).toHaveBeenCalledWith(
'online',
expect.any(Function)
)
expect(window.addEventListener).toHaveBeenCalledWith(
'offline',
expect.any(Function)
)

unmount()

expect(window.removeEventListener).toHaveBeenCalledWith(
'online',
expect.any(Function)
)
expect(window.removeEventListener).toHaveBeenCalledWith(
'offline',
expect.any(Function)
)
})
})
1 change: 1 addition & 0 deletions joi/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@ describe('Exports', () => {
expect(components.useClickOutside).toBeDefined()
expect(components.useOs).toBeDefined()
expect(components.useMediaQuery).toBeDefined()
expect(components.useOnlineStatus).toBeDefined()
})
})
1 change: 1 addition & 0 deletions joi/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ export * from './hooks/useTextSelection'
export * from './hooks/useClickOutside'
export * from './hooks/useOs'
export * from './hooks/useMediaQuery'
export * from './hooks/useOnlineStatus'
18 changes: 16 additions & 2 deletions web/containers/ModelDropdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
ScrollArea,
Tabs,
useClickOutside,
useOnlineStatus,
} from '@janhq/joi'

import { useAtom, useAtomValue, useSetAtom } from 'jotai'
Expand All @@ -18,6 +19,7 @@
ChevronDownIcon,
ChevronUpIcon,
DownloadCloudIcon,
WifiOff,
XIcon,
} from 'lucide-react'
import { twMerge } from 'tailwind-merge'
Expand Down Expand Up @@ -94,12 +96,14 @@

useClickOutside(() => setOpen(false), null, [dropdownOptions, toggle])

const { isOnline } = useOnlineStatus()

const [showEngineListModel, setShowEngineListModel] = useAtom(
showEngineListModelAtom
)

const isModelSupportRagAndTools = useCallback((model: Model) => {
return (

Check warning on line 106 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

106 line is not covered with tests
model?.engine === InferenceEngine.openai ||
isLocalEngine(model?.engine as InferenceEngine)
)
Expand All @@ -110,7 +114,7 @@
configuredModels
.concat(
downloadedModels.filter(
(e) => !configuredModels.some((x) => x.id === e.id)

Check warning on line 117 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

117 line is not covered with tests
)
)
.filter((e) =>
Expand All @@ -120,24 +124,24 @@
if (searchFilter === 'local') {
return isLocalEngine(e.engine)
}
if (searchFilter === 'remote') {
return !isLocalEngine(e.engine)

Check warning on line 128 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

127-128 lines are not covered with tests
}
})
.sort((a, b) => a.name.localeCompare(b.name))

Check warning on line 131 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

131 line is not covered with tests
.sort((a, b) => {
const aInDownloadedModels = downloadedModels.some(
(item) => item.id === a.id

Check warning on line 134 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

133-134 lines are not covered with tests
)
const bInDownloadedModels = downloadedModels.some(
(item) => item.id === b.id

Check warning on line 137 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

136-137 lines are not covered with tests
)
if (aInDownloadedModels && !bInDownloadedModels) {
return -1
} else if (!aInDownloadedModels && bInDownloadedModels) {
return 1

Check warning on line 142 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

139-142 lines are not covered with tests
} else {
return 0

Check warning on line 144 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

144 line is not covered with tests
}
}),
[configuredModels, searchText, searchFilter, downloadedModels]
Expand All @@ -145,7 +149,7 @@

useEffect(() => {
if (open && searchInputRef.current) {
searchInputRef.current.focus()

Check warning on line 152 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

152 line is not covered with tests
}
}, [open])

Expand All @@ -162,9 +166,9 @@

const onClickModelItem = useCallback(
async (modelId: string) => {
const model = downloadedModels.find((m) => m.id === modelId)
setSelectedModel(model)
setOpen(false)

Check warning on line 171 in web/containers/ModelDropdown/index.tsx

View workflow job for this annotation

GitHub Actions / coverage-check

169-171 lines are not covered with tests

if (activeThread) {
// Change assistand tools based on model support RAG
Expand Down Expand Up @@ -544,12 +548,16 @@
key={model.id}
className={twMerge(
'flex items-center justify-between gap-4 px-3 py-2 hover:bg-[hsla(var(--dropdown-menu-hover-bg))]',
!apiKey
(!apiKey || !isOnline) &&
!isLocalEngine(model.engine)
? 'cursor-not-allowed text-[hsla(var(--text-tertiary))]'
: 'text-[hsla(var(--text-primary))]'
)}
onClick={() => {
if (!apiKey && !isLocalEngine(model.engine))
if (
(!apiKey || !isOnline) &&
!isLocalEngine(model.engine)
)
return null
if (isDownloaded) {
onClickModelItem(model.id)
Expand Down Expand Up @@ -613,6 +621,12 @@
</div>
)
})}
{!isOnline && searchFilter === 'remote' && (
<div className="sticky bottom-0 flex items-start gap-2 border-t border-[hsla(var(--app-border))] bg-[hsla(var(--app-bg))] p-3 text-[hsla(var(--text-secondary))]">
<WifiOff />
<p>No internet connection. Please use on-device models.</p>
</div>
)}
</ScrollArea>
</div>
</div>
Expand Down
Loading