From 5373498dc6a4481e85532c62782f8ea9cc543366 Mon Sep 17 00:00:00 2001 From: CodingOnStar Date: Fri, 17 Apr 2026 20:05:16 +0800 Subject: [PATCH] refactor(web): replace PortalToFollowElem with Popover components across various modules --- web/__mocks__/base-ui-popover.tsx | 35 ++++++ .../chat/citation/__tests__/popup.spec.tsx | 2 + .../base/chat/chat/citation/popup.tsx | 39 ++++--- .../date-picker/__tests__/index.spec.tsx | 14 ++- .../date-picker/index.tsx | 87 +++++++-------- .../time-picker/__tests__/index.spec.tsx | 11 +- .../time-picker/index.tsx | 101 +++++++++--------- .../__tests__/component.spec.tsx | 3 + .../plugins/context-block/component.tsx | 52 +++++---- .../__tests__/component.spec.tsx | 2 + .../plugins/history-block/component.tsx | 51 +++++---- .../publisher/__tests__/index.spec.tsx | 61 ++++++++++- .../publisher/__tests__/popup.spec.tsx | 92 ++++++++++++++-- .../rag-pipeline-header/publisher/index.tsx | 49 +++++---- .../rag-pipeline-header/publisher/popup.tsx | 14 ++- 15 files changed, 403 insertions(+), 210 deletions(-) diff --git a/web/__mocks__/base-ui-popover.tsx b/web/__mocks__/base-ui-popover.tsx index 81d10e947e..6fd3389687 100644 --- a/web/__mocks__/base-ui-popover.tsx +++ b/web/__mocks__/base-ui-popover.tsx @@ -14,6 +14,7 @@ type PopoverProps = { type PopoverTriggerProps = React.HTMLAttributes & { children?: ReactNode + nativeButton?: boolean render?: React.ReactElement } @@ -31,6 +32,32 @@ export const Popover = ({ open = false, onOpenChange, }: PopoverProps) => { + React.useEffect(() => { + if (!open) + return + + const handleMouseDown = (event: MouseEvent) => { + const target = event.target as Element | null + if (target?.closest?.('[data-popover-trigger="true"], [data-popover-content="true"]')) + return + + onOpenChange?.(false) + } + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') + onOpenChange?.(false) + } + + document.addEventListener('mousedown', handleMouseDown) + document.addEventListener('keydown', handleKeyDown) + + return () => { + document.removeEventListener('mousedown', handleMouseDown) + document.removeEventListener('keydown', handleKeyDown) + } + }, [open, onOpenChange]) + return ( { @@ -61,9 +89,12 @@ export const PopoverTrigger = ({ ...props, ...childProps, 'data-testid': childProps['data-testid'] ?? 'popover-trigger', + 'data-popover-trigger': 'true', 'onClick': (event: React.MouseEvent) => { childProps.onClick?.(event) onClick?.(event) + if (event.defaultPrevented) + return onOpenChange(!open) }, }) @@ -72,8 +103,11 @@ export const PopoverTrigger = ({ return (
{ onClick?.(event) + if (event.defaultPrevented) + return onOpenChange(!open) }} {...props} @@ -101,6 +135,7 @@ export const PopoverContent = ({ return (
await import('@/__mocks__/base-ui-popover')) + vi.mock('@/service/knowledge/use-document', () => ({ useDocumentDownload: vi.fn(), })) diff --git a/web/app/components/base/chat/chat/citation/popup.tsx b/web/app/components/base/chat/chat/citation/popup.tsx index 51a73bc4b6..9ea4a6b742 100644 --- a/web/app/components/base/chat/chat/citation/popup.tsx +++ b/web/app/components/base/chat/chat/citation/popup.tsx @@ -1,13 +1,9 @@ import type { FC, MouseEvent } from 'react' import type { Resources } from './index' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { Fragment, useState } from 'react' import { useTranslation } from 'react-i18next' import FileIcon from '@/app/components/base/file-icon' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import Link from '@/next/link' import { useDocumentDownload } from '@/service/knowledge/use-document' import { downloadUrl } from '@/utils/download' @@ -47,22 +43,25 @@ const Popup: FC = ({ } return ( - - setOpen(v => !v)}> -
- -
{data.documentName}
-
- - + + +
{data.documentName}
+
+ )} + /> +
@@ -156,8 +155,8 @@ const Popup: FC = ({
- - + + ) } diff --git a/web/app/components/base/date-and-time-picker/date-picker/__tests__/index.spec.tsx b/web/app/components/base/date-and-time-picker/date-picker/__tests__/index.spec.tsx index ea4f1bb928..0e72a69351 100644 --- a/web/app/components/base/date-and-time-picker/date-picker/__tests__/index.spec.tsx +++ b/web/app/components/base/date-and-time-picker/date-picker/__tests__/index.spec.tsx @@ -3,6 +3,15 @@ import { act, fireEvent, render, screen, within } from '@testing-library/react' import dayjs from '../../utils/dayjs' import DatePicker from '../index' +vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) +vi.mock('@langgenius/dify-ui/button', () => ({ + Button: ({ children, onClick, disabled, className }: Record) => ( + + ), +})) + // Mock scrollIntoView beforeAll(() => { Element.prototype.scrollIntoView = vi.fn() @@ -113,14 +122,13 @@ describe('DatePicker', () => { render() openPicker() + expect(screen.getByTestId('popover')).toHaveAttribute('data-open', 'true') - // Simulate a mousedown event outside the container act(() => { document.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) }) - // The picker should now be closed - input shows its value - // The picker should now be closed - input shows its value + expect(screen.getByTestId('popover')).toHaveAttribute('data-open', 'false') expect(screen.getByRole('textbox'))!.toBeInTheDocument() }) }) diff --git a/web/app/components/base/date-and-time-picker/date-picker/index.tsx b/web/app/components/base/date-and-time-picker/date-picker/index.tsx index 9c84e4c096..7858fa2fbe 100644 --- a/web/app/components/base/date-and-time-picker/date-picker/index.tsx +++ b/web/app/components/base/date-and-time-picker/date-picker/index.tsx @@ -1,14 +1,10 @@ import type { Dayjs } from 'dayjs' import type { DatePickerProps, Period } from '../types' import { cn } from '@langgenius/dify-ui/cn' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import * as React from 'react' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import Calendar from '../calendar' import TimePickerHeader from '../time-picker/header' import TimePickerOptions from '../time-picker/options' @@ -35,15 +31,14 @@ const DatePicker = ({ needTimePicker = true, renderTrigger, triggerWrapClassName, - popupZIndexClassname = 'z-11', + popupZIndexClassname, noConfirm, getIsDateDisabled, }: DatePickerProps) => { const { t } = useTranslation() const [isOpen, setIsOpen] = useState(false) const [view, setView] = useState(ViewType.date) - const containerRef = useRef(null) - const isInitial = useRef(true) + const isInitialRef = useRef(true) // Normalize the value to ensure that all subsequent uses are Day.js objects. const normalizedValue = useMemo(() => { @@ -62,46 +57,41 @@ const DatePicker = ({ const [selectedYear, setSelectedYear] = useState(() => (inputValue || defaultValue).year()) useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - if (containerRef.current && !containerRef.current.contains(event.target as Node)) { - setIsOpen(false) - setView(ViewType.date) - } - } - document.addEventListener('mousedown', handleClickOutside) - return () => document.removeEventListener('mousedown', handleClickOutside) - }, []) - - useEffect(() => { - if (isInitial.current) { - isInitial.current = false + if (isInitialRef.current) { + isInitialRef.current = false return } clearMonthMapCache() if (normalizedValue) { const newValue = getDateWithTimezone({ date: normalizedValue, timezone }) + // eslint-disable-next-line react/set-state-in-effect -- timezone changes intentionally resync the displayed calendar state. setCurrentDate(newValue) + // eslint-disable-next-line react/set-state-in-effect -- timezone changes intentionally resync the selected value. setSelectedDate(newValue) onChange(newValue) } else { + // eslint-disable-next-line react/set-state-in-effect -- timezone changes intentionally resync the displayed calendar state. setCurrentDate(prev => getDateWithTimezone({ date: prev, timezone })) + // eslint-disable-next-line react/set-state-in-effect -- timezone changes intentionally resync the selected value. setSelectedDate(prev => prev ? getDateWithTimezone({ date: prev, timezone }) : undefined) } + // eslint-disable-next-line react/exhaustive-deps -- this effect intentionally runs only when timezone changes. }, [timezone]) - const handleClickTrigger = (e: React.MouseEvent) => { - e.stopPropagation() - if (isOpen) { - setIsOpen(false) - return - } + const handleOpenChange = useCallback((nextOpen: boolean) => { + setIsOpen(nextOpen) setView(ViewType.date) - setIsOpen(true) - if (normalizedValue) { + if (nextOpen && normalizedValue) { setCurrentDate(normalizedValue) setSelectedDate(normalizedValue) } + }, [normalizedValue]) + + const handleClickTrigger = (e: React.MouseEvent) => { + e.preventDefault() + e.stopPropagation() + handleOpenChange(!isOpen) } const handleClear = (e: React.MouseEvent) => { @@ -210,21 +200,21 @@ const DatePicker = ({ const placeholderDate = isOpen && selectedDate ? selectedDate.format(timeFormat) : (placeholder || t('defaultPlaceholder', { ns: 'time' })) return ( - - - {renderTrigger - ? ( - renderTrigger({ - value: normalizedValue, - selectedDate, - isOpen, - handleClear, - handleClickTrigger, - })) +
)} - - + /> +
{/* Header */} {view === ViewType.date @@ -319,8 +314,8 @@ const DatePicker = ({ ) }
- - + + ) } diff --git a/web/app/components/base/date-and-time-picker/time-picker/__tests__/index.spec.tsx b/web/app/components/base/date-and-time-picker/time-picker/__tests__/index.spec.tsx index 7fbed3a736..0857bb35f8 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/__tests__/index.spec.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/__tests__/index.spec.tsx @@ -3,6 +3,15 @@ import { fireEvent, render, screen, within } from '@testing-library/react' import dayjs, { isDayjsObject } from '../../utils/dayjs' import TimePicker from '../index' +vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) +vi.mock('@langgenius/dify-ui/button', () => ({ + Button: ({ children, onClick, disabled, className }: Record) => ( + + ), +})) + // Mock scrollIntoView since the test DOM runtime doesn't implement it beforeAll(() => { Element.prototype.scrollIntoView = vi.fn() @@ -106,7 +115,7 @@ describe('TimePicker', () => { expect(input)!.toHaveValue('') fireEvent.mouseDown(document.body) - expect(input)!.toHaveValue('') + expect(input)!.toHaveValue('10:00 AM') }) it('should call onClear when clear is clicked while picker is closed', () => { diff --git a/web/app/components/base/date-and-time-picker/time-picker/index.tsx b/web/app/components/base/date-and-time-picker/time-picker/index.tsx index e07ea177a5..3fcb88215e 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/index.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/index.tsx @@ -1,14 +1,10 @@ import type { Dayjs } from 'dayjs' import type { TimePickerProps } from '../types' import { cn } from '@langgenius/dify-ui/cn' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import * as React from 'react' import { useCallback, useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import TimezoneLabel from '@/app/components/base/timezone-label' import { Period } from '../types' import dayjs, { @@ -43,31 +39,20 @@ const TimePicker = ({ }: TimePickerProps) => { const { t } = useTranslation() const [isOpen, setIsOpen] = useState(false) - const containerRef = useRef(null) - const isInitial = useRef(true) + const isInitialRef = useRef(true) // Initialize selectedTime const [selectedTime, setSelectedTime] = useState(() => { return toDayjs(value, { timezone }) }) - useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - /* v8 ignore next 2 -- outside-click closing is handled by PortalToFollowElem; this local ref guard is a defensive fallback. */ - if (containerRef.current && !containerRef.current.contains(event.target as Node)) - setIsOpen(false) - } - document.addEventListener('mousedown', handleClickOutside) - return () => document.removeEventListener('mousedown', handleClickOutside) - }, []) - // Track previous values to avoid unnecessary updates const prevValueRef = useRef(value) const prevTimezoneRef = useRef(timezone) useEffect(() => { - if (isInitial.current) { - isInitial.current = false + if (isInitialRef.current) { + isInitialRef.current = false // Save initial values on first render prevValueRef.current = value prevTimezoneRef.current = timezone @@ -91,6 +76,7 @@ const TimePicker = ({ if (!dayjsValue) return + // eslint-disable-next-line react/set-state-in-effect -- value/timezone changes intentionally resync the internal selected time. setSelectedTime(dayjsValue) if (timezoneChanged && !valueChanged) @@ -98,6 +84,7 @@ const TimePicker = ({ return } + // eslint-disable-next-line react/set-state-in-effect -- value/timezone changes intentionally resync the internal selected time. setSelectedTime((prev) => { if (!isDayjsObject(prev)) return undefined @@ -105,24 +92,30 @@ const TimePicker = ({ }) }, [timezone, value, onChange]) - const handleClickTrigger = (e: React.MouseEvent) => { - e.stopPropagation() - if (isOpen) { - setIsOpen(false) + const syncSelectedTimeFromValue = useCallback(() => { + if (!value) return - } - setIsOpen(true) - if (value) { - const dayjsValue = toDayjs(value, { timezone }) - const needsUpdate = dayjsValue && ( - !selectedTime - || !isDayjsObject(selectedTime) - || !dayjsValue.isSame(selectedTime, 'minute') - ) - if (needsUpdate) - setSelectedTime(dayjsValue) - } + const dayjsValue = toDayjs(value, { timezone }) + const needsUpdate = dayjsValue && ( + !selectedTime + || !isDayjsObject(selectedTime) + || !dayjsValue.isSame(selectedTime, 'minute') + ) + if (needsUpdate) + setSelectedTime(dayjsValue) + }, [selectedTime, timezone, value]) + + const handleOpenChange = useCallback((nextOpen: boolean) => { + setIsOpen(nextOpen) + if (nextOpen) + syncSelectedTimeFromValue() + }, [syncSelectedTimeFromValue]) + + const handleClickTrigger = (e: React.MouseEvent) => { + e.preventDefault() + e.stopPropagation() + handleOpenChange(!isOpen) } const handleClear = (e: React.MouseEvent) => { @@ -132,7 +125,7 @@ const TimePicker = ({ onClear() } - const handleTimeSelect = (hour: string, minute: string, period: Period) => { + const handleTimeSelect = useCallback((hour: string, minute: string, period: Period) => { const periodAdjustedHour = to24Hour(hour, period) const nextMinute = Number.parseInt(minute, 10) setSelectedTime((prev) => { @@ -145,7 +138,7 @@ const TimePicker = ({ .set('second', 0) .set('millisecond', 0) }) - } + }, [timezone]) const getSafeTimeObject = useCallback(() => { if (isDayjsObject(selectedTime)) @@ -156,17 +149,17 @@ const TimePicker = ({ const handleSelectHour = useCallback((hour: string) => { const time = getSafeTimeObject() handleTimeSelect(hour, time.minute().toString().padStart(2, '0'), time.format('A') as Period) - }, [getSafeTimeObject]) + }, [getSafeTimeObject, handleTimeSelect]) const handleSelectMinute = useCallback((minute: string) => { const time = getSafeTimeObject() handleTimeSelect(getHourIn12Hour(time).toString().padStart(2, '0'), minute, time.format('A') as Period) - }, [getSafeTimeObject]) + }, [getSafeTimeObject, handleTimeSelect]) const handleSelectPeriod = useCallback((period: Period) => { const time = getSafeTimeObject() handleTimeSelect(getHourIn12Hour(time).toString().padStart(2, '0'), time.minute().toString().padStart(2, '0'), period) - }, [getSafeTimeObject]) + }, [getSafeTimeObject, handleTimeSelect]) const handleSelectCurrentTime = useCallback(() => { const newDate = getDateWithTimezone({ timezone }) @@ -207,18 +200,19 @@ const TimePicker = ({ /> ) return ( - - - {renderTrigger - ? (renderTrigger({ + - + /> +
{/* Header */}
@@ -258,8 +257,8 @@ const TimePicker = ({ />
- - + + ) } diff --git a/web/app/components/base/prompt-editor/plugins/context-block/__tests__/component.spec.tsx b/web/app/components/base/prompt-editor/plugins/context-block/__tests__/component.spec.tsx index eb011af528..32c2e72f14 100644 --- a/web/app/components/base/prompt-editor/plugins/context-block/__tests__/component.spec.tsx +++ b/web/app/components/base/prompt-editor/plugins/context-block/__tests__/component.spec.tsx @@ -2,6 +2,9 @@ import { act, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { UPDATE_DATASETS_EVENT_EMITTER } from '../../../constants' import ContextBlockComponent from '../component' + +vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) + // Mock the hooks used by ContextBlockComponent const mockUseSelectOrDelete = vi.fn() const mockUseTrigger = vi.fn() diff --git a/web/app/components/base/prompt-editor/plugins/context-block/component.tsx b/web/app/components/base/prompt-editor/plugins/context-block/component.tsx index 35f6948e07..b430692b94 100644 --- a/web/app/components/base/prompt-editor/plugins/context-block/component.tsx +++ b/web/app/components/base/prompt-editor/plugins/context-block/component.tsx @@ -1,13 +1,9 @@ import type { FC } from 'react' import type { Dataset } from './index' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { useEventEmitterContextContext } from '@/context/event-emitter' import { UPDATE_DATASETS_EVENT_EMITTER } from '../../constants' import { useSelectOrDelete, useTrigger } from '../../hooks' @@ -20,6 +16,11 @@ type ContextBlockComponentProps = { canNotAddContext?: boolean } +type DatasetsEventPayload = { + type?: string + payload?: Dataset[] +} + const ContextBlockComponent: FC = ({ nodeKey, datasets = [], @@ -32,9 +33,9 @@ const ContextBlockComponent: FC = ({ const { eventEmitter } = useEventEmitterContextContext() const [localDatasets, setLocalDatasets] = useState(datasets) - eventEmitter?.useSubscription((v: any) => { - if (v?.type === UPDATE_DATASETS_EVENT_EMITTER) - setLocalDatasets(v.payload) + eventEmitter?.useSubscription((event?: DatasetsEventPayload) => { + if (event?.type === UPDATE_DATASETS_EVENT_EMITTER && event.payload) + setLocalDatasets(event.payload) }) return ( @@ -49,24 +50,29 @@ const ContextBlockComponent: FC = ({
{t('promptEditor.context.item.title', { ns: 'common' })}
{!canNotAddContext && ( - - -
- {localDatasets.length} -
- - + `} + > + {localDatasets.length} + + )} + /> +
@@ -95,8 +101,8 @@ const ContextBlockComponent: FC = ({ {t('promptEditor.context.modal.footer', { ns: 'common' })}
- - + + )}
diff --git a/web/app/components/base/prompt-editor/plugins/history-block/__tests__/component.spec.tsx b/web/app/components/base/prompt-editor/plugins/history-block/__tests__/component.spec.tsx index aa4f0a85ca..adad2a61ef 100644 --- a/web/app/components/base/prompt-editor/plugins/history-block/__tests__/component.spec.tsx +++ b/web/app/components/base/prompt-editor/plugins/history-block/__tests__/component.spec.tsx @@ -6,6 +6,8 @@ import { UPDATE_HISTORY_EVENT_EMITTER } from '../../../constants' import HistoryBlockComponent from '../component' import { DELETE_HISTORY_BLOCK_COMMAND } from '../index' +vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) + type HistoryEventPayload = { type?: string payload?: RoleName diff --git a/web/app/components/base/prompt-editor/plugins/history-block/component.tsx b/web/app/components/base/prompt-editor/plugins/history-block/component.tsx index 15ce102bd9..827becf857 100644 --- a/web/app/components/base/prompt-editor/plugins/history-block/component.tsx +++ b/web/app/components/base/prompt-editor/plugins/history-block/component.tsx @@ -1,16 +1,12 @@ import type { FC } from 'react' import type { RoleName } from './index' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { RiMoreFill, } from '@remixicon/react' import { useState } from 'react' import { useTranslation } from 'react-i18next' import { MessageClockCircle } from '@/app/components/base/icons/src/vender/solid/general' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { useEventEmitterContextContext } from '@/context/event-emitter' import { UPDATE_HISTORY_EVENT_EMITTER } from '../../constants' import { useSelectOrDelete, useTrigger } from '../../hooks' @@ -22,6 +18,11 @@ type HistoryBlockComponentProps = { onEditRole: () => void } +type HistoryEventPayload = { + type?: string + payload?: RoleName +} + const HistoryBlockComponent: FC = ({ nodeKey, roleName = { user: '', assistant: '' }, @@ -33,9 +34,9 @@ const HistoryBlockComponent: FC = ({ const { eventEmitter } = useEventEmitterContextContext() const [localRoleName, setLocalRoleName] = useState(roleName) - eventEmitter?.useSubscription((v: any) => { - if (v?.type === UPDATE_HISTORY_EVENT_EMITTER) - setLocalRoleName(v.payload) + eventEmitter?.useSubscription((event?: HistoryEventPayload) => { + if (event?.type === UPDATE_HISTORY_EVENT_EMITTER && event.payload) + setLocalRoleName(event.payload) }) return ( @@ -49,25 +50,29 @@ const HistoryBlockComponent: FC = ({ >
{t('promptEditor.history.item.title', { ns: 'common' })}
- - -
- -
- - + > + + + )} + /> +
{t('promptEditor.history.modal.title', { ns: 'common' })}
@@ -87,8 +92,8 @@ const HistoryBlockComponent: FC = ({ {t('promptEditor.history.modal.edit', { ns: 'common' })}
- - + + ) } diff --git a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/index.spec.tsx b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/index.spec.tsx index abb334b393..afd7c04ed1 100644 --- a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/index.spec.tsx +++ b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/index.spec.tsx @@ -6,6 +6,40 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import Publisher from '../index' import Popup from '../popup' +vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) +vi.mock('@langgenius/dify-ui/button', () => ({ + Button: ({ children, onClick, disabled, variant, className }: Record) => ( + + ), +})) +vi.mock('@langgenius/dify-ui/alert-dialog', () => ({ + AlertDialog: ({ children, open, onOpenChange }: { children: React.ReactNode, open?: boolean, onOpenChange?: (open: boolean) => void }) => ( + open + ? ( +
+ {children} + +
+ ) + : null + ), + AlertDialogActions: ({ children }: { children: React.ReactNode }) =>
{children}
, + AlertDialogCancelButton: ({ children }: { children: React.ReactNode }) => , + AlertDialogConfirmButton: ({ children, onClick, disabled }: Record) => , + AlertDialogContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + AlertDialogDescription: ({ children }: { children: React.ReactNode }) =>
{children}
, + AlertDialogTitle: ({ children }: { children: React.ReactNode }) =>
{children}
, +})) + const mockPush = vi.fn() vi.mock('@/next/navigation', () => ({ useParams: () => ({ datasetId: 'test-dataset-id' }), @@ -60,7 +94,8 @@ vi.mock('@/context/dataset-detail', () => ({ const mockSetShowPricingModal = vi.fn() vi.mock('@/context/modal-context', () => ({ - useModalContextSelector: () => mockSetShowPricingModal, + useModalContextSelector: (selector: (state: { setShowPricingModal: typeof mockSetShowPricingModal }) => T): T => + selector({ setShowPricingModal: mockSetShowPricingModal }), })) const mockIsAllowPublishAsCustomKnowledgePipelineTemplate = vi.fn(() => true) @@ -200,8 +235,7 @@ describe('publisher', () => { it('should render portal element in closed state by default', () => { renderWithQueryClient() - const trigger = screen.getByText('workflow.common.publish').closest('[data-state]') - expect(trigger).toHaveAttribute('data-state', 'closed') + expect(screen.getByTestId('popover')).toHaveAttribute('data-open', 'false') expect(screen.queryByText('workflow.common.publishUpdate')).not.toBeInTheDocument() }) @@ -277,6 +311,25 @@ describe('publisher', () => { expect(screen.getByText('workflow.common.publishUpdate')).toBeInTheDocument() }) }) + + it('should close the outer popover before opening publish-as follow-up flow', async () => { + mockPublishedAt.mockReturnValue(1700000000) + mockIsAllowPublishAsCustomKnowledgePipelineTemplate.mockReturnValue(false) + renderWithQueryClient() + + fireEvent.click(screen.getByText('workflow.common.publish')) + + await waitFor(() => { + expect(screen.getByText('pipeline.common.publishAs')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByText('pipeline.common.publishAs')) + + await waitFor(() => { + expect(screen.queryByText('pipeline.common.publishAs')).not.toBeInTheDocument() + }) + expect(mockSetShowPricingModal).toHaveBeenCalled() + }) }) }) @@ -688,7 +741,7 @@ describe('publisher', () => { expect(screen.getByText('pipeline.common.confirmPublish')).toBeInTheDocument() }) - fireEvent.click(screen.getByRole('button', { name: 'common.operation.cancel' })) + fireEvent.click(screen.getByTestId('alert-dialog-close')) await waitFor(() => { expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument() diff --git a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/popup.spec.tsx b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/popup.spec.tsx index 103ee53210..6cd00d93bc 100644 --- a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/popup.spec.tsx +++ b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/__tests__/popup.spec.tsx @@ -3,6 +3,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import Popup from '../popup' +vi.mock('@langgenius/dify-ui/alert-dialog', () => ({ + AlertDialog: ({ children, open, onOpenChange }: { children: React.ReactNode, open?: boolean, onOpenChange?: (open: boolean) => void }) => ( + open + ? ( +
+ {children} + +
+ ) + : null + ), + AlertDialogActions: ({ children }: { children: unknown }) =>
{children}
, + AlertDialogCancelButton: ({ children }: { children: unknown }) => , + AlertDialogConfirmButton: ({ children, onClick, disabled }: Record) => , + AlertDialogContent: ({ children }: { children: unknown }) =>
{children}
, + AlertDialogDescription: ({ children }: { children: unknown }) =>
{children}
, + AlertDialogTitle: ({ children }: { children: unknown }) =>
{children}
, +})) + const mockPublishWorkflow = vi.fn().mockResolvedValue({ created_at: '2024-01-01T00:00:00Z' }) const mockPublishAsCustomizedPipeline = vi.fn().mockResolvedValue({}) const toastMocks = vi.hoisted(() => ({ @@ -36,6 +57,8 @@ let mockPublishedAt: string | undefined = '2024-01-01T00:00:00Z' let mockDraftUpdatedAt: string | undefined = '2024-06-01T00:00:00Z' let mockPipelineId: string | undefined = 'pipeline-123' let mockIsAllowPublishAsCustom = true +const mockUseBoolean = vi.hoisted(() => vi.fn()) +const mockUseKeyPress = vi.hoisted(() => vi.fn()) vi.mock('@/next/navigation', () => ({ useParams: () => ({ datasetId: 'ds-123' }), useRouter: () => ({ push: mockPush }), @@ -48,14 +71,8 @@ vi.mock('@/next/link', () => ({ })) vi.mock('ahooks', () => ({ - useBoolean: (initial: boolean) => { - const state = { value: initial } - return [state.value, { - setFalse: vi.fn(), - setTrue: vi.fn(), - }] - }, - useKeyPress: vi.fn(), + useBoolean: (initial: boolean) => mockUseBoolean(initial), + useKeyPress: (...args: unknown[]) => mockUseKeyPress(...args), })) vi.mock('@/app/components/workflow/store', () => ({ @@ -126,7 +143,8 @@ vi.mock('@/context/i18n', () => ({ })) vi.mock('@/context/modal-context', () => ({ - useModalContextSelector: () => mockSetShowPricingModal, + useModalContextSelector: (selector: (state: { setShowPricingModal: typeof mockSetShowPricingModal }) => T) => + selector({ setShowPricingModal: mockSetShowPricingModal }), })) vi.mock('@/context/provider-context', () => ({ @@ -194,6 +212,11 @@ describe('Popup', () => { mockDraftUpdatedAt = '2024-06-01T00:00:00Z' mockPipelineId = 'pipeline-123' mockIsAllowPublishAsCustom = true + mockUseBoolean.mockImplementation((initial: boolean) => [initial, { + setFalse: vi.fn(), + setTrue: vi.fn(), + }]) + mockUseKeyPress.mockImplementation(() => {}) }) afterEach(() => { @@ -289,12 +312,61 @@ describe('Popup', () => { describe('Publish As Knowledge Pipeline', () => { it('should show pricing modal when not allowed', () => { mockIsAllowPublishAsCustom = false - render() + const onRequestClose = vi.fn() + render() fireEvent.click(screen.getByText('pipeline.common.publishAs')) + expect(onRequestClose).toHaveBeenCalledTimes(1) expect(mockSetShowPricingModal).toHaveBeenCalled() }) + + it('should request closing the outer popover before opening publish-as modal', () => { + const onRequestClose = vi.fn() + render() + + fireEvent.click(screen.getByText('pipeline.common.publishAs')) + + expect(onRequestClose).toHaveBeenCalledTimes(1) + }) + }) + + describe('Overlay cleanup', () => { + it('should close confirm dialog when alert dialog requests close', () => { + const hideConfirm = vi.fn() + mockUseBoolean + .mockImplementationOnce(() => [true, { setFalse: hideConfirm, setTrue: vi.fn() }]) + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + + render() + + fireEvent.click(screen.getByTestId('alert-dialog-close')) + + expect(hideConfirm).toHaveBeenCalledTimes(1) + }) + }) + + describe('Publish params', () => { + it('should publish as template with empty pipeline id fallback', async () => { + mockPipelineId = undefined + mockUseBoolean + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + .mockImplementationOnce(() => [true, { setFalse: vi.fn(), setTrue: vi.fn() }]) + .mockImplementationOnce((initial: boolean) => [initial, { setFalse: vi.fn(), setTrue: vi.fn() }]) + render() + + fireEvent.click(screen.getByTestId('publish-as-confirm')) + + expect(mockPublishAsCustomizedPipeline).toHaveBeenCalledWith({ + pipelineId: '', + name: 'My Pipeline', + icon_info: { icon_type: 'emoji' }, + description: 'desc', + }) + }) }) describe('Time formatting', () => { diff --git a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/index.tsx b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/index.tsx index 3ea9aa0c1f..d846d7d749 100644 --- a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/index.tsx +++ b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/index.tsx @@ -1,4 +1,5 @@ import { Button } from '@langgenius/dify-ui/button' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { RiArrowDownSLine } from '@remixicon/react' import { memo, @@ -6,11 +7,6 @@ import { useState, } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { useNodesSyncDraft } from '@/app/components/workflow/hooks' import Popup from './popup' @@ -26,28 +22,31 @@ const Publisher = () => { }, [handleSyncWorkflowDraft]) return ( - - handleOpenChange(!open)}> - - - - - - + + {t('common.publish', { ns: 'workflow' })} + + + )} + /> + + handleOpenChange(false)} /> + + ) } diff --git a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/popup.tsx b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/popup.tsx index 9cb026dffe..31f5957029 100644 --- a/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/popup.tsx +++ b/web/app/components/rag-pipeline/components/rag-pipeline-header/publisher/popup.tsx @@ -39,7 +39,11 @@ import { usePublishWorkflow } from '@/service/use-workflow' import PublishAsKnowledgePipelineModal from '../../publish-as-knowledge-pipeline-modal' const PUBLISH_SHORTCUT = ['ctrl', '⇧', 'P'] -const Popup = () => { +type PopupProps = { + onRequestClose?: () => void +} + +const Popup = ({ onRequestClose }: PopupProps) => { const { t } = useTranslation() const { datasetId } = useParams() const { push } = useRouter() @@ -70,6 +74,7 @@ const Popup = () => { const checked = await handleCheckBeforePublish() if (checked) { if (!publishedAt && !confirmVisible) { + onRequestClose?.() showConfirm() return } @@ -114,7 +119,7 @@ const Popup = () => { if (confirmVisible) hideConfirm() } - }, [publishing, handleCheckBeforePublish, publishedAt, confirmVisible, showPublishing, publishWorkflow, pipelineId, datasetId, showConfirm, t, workflowStore, mutateDatasetRes, invalidPublishedPipelineInfo, invalidDatasetList, hidePublishing, hideConfirm]) + }, [publishing, handleCheckBeforePublish, publishedAt, confirmVisible, showPublishing, publishWorkflow, pipelineId, datasetId, showConfirm, t, workflowStore, mutateDatasetRes, invalidPublishedPipelineInfo, invalidDatasetList, hidePublishing, hideConfirm, onRequestClose]) useKeyPress(`${getKeyboardKeyCodeBySystem('ctrl')}.shift.p`, (e) => { e.preventDefault() if (published) @@ -155,13 +160,14 @@ const Popup = () => { hidePublishingAsCustomizedPipeline() hidePublishAsKnowledgePipelineModal() } - }, [showPublishingAsCustomizedPipeline, publishAsCustomizedPipeline, pipelineId, t, invalidCustomizedTemplateList, hidePublishingAsCustomizedPipeline, hidePublishAsKnowledgePipelineModal]) + }, [showPublishingAsCustomizedPipeline, publishAsCustomizedPipeline, pipelineId, t, invalidCustomizedTemplateList, hidePublishingAsCustomizedPipeline, hidePublishAsKnowledgePipelineModal, docLink]) const handleClickPublishAsKnowledgePipeline = useCallback(() => { + onRequestClose?.() if (!isAllowPublishAsCustomKnowledgePipelineTemplate) setShowPricingModal() else setShowPublishAsKnowledgePipelineModal() - }, [isAllowPublishAsCustomKnowledgePipelineTemplate, setShowPublishAsKnowledgePipelineModal, setShowPricingModal]) + }, [isAllowPublishAsCustomKnowledgePipelineTemplate, onRequestClose, setShowPublishAsKnowledgePipelineModal, setShowPricingModal]) return (