Skip to content

Commit 4544fd4

Browse files
authored
improvement(ui): fix nav loading flash, skeleton mismatches, and React anti-patterns across resource pages (#3864)
* improvement(ui): fix nav loading flash, skeleton mismatches, and React anti-patterns across resource pages - Convert knowledge, files, tables, scheduled-tasks, and home page.tsx files from async server components to simple client re-exports, eliminating the loading.tsx flash on every navigation - Add client-side permission redirects (usePermissionConfig) to knowledge, files, and tables components to replace server-side checks - Fix knowledge loading.tsx skeleton column count (6→7) and tables loading.tsx (remove phantom checkbox column) - Fix connector document live updates: use isConnectorSyncingOrPending instead of status === 'syncing' so polling activates immediately after connector creation - Remove dead chunk-switch useEffect in ChunkEditor (redundant with key prop remount) - Replace useState+useEffect debounce with useDebounce hook in document.tsx - Replace useRef+useEffect URL init with lazy useState initializers in document.tsx and logs.tsx - Make handleToggleEnabled optimistic in document.tsx (cache first, onError rollback) - Replace mutate+new Promise wrapper with mutateAsync+try/catch in base.tsx - Fix schedule-modal.tsx: replace 15-setter useEffect with useState lazy initializers + key prop remount; wrap parseCronToScheduleType in useMemo - Fix logs search: eliminate mount-only useEffect with eslint-disable by passing initialQuery to useSearchState; parse query once via shared initialParsed state - Add useWorkspaceFileRecord hook to workspace-files.ts; refactor FileViewer to self-fetch - Fix value: any → value: string in useTagSelection and collaborativeSetTagSelection - Fix knowledge-tag-filters.tsx: pass '' instead of null when filters are cleared (type safety) * fix(kb): use active scope in useWorkspaceFileRecord to share cache with useWorkspaceFiles * fix(logs,kb,tasks): lazy-init useRef for URL param, add cold-path docs to useWorkspaceFileRecord, document key remount requirement in ScheduleModal * fix(files): redirect to files list when file record not found in viewer * revert(files): remove useEffect redirect from file-viewer, keep simple null return * fix(scheduled-tasks): correct useMemo dep from schedule?.cronExpression to schedule
1 parent 019630b commit 4544fd4

File tree

28 files changed

+166
-373
lines changed

28 files changed

+166
-373
lines changed
Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,9 @@
11
import type { Metadata } from 'next'
2-
import { redirect } from 'next/navigation'
3-
import { getSession } from '@/lib/auth'
4-
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
5-
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
62
import { Files } from '../files'
73

84
export const metadata: Metadata = {
95
title: 'Files',
106
robots: { index: false },
117
}
128

13-
interface FileDetailPageProps {
14-
params: Promise<{
15-
workspaceId: string
16-
fileId: string
17-
}>
18-
}
19-
20-
export default async function FileDetailPage({ params }: FileDetailPageProps) {
21-
const { workspaceId } = await params
22-
const session = await getSession()
23-
24-
if (!session?.user?.id) {
25-
redirect('/')
26-
}
27-
28-
const hasPermission = await verifyWorkspaceMembership(session.user.id, workspaceId)
29-
if (!hasPermission) {
30-
redirect('/')
31-
}
32-
33-
const permissionConfig = await getUserPermissionConfig(session.user.id)
34-
if (permissionConfig?.hideFilesTab) {
35-
redirect(`/workspace/${workspaceId}`)
36-
}
37-
38-
return <Files />
39-
}
9+
export default Files

apps/sim/app/workspace/[workspaceId]/files/[fileId]/view/file-viewer.tsx

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
'use client'
22

33
import { createLogger } from '@sim/logger'
4-
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
4+
import { useParams } from 'next/navigation'
5+
import { useWorkspaceFileRecord } from '@/hooks/queries/workspace-files'
56

67
const logger = createLogger('FileViewer')
78

8-
interface FileViewerProps {
9-
file: WorkspaceFileRecord
10-
}
9+
export function FileViewer() {
10+
const params = useParams()
11+
const workspaceId = params?.workspaceId as string
12+
const fileId = params?.fileId as string
13+
14+
const { data: file, isLoading } = useWorkspaceFileRecord(workspaceId, fileId)
15+
16+
if (isLoading || !file) {
17+
return null
18+
}
1119

12-
export function FileViewer({ file }: FileViewerProps) {
1320
const serveUrl = `/api/files/serve/${encodeURIComponent(file.key)}?context=workspace`
1421

1522
return (
@@ -18,7 +25,7 @@ export function FileViewer({ file }: FileViewerProps) {
1825
src={serveUrl}
1926
className='h-full w-full border-0'
2027
title={file.name}
21-
onError={(e) => {
28+
onError={() => {
2229
logger.error(`Failed to load file: ${file.name}`)
2330
}}
2431
/>
Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,9 @@
11
import type { Metadata } from 'next'
2-
import { redirect, unstable_rethrow } from 'next/navigation'
3-
import { getSession } from '@/lib/auth'
4-
import { getWorkspaceFile } from '@/lib/uploads/contexts/workspace'
5-
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
6-
import { FileViewer } from '@/app/workspace/[workspaceId]/files/[fileId]/view/file-viewer'
2+
import { FileViewer } from './file-viewer'
73

84
export const metadata: Metadata = {
95
title: 'File',
106
robots: { index: false },
117
}
128

13-
interface FileViewerPageProps {
14-
params: Promise<{
15-
workspaceId: string
16-
fileId: string
17-
}>
18-
}
19-
20-
export default async function FileViewerPage({ params }: FileViewerPageProps) {
21-
const { workspaceId, fileId } = await params
22-
23-
const session = await getSession()
24-
if (!session?.user?.id) {
25-
redirect('/')
26-
}
27-
28-
const hasPermission = await verifyWorkspaceMembership(session.user.id, workspaceId)
29-
if (!hasPermission) {
30-
redirect(`/workspace/${workspaceId}`)
31-
}
32-
33-
let fileRecord: Awaited<ReturnType<typeof getWorkspaceFile>>
34-
try {
35-
fileRecord = await getWorkspaceFile(workspaceId, fileId)
36-
} catch (error) {
37-
unstable_rethrow(error)
38-
redirect(`/workspace/${workspaceId}`)
39-
}
40-
41-
if (!fileRecord) {
42-
redirect(`/workspace/${workspaceId}`)
43-
}
44-
45-
return <FileViewer file={fileRecord} />
46-
}
9+
export default FileViewer

apps/sim/app/workspace/[workspaceId]/files/files.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import {
7575
} from '@/hooks/queries/workspace-files'
7676
import { useDebounce } from '@/hooks/use-debounce'
7777
import { useInlineRename } from '@/hooks/use-inline-rename'
78+
import { usePermissionConfig } from '@/hooks/use-permission-config'
7879

7980
type SaveStatus = 'idle' | 'saving' | 'saved' | 'error'
8081

@@ -136,6 +137,13 @@ export function Files() {
136137
const fileIdFromRoute =
137138
typeof params?.fileId === 'string' && params.fileId.length > 0 ? params.fileId : null
138139
const userPermissions = useUserPermissionsContext()
140+
const { config: permissionConfig } = usePermissionConfig()
141+
142+
useEffect(() => {
143+
if (permissionConfig.hideFilesTab) {
144+
router.replace(`/workspace/${workspaceId}`)
145+
}
146+
}, [permissionConfig.hideFilesTab, router, workspaceId])
139147

140148
const { data: files = [], isLoading, error } = useWorkspaceFiles(workspaceId)
141149
const { data: members } = useWorkspaceMembersQuery(workspaceId)
Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,9 @@
11
import type { Metadata } from 'next'
2-
import { redirect } from 'next/navigation'
3-
import { getSession } from '@/lib/auth'
4-
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
5-
import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check'
62
import { Files } from './files'
73

84
export const metadata: Metadata = {
95
title: 'Files',
106
robots: { index: false },
117
}
128

13-
interface FilesPageProps {
14-
params: Promise<{
15-
workspaceId: string
16-
}>
17-
}
18-
19-
export default async function FilesPage({ params }: FilesPageProps) {
20-
const { workspaceId } = await params
21-
const session = await getSession()
22-
23-
if (!session?.user?.id) {
24-
redirect('/')
25-
}
26-
27-
const hasPermission = await verifyWorkspaceMembership(session.user.id, workspaceId)
28-
if (!hasPermission) {
29-
redirect('/')
30-
}
31-
32-
const permissionConfig = await getUserPermissionConfig(session.user.id)
33-
if (permissionConfig?.hideFilesTab) {
34-
redirect(`/workspace/${workspaceId}`)
35-
}
36-
37-
return <Files />
38-
}
9+
export default Files
Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,8 @@
11
import type { Metadata } from 'next'
2-
import { redirect } from 'next/navigation'
3-
import { getSession } from '@/lib/auth'
4-
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
52
import { Home } from './home'
63

74
export const metadata: Metadata = {
85
title: 'Home',
96
}
107

11-
interface HomePageProps {
12-
params: Promise<{
13-
workspaceId: string
14-
}>
15-
}
16-
17-
export default async function HomePage({ params }: HomePageProps) {
18-
const { workspaceId } = await params
19-
const session = await getSession()
20-
21-
if (!session?.user?.id) {
22-
redirect('/')
23-
}
24-
25-
const hasPermission = await verifyWorkspaceMembership(session.user.id, workspaceId)
26-
if (!hasPermission) {
27-
redirect('/')
28-
}
29-
30-
return <Home key='home' />
31-
}
8+
export default Home

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-editor/chunk-editor.tsx

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,11 @@ export function ChunkEditor({
5656
const [savedContent, setSavedContent] = useState(chunkContent)
5757
const [tokenizerOn, setTokenizerOn] = useState(false)
5858
const [hoveredTokenIndex, setHoveredTokenIndex] = useState<number | null>(null)
59-
const prevChunkIdRef = useRef(chunk?.id)
6059
const savedContentRef = useRef(chunkContent)
6160

6261
const editedContentRef = useRef(editedContent)
6362
editedContentRef.current = editedContent
6463

65-
useEffect(() => {
66-
if (isCreateMode) return
67-
if (chunk?.id !== prevChunkIdRef.current) {
68-
prevChunkIdRef.current = chunk?.id
69-
savedContentRef.current = chunkContent
70-
setSavedContent(chunkContent)
71-
setEditedContent(chunkContent)
72-
}
73-
}, [isCreateMode, chunk?.id, chunkContent])
74-
7564
useEffect(() => {
7665
if (isCreateMode || !chunk?.id) return
7766
const controller = new AbortController()

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx

Lines changed: 11 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { startTransition, useCallback, useEffect, useMemo, useRef, useState } from 'react'
3+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import { ChevronDown, ChevronUp, FileText, Pencil, Tag } from 'lucide-react'
66
import { useParams, useRouter, useSearchParams } from 'next/navigation'
@@ -47,6 +47,7 @@ import {
4747
useUpdateChunk,
4848
useUpdateDocument,
4949
} from '@/hooks/queries/kb/knowledge'
50+
import { useDebounce } from '@/hooks/use-debounce'
5051
import { useInlineRename } from '@/hooks/use-inline-rename'
5152

5253
const logger = createLogger('Document')
@@ -152,7 +153,7 @@ export function Document({
152153
const [showTagsModal, setShowTagsModal] = useState(false)
153154

154155
const [searchQuery, setSearchQuery] = useState('')
155-
const [debouncedSearchQuery, setDebouncedSearchQuery] = useState('')
156+
const debouncedSearchQuery = useDebounce(searchQuery, 200)
156157
const [enabledFilter, setEnabledFilter] = useState<string[]>([])
157158
const [activeSort, setActiveSort] = useState<{
158159
column: string
@@ -168,11 +169,8 @@ export function Document({
168169
chunks: initialChunks,
169170
currentPage: initialPage,
170171
totalPages: initialTotalPages,
171-
hasNextPage: initialHasNextPage,
172-
hasPrevPage: initialHasPrevPage,
173172
goToPage: initialGoToPage,
174173
error: initialError,
175-
refreshChunks: initialRefreshChunks,
176174
updateChunk: initialUpdateChunk,
177175
isFetching: isFetchingChunks,
178176
} = useDocumentChunks(
@@ -207,7 +205,9 @@ export function Document({
207205
const [selectedChunks, setSelectedChunks] = useState<Set<string>>(() => new Set())
208206

209207
// Inline editor state
210-
const [selectedChunkId, setSelectedChunkId] = useState<string | null>(null)
208+
const [selectedChunkId, setSelectedChunkId] = useState<string | null>(() =>
209+
searchParams.get('chunk')
210+
)
211211
const [isCreatingNewChunk, setIsCreatingNewChunk] = useState(false)
212212
const [isDirty, setIsDirty] = useState(false)
213213
const [saveStatus, setSaveStatus] = useState<SaveStatus>('idle')
@@ -217,27 +217,6 @@ export function Document({
217217
const saveStatusRef = useRef<SaveStatus>('idle')
218218
saveStatusRef.current = saveStatus
219219

220-
// Auto-select chunk from URL param on mount
221-
const initialChunkParam = useRef(searchParams.get('chunk'))
222-
useEffect(() => {
223-
if (initialChunkParam.current) {
224-
setSelectedChunkId(initialChunkParam.current)
225-
initialChunkParam.current = null
226-
}
227-
}, [])
228-
229-
useEffect(() => {
230-
const handler = setTimeout(() => {
231-
startTransition(() => {
232-
setDebouncedSearchQuery(searchQuery)
233-
})
234-
}, 200)
235-
236-
return () => {
237-
clearTimeout(handler)
238-
}
239-
}, [searchQuery])
240-
241220
const isSearching = debouncedSearchQuery.trim().length > 0
242221
const showingSearch = isSearching && searchQuery.trim().length > 0 && searchResults.length > 0
243222
const SEARCH_PAGE_SIZE = 50
@@ -259,8 +238,6 @@ export function Document({
259238

260239
const currentPage = showingSearch ? searchCurrentPage : initialPage
261240
const totalPages = showingSearch ? searchTotalPages : initialTotalPages
262-
const hasNextPage = showingSearch ? searchCurrentPage < searchTotalPages : initialHasNextPage
263-
const hasPrevPage = showingSearch ? searchCurrentPage > 1 : initialHasPrevPage
264241

265242
// Keep refs to displayChunks and totalPages so polling callbacks can read fresh data
266243
const displayChunksRef = useRef(displayChunks)
@@ -281,12 +258,11 @@ export function Document({
281258
if (showingSearch) {
282259
return
283260
}
284-
return await initialGoToPage(page)
261+
return initialGoToPage(page)
285262
},
286263
[showingSearch, initialGoToPage]
287264
)
288265

289-
const refreshChunks = showingSearch ? async () => {} : initialRefreshChunks
290266
const updateChunk = showingSearch
291267
? (_id: string, _updates: Record<string, unknown>) => {}
292268
: initialUpdateChunk
@@ -309,7 +285,6 @@ export function Document({
309285
const {
310286
isOpen: isContextMenuOpen,
311287
position: contextMenuPosition,
312-
menuRef,
313288
handleContextMenu: baseHandleContextMenu,
314289
closeMenu: closeContextMenu,
315290
} = useContextMenu()
@@ -661,18 +636,11 @@ export function Document({
661636
const chunk = displayChunks.find((c) => c.id === chunkId)
662637
if (!chunk) return
663638

639+
const newEnabled = !chunk.enabled
640+
updateChunk(chunkId, { enabled: newEnabled })
664641
updateChunkMutation(
665-
{
666-
knowledgeBaseId,
667-
documentId,
668-
chunkId,
669-
enabled: !chunk.enabled,
670-
},
671-
{
672-
onSuccess: () => {
673-
updateChunk(chunkId, { enabled: !chunk.enabled })
674-
},
675-
}
642+
{ knowledgeBaseId, documentId, chunkId, enabled: newEnabled },
643+
{ onError: () => updateChunk(chunkId, { enabled: chunk.enabled }) }
676644
)
677645
},
678646
[displayChunks, knowledgeBaseId, documentId, updateChunk]

0 commit comments

Comments
 (0)