From cab215e20979dba5198f0916dd43b8fac4dfb6e7 Mon Sep 17 00:00:00 2001 From: Jingyi Date: Tue, 26 May 2026 22:07:40 -0700 Subject: [PATCH] fix(web): add loading skeletons for tools and knowledge lists (#36712) --- .../datasets/list/__tests__/datasets.spec.tsx | 123 ++++++++++++++++++ .../datasets/list/dataset-card-skeleton.tsx | 40 ++++++ web/app/components/datasets/list/datasets.tsx | 18 ++- .../tools/__tests__/provider-list.spec.tsx | 40 +++++- .../tools/mcp/__tests__/index.spec.tsx | 38 ++++-- web/app/components/tools/mcp/index.tsx | 47 +++---- web/app/components/tools/provider-list.tsx | 65 ++++----- .../tools/provider/tool-card-skeleton.tsx | 50 +++++++ web/service/knowledge/use-dataset.ts | 1 + 9 files changed, 344 insertions(+), 78 deletions(-) create mode 100644 web/app/components/datasets/list/dataset-card-skeleton.tsx create mode 100644 web/app/components/tools/provider/tool-card-skeleton.tsx diff --git a/web/app/components/datasets/list/__tests__/datasets.spec.tsx b/web/app/components/datasets/list/__tests__/datasets.spec.tsx index f78622a9cd..566abe274f 100644 --- a/web/app/components/datasets/list/__tests__/datasets.spec.tsx +++ b/web/app/components/datasets/list/__tests__/datasets.spec.tsx @@ -2,6 +2,7 @@ import type { DataSet } from '@/models/datasets' import { render, screen, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { IndexingType } from '@/app/components/datasets/create/step-two' +import { useSelector as useAppContextSelector } from '@/context/app-context' import { ChunkingMode, DatasetPermission, DataSourceType } from '@/models/datasets' import { RETRIEVE_METHOD } from '@/types/app' import Datasets from '../datasets' @@ -44,6 +45,8 @@ vi.mock('@/service/knowledge/use-dataset', () => ({ hasNextPage: false, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, })), useInvalidDatasetList: () => mockInvalidDatasetList, })) @@ -155,6 +158,7 @@ describe('Datasets', () => { // Setup IntersectionObserver mock vi.stubGlobal('IntersectionObserver', MockIntersectionObserver) + vi.mocked(useAppContextSelector).mockReturnValue(true) }) afterEach(() => { @@ -238,6 +242,77 @@ describe('Datasets', () => { }) describe('Loading States', () => { + it('should show dataset card skeletons while initial dataset list is loading', async () => { + const { useDatasetList } = await import('@/service/knowledge/use-dataset') + vi.mocked(useDatasetList).mockReturnValue({ + data: undefined, + fetchNextPage: mockFetchNextPage, + hasNextPage: false, + isFetching: true, + isFetchingNextPage: false, + isLoading: true, + isPlaceholderData: false, + } as unknown as ReturnType) + + render() + + expect(screen.getByRole('status', { name: /common\.loading/ })).toBeInTheDocument() + expect(screen.queryByText('Dataset 1')).not.toBeInTheDocument() + }) + + it('should not show dataset card skeletons after an empty dataset list has loaded', async () => { + const { useDatasetList } = await import('@/service/knowledge/use-dataset') + vi.mocked(useDatasetList).mockReturnValue({ + data: { pages: [{ data: [] }] }, + fetchNextPage: mockFetchNextPage, + hasNextPage: false, + isFetching: false, + isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, + } as unknown as ReturnType) + + render() + + expect(screen.queryByRole('status', { name: /common\.loading/ })).not.toBeInTheDocument() + expect(screen.getByText(/createDataset/)).toBeInTheDocument() + }) + + it('should show dataset card skeletons when placeholder data is empty and the next query is fetching', async () => { + const { useDatasetList } = await import('@/service/knowledge/use-dataset') + vi.mocked(useDatasetList).mockReturnValue({ + data: { pages: [{ data: [] }] }, + fetchNextPage: mockFetchNextPage, + hasNextPage: false, + isFetching: true, + isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: true, + } as unknown as ReturnType) + + render() + + expect(screen.getByRole('status', { name: /common\.loading/ })).toBeInTheDocument() + }) + + it('should keep rendered dataset cards when placeholder data has results during refetch', async () => { + const { useDatasetList } = await import('@/service/knowledge/use-dataset') + vi.mocked(useDatasetList).mockReturnValue({ + data: { pages: [{ data: [createMockDataset({ id: 'dataset-1', name: 'Dataset 1' })] }] }, + fetchNextPage: mockFetchNextPage, + hasNextPage: false, + isFetching: true, + isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: true, + } as unknown as ReturnType) + + render() + + expect(screen.queryByRole('status', { name: /common\.loading/ })).not.toBeInTheDocument() + expect(screen.getByText('Dataset 1')).toBeInTheDocument() + }) + it('should show Loading component when isFetchingNextPage is true', async () => { const { useDatasetList } = await import('@/service/knowledge/use-dataset') vi.mocked(useDatasetList).mockReturnValue({ @@ -246,6 +321,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: true, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -262,6 +339,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -278,6 +357,8 @@ describe('Datasets', () => { hasNextPage: false, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -292,6 +373,8 @@ describe('Datasets', () => { hasNextPage: false, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -306,6 +389,8 @@ describe('Datasets', () => { hasNextPage: false, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -322,6 +407,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -338,6 +425,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -361,6 +450,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -383,6 +474,8 @@ describe('Datasets', () => { hasNextPage: false, // No more pages isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() @@ -405,6 +498,32 @@ describe('Datasets', () => { hasNextPage: true, isFetching: true, // Already fetching isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, + } as unknown as ReturnType) + + render() + + if (intersectionObserverCallback) { + intersectionObserverCallback( + [{ isIntersecting: true } as IntersectionObserverEntry], + {} as IntersectionObserver, + ) + } + + expect(mockFetchNextPage).not.toHaveBeenCalled() + }) + + it('should NOT call fetchNextPage when placeholder data is showing', async () => { + const { useDatasetList } = await import('@/service/knowledge/use-dataset') + vi.mocked(useDatasetList).mockReturnValue({ + data: { pages: [{ data: [createMockDataset({ id: 'dataset-1', name: 'Dataset 1' })] }] }, + fetchNextPage: mockFetchNextPage, + hasNextPage: true, + isFetching: false, + isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: true, } as unknown as ReturnType) render() @@ -427,6 +546,8 @@ describe('Datasets', () => { hasNextPage: true, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) const { unmount } = render() @@ -471,6 +592,8 @@ describe('Datasets', () => { hasNextPage: false, isFetching: false, isFetchingNextPage: false, + isLoading: false, + isPlaceholderData: false, } as unknown as ReturnType) render() diff --git a/web/app/components/datasets/list/dataset-card-skeleton.tsx b/web/app/components/datasets/list/dataset-card-skeleton.tsx new file mode 100644 index 0000000000..8535dd3601 --- /dev/null +++ b/web/app/components/datasets/list/dataset-card-skeleton.tsx @@ -0,0 +1,40 @@ +import { SkeletonContainer, SkeletonRectangle, SkeletonRow } from '@/app/components/base/skeleton' + +type DatasetCardSkeletonProps = { + label: string + count?: number +} + +const DatasetCardSkeleton = ({ + label, + count = 6, +}: DatasetCardSkeletonProps) => ( +
+ {Array.from({ length: count }, (_, index) => ( +
+ + + +
+ + +
+
+
+ + +
+
+ + +
+
+
+ ))} +
+) + +export default DatasetCardSkeleton diff --git a/web/app/components/datasets/list/datasets.tsx b/web/app/components/datasets/list/datasets.tsx index 1e5c268636..7ba9326ce5 100644 --- a/web/app/components/datasets/list/datasets.tsx +++ b/web/app/components/datasets/list/datasets.tsx @@ -6,6 +6,7 @@ import Loading from '@/app/components/base/loading' import { useSelector as useAppContextWithSelector } from '@/context/app-context' import { useDatasetList, useInvalidDatasetList } from '@/service/knowledge/use-dataset' import DatasetCard from './dataset-card' +import DatasetCardSkeleton from './dataset-card-skeleton' import NewDatasetCard from './new-dataset-card' type Props = { @@ -29,6 +30,8 @@ const Datasets = ({ hasNextPage, isFetching, isFetchingNextPage, + isLoading, + isPlaceholderData, } = useDatasetList({ initialPage: 1, tag_ids: tags, @@ -39,6 +42,9 @@ const Datasets = ({ const invalidDatasetList = useInvalidDatasetList() const anchorRef = useRef(null) const observerRef = useRef(null) + const pages = datasetList?.pages ?? [] + const datasets = pages.flatMap(({ data }) => data) + const showDatasetSkeleton = !isFetchingNextPage && (isLoading || (isPlaceholderData && isFetching && datasets.length === 0)) useEffect(() => { document.title = `${t('knowledge', { ns: 'dataset' })} - Dify` @@ -47,7 +53,7 @@ const Datasets = ({ useEffect(() => { if (anchorRef.current) { observerRef.current = new IntersectionObserver((entries) => { - if (entries[0]!.isIntersecting && hasNextPage && !isFetching) + if (entries[0]!.isIntersecting && hasNextPage && !isFetching && !isPlaceholderData) fetchNextPage() }, { rootMargin: '100px', @@ -55,15 +61,17 @@ const Datasets = ({ observerRef.current.observe(anchorRef.current) } return () => observerRef.current?.disconnect() - }, [anchorRef, hasNextPage, isFetching, fetchNextPage]) + }, [anchorRef, hasNextPage, isFetching, isPlaceholderData, fetchNextPage]) return ( <> diff --git a/web/app/components/tools/__tests__/provider-list.spec.tsx b/web/app/components/tools/__tests__/provider-list.spec.tsx index 30c2854fa3..a42bfa7865 100644 --- a/web/app/components/tools/__tests__/provider-list.spec.tsx +++ b/web/app/components/tools/__tests__/provider-list.spec.tsx @@ -87,10 +87,12 @@ const createDefaultCollections = () => [ ] let mockCollectionData: ReturnType = [] +let mockIsLoadingToolProviders = false const mockRefetch = vi.fn() vi.mock('@/service/use-tools', () => ({ useAllToolProviders: () => ({ data: mockCollectionData, + isLoading: mockIsLoadingToolProviders, refetch: mockRefetch, }), })) @@ -106,7 +108,19 @@ vi.mock('@/service/use-plugins', () => ({ vi.mock('@/app/components/plugins/card', () => ({ default: ({ payload, className }: { payload: { name: string }, className?: string }) => ( -
{payload.name}
+
+ {payload.name} +
+ ), +})) + +vi.mock('@/app/components/tools/provider/tool-card-skeleton', () => ({ + default: () => ( + <> + {Array.from({ length: 6 }, (_, index) => ( +
Loading tool
+ ))} + ), })) @@ -221,6 +235,7 @@ describe('ProviderList', () => { vi.clearAllMocks() mockEnableMarketplace = false mockCollectionData = createDefaultCollections() + mockIsLoadingToolProviders = false mockCheckedInstalledData = null Element.prototype.scrollTo = vi.fn() }) @@ -331,6 +346,13 @@ describe('ProviderList', () => { renderProviderList({ category: 'api' }) expect(screen.getByTestId('custom-create-card')).toBeInTheDocument() }) + + it('shows card skeletons instead of custom create card while tool providers are loading', () => { + mockIsLoadingToolProviders = true + renderProviderList({ category: 'api' }) + expect(screen.getAllByTestId('tool-card-skeleton')).toHaveLength(6) + expect(screen.queryByTestId('custom-create-card')).not.toBeInTheDocument() + }) }) describe('Workflow Tab', () => { @@ -344,6 +366,14 @@ describe('ProviderList', () => { renderProviderList({ category: 'workflow' }) expect(screen.getByTestId('workflow-empty')).toBeInTheDocument() }) + + it('shows card skeletons instead of empty state while tool providers are loading', () => { + mockIsLoadingToolProviders = true + mockCollectionData = [] + renderProviderList({ category: 'workflow' }) + expect(screen.getAllByTestId('tool-card-skeleton')).toHaveLength(6) + expect(screen.queryByTestId('workflow-empty')).not.toBeInTheDocument() + }) }) describe('Builtin Tab Empty State', () => { @@ -353,6 +383,14 @@ describe('ProviderList', () => { expect(screen.getByTestId('empty')).toBeInTheDocument() }) + it('shows card skeletons instead of empty component while tool providers are loading', () => { + mockIsLoadingToolProviders = true + mockCollectionData = [] + renderProviderList() + expect(screen.getAllByTestId('tool-card-skeleton')).toHaveLength(6) + expect(screen.queryByTestId('empty')).not.toBeInTheDocument() + }) + it('renders collection that has no labels property', () => { mockCollectionData = [{ id: 'no-labels', diff --git a/web/app/components/tools/mcp/__tests__/index.spec.tsx b/web/app/components/tools/mcp/__tests__/index.spec.tsx index 58510dab4c..52f057d3c9 100644 --- a/web/app/components/tools/mcp/__tests__/index.spec.tsx +++ b/web/app/components/tools/mcp/__tests__/index.spec.tsx @@ -13,14 +13,26 @@ type MockDetail = MockProvider | undefined // Mock dependencies const mockRefetch = vi.fn() let mockProviders: MockProvider[] = [] +let mockIsLoadingToolProviders = false vi.mock('@/service/use-tools', () => ({ useAllToolProviders: () => ({ data: mockProviders, + isLoading: mockIsLoadingToolProviders, refetch: mockRefetch, }), })) +vi.mock('@/app/components/tools/provider/tool-card-skeleton', () => ({ + default: () => ( + <> + {Array.from({ length: 6 }, (_, index) => ( +
Loading MCP
+ ))} + + ), +})) + // Mock child components vi.mock('../create-card', () => ({ default: ({ handleCreate }: { handleCreate: (provider: { id: string, name: string }) => void }) => ( @@ -65,6 +77,7 @@ describe('MCPList', () => { vi.clearAllMocks() vi.useFakeTimers() mockProviders = [] + mockIsLoadingToolProviders = false mockRefetch.mockResolvedValue(undefined) }) @@ -85,15 +98,18 @@ describe('MCPList', () => { expect(screen.getByTestId('create-card')).toBeInTheDocument() }) - it('should render default skeleton cards when list is empty', () => { + it('should render card skeletons while tool providers are loading', () => { + mockIsLoadingToolProviders = true render() - // Should render skeleton cards when no providers - const container = document.querySelector('.grid') - expect(container).toBeInTheDocument() - // Check for skeleton cards (36 of them) - const skeletonCards = document.querySelectorAll('.h-\\[111px\\]') - expect(skeletonCards.length).toBe(36) + expect(screen.getAllByTestId('mcp-card-skeleton')).toHaveLength(6) + expect(screen.queryByTestId('provider-card-1')).not.toBeInTheDocument() + }) + + it('should not render card skeletons when the loaded list is empty', () => { + render() + + expect(screen.queryByTestId('mcp-card-skeleton')).not.toBeInTheDocument() }) it('should not render skeleton cards when providers exist', () => { @@ -102,8 +118,7 @@ describe('MCPList', () => { ] render() - const skeletonCards = document.querySelectorAll('.h-\\[111px\\]') - expect(skeletonCards.length).toBe(0) + expect(screen.queryByTestId('mcp-card-skeleton')).not.toBeInTheDocument() }) }) @@ -325,15 +340,16 @@ describe('MCPList', () => { expect(grid).toHaveClass('xl:grid-cols-4') }) - it('should have overflow hidden when list is empty', () => { + it('should have overflow hidden while loading', () => { mockProviders = [] + mockIsLoadingToolProviders = true render() const grid = document.querySelector('.grid') expect(grid).toHaveClass('overflow-hidden') }) - it('should not have overflow hidden when list has providers', () => { + it('should not have overflow hidden when loading is complete', () => { mockProviders = [{ id: '1', name: 'Provider 1', type: 'mcp' }] render() diff --git a/web/app/components/tools/mcp/index.tsx b/web/app/components/tools/mcp/index.tsx index f4b4b44c88..a9012d0652 100644 --- a/web/app/components/tools/mcp/index.tsx +++ b/web/app/components/tools/mcp/index.tsx @@ -2,6 +2,7 @@ import type { ToolWithProvider } from '@/app/components/workflow/types' import { cn } from '@langgenius/dify-ui/cn' import { useMemo, useState } from 'react' +import ToolCardSkeletonGrid from '@/app/components/tools/provider/tool-card-skeleton' import { useAllToolProviders, } from '@/service/use-tools' @@ -13,29 +14,10 @@ type Props = { searchText: string } -function renderDefaultCard() { - const defaultCards = Array.from({ length: 36 }, (_, index) => ( -
= 4 && index < 8 && 'opacity-50', - index >= 8 && index < 12 && 'opacity-40', - index >= 12 && index < 16 && 'opacity-30', - index >= 16 && index < 20 && 'opacity-25', - index >= 20 && index < 24 && 'opacity-20', - )} - > -
- )) - return defaultCards -} - const MCPList = ({ searchText, }: Props) => { - const { data: list = [] as ToolWithProvider[], refetch } = useAllToolProviders() + const { data: list = [] as ToolWithProvider[], isLoading, refetch } = useAllToolProviders() const [isTriggerAuthorize, setIsTriggerAuthorize] = useState(false) const filteredList = useMemo(() => { @@ -68,21 +50,22 @@ const MCPList = ({
- {filteredList.map(provider => ( - - ))} - {!list.length && renderDefaultCard()} + {isLoading + ? + : filteredList.map(provider => ( + + ))}
{currentProvider && ( { const handleKeywordsChange = (value: string) => { setKeywords(value) } - const { data: collectionList = [], refetch } = useAllToolProviders() + const { data: collectionList = [], isLoading: isCollectionListLoading, refetch } = useAllToolProviders() const filteredCollectionList = useMemo(() => { return collectionList.filter((collection) => { if (collection.type !== activeTab) @@ -165,36 +166,42 @@ const ProviderList = () => { !filteredCollectionList.length && activeTab === 'workflow' && 'grow', )} > - {activeTab === 'api' && } - {filteredCollectionList.map(collection => ( -
setCurrentProviderId(collection.id)} - > - getTagLabel(label)) || []} - /> - )} - /> -
- ))} - {!filteredCollectionList.length && activeTab === 'workflow' &&
} + {isCollectionListLoading + ? + : ( + <> + {activeTab === 'api' && } + {filteredCollectionList.map(collection => ( +
setCurrentProviderId(collection.id)} + > + getTagLabel(label)) || []} + /> + )} + /> +
+ ))} + {!filteredCollectionList.length && activeTab === 'workflow' &&
} + + )} )} - {!filteredCollectionList.length && activeTab === 'builtin' && ( + {!isCollectionListLoading && !filteredCollectionList.length && activeTab === 'builtin' && ( )}
diff --git a/web/app/components/tools/provider/tool-card-skeleton.tsx b/web/app/components/tools/provider/tool-card-skeleton.tsx new file mode 100644 index 0000000000..72dedfb24c --- /dev/null +++ b/web/app/components/tools/provider/tool-card-skeleton.tsx @@ -0,0 +1,50 @@ +import { cn } from '@langgenius/dify-ui/cn' +import { SkeletonContainer, SkeletonPoint, SkeletonRectangle, SkeletonRow } from '@/app/components/base/skeleton' + +type ToolCardSkeletonGridProps = { + className?: string + count?: number +} + +const ToolCardSkeleton = () => ( +
+
+
+ +
+
+ +
+ + + + + +
+
+ + + + +
+ + +
+
+
+) + +const ToolCardSkeletonGrid = ({ + className, + count = 6, +}: ToolCardSkeletonGridProps) => ( + <> + {Array.from({ length: count }, (_, index) => ( +
+ +
+ ))} + +) + +export default ToolCardSkeletonGrid diff --git a/web/service/knowledge/use-dataset.ts b/web/service/knowledge/use-dataset.ts index a6183028bf..152aea73e0 100644 --- a/web/service/knowledge/use-dataset.ts +++ b/web/service/knowledge/use-dataset.ts @@ -96,6 +96,7 @@ export const useDatasetList = (params: DatasetListRequest) => { }, getNextPageParam: lastPage => lastPage.has_more ? lastPage.page + 1 : null, initialPageParam: initialPage, + placeholderData: keepPreviousData, }) }