diff --git a/web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx b/web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx index c4f614737a..099a146866 100644 --- a/web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx +++ b/web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx @@ -1,6 +1,6 @@ import { noop } from 'es-toolkit/function' import * as React from 'react' -import { useState } from 'react' +import { useCallback, useState } from 'react' import { Trans, useTranslation } from 'react-i18next' import { useContext } from 'use-context-selector' import Button from '@/app/components/base/button' @@ -36,18 +36,33 @@ const TransferOwnershipModal = ({ onClose, show }: Props) => { const [stepToken, setStepToken] = useState('') const [newOwner, setNewOwner] = useState('') const [isTransfer, setIsTransfer] = useState(false) + const timerIdRef = React.useRef(undefined) + + const retimeCountdown = useCallback((timerId?: number) => { + if (timerIdRef.current !== undefined) + window.clearInterval(timerIdRef.current) + + timerIdRef.current = timerId + }, []) + + React.useEffect(() => { + if (!show) + retimeCountdown() + + return retimeCountdown + }, [retimeCountdown, show]) const startCount = () => { setTime(60) - const timer = setInterval(() => { + retimeCountdown(window.setInterval(() => { setTime((prev) => { - if (prev <= 0) { - clearInterval(timer) + if (prev <= 1) { + retimeCountdown() return 0 } return prev - 1 }) - }, 1000) + }, 1000)) } const sendEmail = async () => { diff --git a/web/app/components/plugins/plugin-detail-panel/app-selector/__tests__/index.spec.tsx b/web/app/components/plugins/plugin-detail-panel/app-selector/__tests__/index.spec.tsx index 5497786794..4dd604a03e 100644 --- a/web/app/components/plugins/plugin-detail-panel/app-selector/__tests__/index.spec.tsx +++ b/web/app/components/plugins/plugin-detail-panel/app-selector/__tests__/index.spec.tsx @@ -76,16 +76,16 @@ afterAll(() => { // Mock portal components for controlled positioning in tests // Use React context to properly scope open state per portal instance (for nested portals) -const _PortalOpenContext = React.createContext(false) - vi.mock('@/app/components/base/portal-to-follow-elem', () => { // Context reference shared across mock components let sharedContext: React.Context | null = null // Lazily get or create the context const getContext = (): React.Context => { - if (!sharedContext) - sharedContext = React.createContext(false) + if (!sharedContext) { + const PortalOpenContext = React.createContext(false) + sharedContext = PortalOpenContext + } return sharedContext } @@ -725,6 +725,39 @@ describe('AppPicker', () => { triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) expect(onLoadMore).toHaveBeenCalledTimes(2) }) + + it('should reset loadingRef when the picker closes before the debounce timeout finishes', () => { + const onLoadMore = vi.fn() + const { rerender } = render( + , + ) + + triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) + expect(onLoadMore).toHaveBeenCalledTimes(1) + + rerender() + rerender() + + triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) + expect(onLoadMore).toHaveBeenCalledTimes(2) + }) + + it('should reset loadingRef when the picker unmounts before the debounce timeout finishes', () => { + const onLoadMore = vi.fn() + const { unmount } = render( + , + ) + + triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) + expect(onLoadMore).toHaveBeenCalledTimes(1) + + unmount() + + render() + + triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) + expect(onLoadMore).toHaveBeenCalledTimes(2) + }) }) describe('Memoization', () => { @@ -1539,7 +1572,7 @@ describe('AppSelector', () => { expect(screen.getByTestId('portal-content')).toBeInTheDocument() }) - it('should manage isLoadingMore state during load more', () => { + it('should render correctly during load more setup', () => { mockHasNextPage = true mockFetchNextPage.mockImplementation(() => new Promise(resolve => setTimeout(resolve, 100))) @@ -1739,7 +1772,7 @@ describe('AppSelector', () => { expect(mockFetchNextPage).toHaveBeenCalled() }) - it('should set isLoadingMore and reset after delay in handleLoadMore', async () => { + it('should avoid duplicate fetches while the picker debounce is active', async () => { mockHasNextPage = true mockIsFetchingNextPage = false mockFetchNextPage.mockResolvedValue(undefined) @@ -1756,34 +1789,15 @@ describe('AppSelector', () => { expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - // Try to trigger again immediately - should be blocked by isLoadingMore + // Try to trigger again immediately - should be blocked by AppPicker loadingRef triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - // Still only one call due to isLoadingMore + // Still only one call due to the picker-level debounce expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - // This verifies the debounce logic is working - multiple calls are blocked expect(screen.getAllByTestId('portal-content').length).toBeGreaterThan(0) }) - it('should not call fetchNextPage when isLoadingMore is true', async () => { - mockHasNextPage = true - mockIsFetchingNextPage = false - mockFetchNextPage.mockImplementation(() => new Promise(resolve => setTimeout(resolve, 1000))) - - renderWithQueryClient() - - // Open portals - fireEvent.click(screen.getAllByTestId('portal-trigger')[0]) - const triggers = screen.getAllByTestId('portal-trigger') - fireEvent.click(triggers[1]) - - // Trigger intersection - this starts loading - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - - expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - }) - it('should skip handleLoadMore when isFetchingNextPage is true', async () => { mockHasNextPage = true mockIsFetchingNextPage = true // This will block the handleLoadMore @@ -1821,89 +1835,7 @@ describe('AppSelector', () => { // fetchNextPage should NOT be called because hasMore is false expect(mockFetchNextPage).not.toHaveBeenCalled() }) - - it('should return early from handleLoadMore when isLoadingMore is true', async () => { - mockHasNextPage = true - mockIsFetchingNextPage = false - // Make fetchNextPage slow to keep isLoadingMore true - mockFetchNextPage.mockImplementation(() => new Promise(resolve => setTimeout(resolve, 5000))) - - renderWithQueryClient() - - fireEvent.click(screen.getAllByTestId('portal-trigger')[0]) - const triggers = screen.getAllByTestId('portal-trigger') - fireEvent.click(triggers[1]) - - // First call starts loading - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - - // Second call should return early due to isLoadingMore - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - - // Still only 1 call because isLoadingMore blocks it - expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - }) - - it('should reset isLoadingMore via setTimeout after fetchNextPage resolves', async () => { - mockHasNextPage = true - mockIsFetchingNextPage = false - mockFetchNextPage.mockResolvedValue(undefined) - - renderWithQueryClient() - - fireEvent.click(screen.getAllByTestId('portal-trigger')[0]) - const triggers = screen.getAllByTestId('portal-trigger') - fireEvent.click(triggers[1]) - - // Trigger load more - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - - // Wait for fetchNextPage to complete and setTimeout to fire - await act(async () => { - await Promise.resolve() - vi.advanceTimersByTime(350) // Past the 300ms setTimeout - }) - - // Should be able to load more again - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - - // This might trigger another fetch if loadingRef also reset - expect(screen.getAllByTestId('portal-content').length).toBeGreaterThan(0) - }) - - it('should reset isLoadingMore after fetchNextPage completes with setTimeout', async () => { - mockHasNextPage = true - mockIsFetchingNextPage = false - mockFetchNextPage.mockResolvedValue(undefined) - - renderWithQueryClient() - - // Open portals - fireEvent.click(screen.getAllByTestId('portal-trigger')[0]) - const triggers = screen.getAllByTestId('portal-trigger') - fireEvent.click(triggers[1]) - - // Trigger first intersection - triggerIntersection([{ isIntersecting: true } as IntersectionObserverEntry]) - - expect(mockFetchNextPage).toHaveBeenCalledTimes(1) - - // Advance timer past the 300ms setTimeout in finally block - await act(async () => { - vi.advanceTimersByTime(400) - }) - - // Also advance past the loadingRef timeout in AppPicker (500ms) - await act(async () => { - vi.advanceTimersByTime(200) - }) - - // Verify component is still rendered correctly - expect(screen.getAllByTestId('portal-content').length).toBeGreaterThan(0) - }) }) - describe('Form Change Handling', () => { it('should handle form change with image file', () => { const onSelect = vi.fn() @@ -2284,7 +2216,7 @@ describe('AppSelector Integration', () => { expect(screen.getByTestId('portal-content')).toBeInTheDocument() }) - it('should set isLoadingMore to false after fetchNextPage completes', async () => { + it('should stay stable after fetchNextPage completes', async () => { mockHasNextPage = true mockIsFetchingNextPage = false mockFetchNextPage.mockResolvedValue(undefined) @@ -2293,16 +2225,10 @@ describe('AppSelector Integration', () => { fireEvent.click(screen.getAllByTestId('portal-trigger')[0]) - // Advance timers past the 300ms delay - await act(async () => { - vi.advanceTimersByTime(400) - }) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() }) it('should not call fetchNextPage when conditions prevent it', () => { - // isLoadingMore would be true internally mockHasNextPage = false mockIsFetchingNextPage = true diff --git a/web/app/components/plugins/plugin-detail-panel/app-selector/app-picker.tsx b/web/app/components/plugins/plugin-detail-panel/app-selector/app-picker.tsx index c32e959652..b849ced8fd 100644 --- a/web/app/components/plugins/plugin-detail-panel/app-selector/app-picker.tsx +++ b/web/app/components/plugins/plugin-detail-panel/app-selector/app-picker.tsx @@ -51,9 +51,30 @@ const AppPicker: FC = ({ onSearchChange, }) => { const { t } = useTranslation() - const observerTarget = useRef(null) + const observerTargetRef = useRef(null) const observerRef = useRef(null) const loadingRef = useRef(false) + const loadingResetTimerIdRef = useRef(undefined) + + const retimeLoadingReset = useCallback((timerId?: number) => { + if (loadingResetTimerIdRef.current !== undefined) + globalThis.clearTimeout(loadingResetTimerIdRef.current) + + loadingResetTimerIdRef.current = timerId + }, []) + + const resetLoadingState = useCallback(() => { + retimeLoadingReset() + loadingRef.current = false + }, [retimeLoadingReset]) + + const disconnectObserver = useCallback(() => { + if (!observerRef.current) + return + + observerRef.current.disconnect() + observerRef.current = null + }, []) const handleIntersection = useCallback((entries: IntersectionObserverEntry[]) => { const target = entries[0] @@ -62,27 +83,27 @@ const AppPicker: FC = ({ loadingRef.current = true onLoadMore() - // Reset loading state - setTimeout(() => { + retimeLoadingReset(window.setTimeout(() => { loadingRef.current = false - }, 500) - }, [hasMore, isLoading, onLoadMore]) + retimeLoadingReset() + }, 500)) + }, [hasMore, isLoading, onLoadMore, retimeLoadingReset]) useEffect(() => { if (!isShow) { - if (observerRef.current) { - observerRef.current.disconnect() - observerRef.current = null - } + resetLoadingState() + disconnectObserver() return } let mutationObserver: MutationObserver | null = null const setupIntersectionObserver = () => { - if (!observerTarget.current) + if (!observerTargetRef.current) return + disconnectObserver() + // Create new observer observerRef.current = new IntersectionObserver(handleIntersection, { root: null, @@ -90,12 +111,12 @@ const AppPicker: FC = ({ threshold: 0.1, }) - observerRef.current.observe(observerTarget.current) + observerRef.current.observe(observerTargetRef.current) } // Set up MutationObserver to watch DOM changes mutationObserver = new MutationObserver((_mutations) => { - if (observerTarget.current) { + if (observerTargetRef.current) { setupIntersectionObserver() mutationObserver?.disconnect() } @@ -108,17 +129,15 @@ const AppPicker: FC = ({ }) // If element exists, set up IntersectionObserver directly - if (observerTarget.current) + if (observerTargetRef.current) setupIntersectionObserver() return () => { - if (observerRef.current) { - observerRef.current.disconnect() - observerRef.current = null - } + resetLoadingState() + disconnectObserver() mutationObserver?.disconnect() } - }, [isShow, handleIntersection]) + }, [disconnectObserver, handleIntersection, isShow, resetLoadingState]) const getAppType = (app: App) => { switch (app.mode) { @@ -180,7 +199,7 @@ const AppPicker: FC = ({ background={app.icon_background} imageUrl={app.icon_url} /> -
+
{app.name} ( @@ -188,10 +207,10 @@ const AppPicker: FC = ({ )
-
{getAppType(app)}
+
{getAppType(app)}
))} -
+
{isLoading && (
{t('loading', { ns: 'common' })}
diff --git a/web/app/components/plugins/plugin-detail-panel/app-selector/index.tsx b/web/app/components/plugins/plugin-detail-panel/app-selector/index.tsx index 5d0fa6d4b8..92960195a4 100644 --- a/web/app/components/plugins/plugin-detail-panel/app-selector/index.tsx +++ b/web/app/components/plugins/plugin-detail-panel/app-selector/index.tsx @@ -47,9 +47,8 @@ const AppSelector: FC = ({ onSelect, }) => { const { t } = useTranslation() - const [isShow, onShowChange] = useState(false) + const [isShow, setIsShow] = useState(false) const [searchText, setSearchText] = useState('') - const [isLoadingMore, setIsLoadingMore] = useState(false) const { data, @@ -97,25 +96,16 @@ const AppSelector: FC = ({ const hasMore = hasNextPage ?? true const handleLoadMore = useCallback(async () => { - if (isLoadingMore || isFetchingNextPage || !hasMore) + if (isFetchingNextPage || !hasMore) return - setIsLoadingMore(true) - try { - await fetchNextPage() - } - finally { - // Add a small delay to ensure state updates are complete - setTimeout(() => { - setIsLoadingMore(false) - }, 300) - } - }, [isLoadingMore, isFetchingNextPage, hasMore, fetchNextPage]) + await fetchNextPage() + }, [fetchNextPage, hasMore, isFetchingNextPage]) const handleTriggerClick = () => { if (disabled) return - onShowChange(true) + setIsShow(true) } const [isShowChooseApp, setIsShowChooseApp] = useState(false) @@ -157,7 +147,7 @@ const AppSelector: FC = ({ placement={placement} offset={offset} open={isShow} - onOpenChange={onShowChange} + onOpenChange={setIsShow} > = ({
-
{t('appSelector.label', { ns: 'app' })}
+
{t('appSelector.label', { ns: 'app' })}
= ({ onSelect={handleSelectApp} scope={scope || 'all'} apps={appsForPicker} - isLoading={isLoading || isLoadingMore || isFetchingNextPage} + isLoading={isLoading || isFetchingNextPage} hasMore={hasMore} onLoadMore={handleLoadMore} searchText={searchText} diff --git a/web/eslint-suppressions.json b/web/eslint-suppressions.json index 174b7a875c..ae1d44770c 100644 --- a/web/eslint-suppressions.json +++ b/web/eslint-suppressions.json @@ -5172,9 +5172,6 @@ "app/components/plugins/plugin-detail-panel/app-selector/app-picker.tsx": { "no-restricted-imports": { "count": 1 - }, - "tailwindcss/enforce-consistent-class-order": { - "count": 2 } }, "app/components/plugins/plugin-detail-panel/app-selector/app-trigger.tsx": { @@ -5185,9 +5182,6 @@ "app/components/plugins/plugin-detail-panel/app-selector/index.tsx": { "no-restricted-imports": { "count": 1 - }, - "tailwindcss/enforce-consistent-class-order": { - "count": 1 } }, "app/components/plugins/plugin-detail-panel/datasource-action-list.tsx": {