From ea47036a5dacc8d68b13a3efbb9a17621e140a62 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Tue, 28 Apr 2026 13:31:38 +0800 Subject: [PATCH] refactor(web): improve a11y and design-system consistency for date/time picker and auto-update strategy picker (#35627) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- eslint-suppressions.json | 5 - .../__tests__/option-list-item.spec.tsx | 10 +- .../common/__tests__/option-list.spec.tsx | 28 ++ .../common/option-list-item.tsx | 28 +- .../common/option-list.tsx | 26 ++ .../time-picker/__tests__/options.spec.tsx | 4 +- .../time-picker/options.tsx | 13 +- .../year-and-month-picker/options.tsx | 9 +- .../__tests__/index.spec.tsx | 242 +++++++----------- .../__tests__/strategy-picker.spec.tsx | 98 +++---- .../auto-update-setting/strategy-picker.tsx | 93 +++---- 11 files changed, 264 insertions(+), 292 deletions(-) create mode 100644 web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx create mode 100644 web/app/components/base/date-and-time-picker/common/option-list.tsx diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 3f3bd5f1f7..e1c8bda126 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3506,11 +3506,6 @@ "count": 1 } }, - "web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/plugins/reference-setting-modal/auto-update-setting/types.ts": { "erasable-syntax-only/enums": { "count": 2 diff --git a/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx b/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx index 8ccf8fab73..feca6d2e92 100644 --- a/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx +++ b/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx @@ -43,7 +43,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') expect(item).toHaveClass('bg-components-button-ghost-bg-hover') }) @@ -54,7 +54,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') expect(item).not.toHaveClass('bg-components-button-ghost-bg-hover') }) }) @@ -100,7 +100,7 @@ describe('OptionListItem', () => { Clickable , ) - fireEvent.click(screen.getByRole('listitem')) + fireEvent.click(screen.getByRole('button')) expect(handleClick).toHaveBeenCalledTimes(1) }) @@ -111,7 +111,7 @@ describe('OptionListItem', () => { Item , ) - fireEvent.click(screen.getByRole('listitem')) + fireEvent.click(screen.getByRole('button')) expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth' }) }) @@ -126,7 +126,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') fireEvent.click(item) fireEvent.click(item) fireEvent.click(item) diff --git a/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx b/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx new file mode 100644 index 0000000000..9e6a8cace9 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx @@ -0,0 +1,28 @@ +import { render, screen } from '@testing-library/react' +import OptionList from '../option-list' + +describe('OptionList', () => { + it('should render a scrollable list with hidden scrollbar styles', () => { + render( + +
  • Item
    • +     , + ) + + const list = screen.getByRole('list') + + expect(list).toHaveClass('overflow-y-auto') + expect(list).toHaveClass('[scrollbar-width:none]') + expect(list).toHaveClass('[&::-webkit-scrollbar]:hidden') + }) + + it('should append caller className after default classes', () => { + render( + +
  • Item
    • +     , + ) + + expect(screen.getByRole('list')).toHaveClass('custom-list') + }) +}) diff --git a/web/app/components/base/date-and-time-picker/common/option-list-item.tsx b/web/app/components/base/date-and-time-picker/common/option-list-item.tsx index 31b303df1f..e1bfdde4be 100644 --- a/web/app/components/base/date-and-time-picker/common/option-list-item.tsx +++ b/web/app/components/base/date-and-time-picker/common/option-list-item.tsx @@ -1,4 +1,4 @@ -import type { FC } from 'react' +import type { FC, ReactNode } from 'react' import { cn } from '@langgenius/dify-ui/cn' import * as React from 'react' import { useEffect, useRef } from 'react' @@ -7,7 +7,8 @@ type OptionListItemProps = { isSelected: boolean onClick: () => void noAutoScroll?: boolean -} & React.LiHTMLAttributes + children: ReactNode +} const OptionListItem: FC = ({ isSelected, @@ -25,16 +26,21 @@ const OptionListItem: FC = ({ return (
  • { - listItemRef.current?.scrollIntoView({ behavior: 'smooth' }) - onClick() - }} > - {children} +
    • ) } diff --git a/web/app/components/base/date-and-time-picker/common/option-list.tsx b/web/app/components/base/date-and-time-picker/common/option-list.tsx new file mode 100644 index 0000000000..8fc2407089 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/common/option-list.tsx @@ -0,0 +1,26 @@ +import type { HTMLAttributes, ReactNode } from 'react' +import { cn } from '@langgenius/dify-ui/cn' +import * as React from 'react' + +type OptionListProps = { + children: ReactNode +} & HTMLAttributes + +const optionListClassName = cn( + 'flex h-[208px] flex-col gap-y-0.5 overflow-y-auto pb-[184px]', + '[scrollbar-width:none] [&::-webkit-scrollbar]:hidden', +) + +const OptionList = ({ + children, + className, + ...props +}: OptionListProps) => { + return ( +
      + {children} +
    + ) +} + +export default React.memo(OptionList) diff --git a/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx b/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx index d7fa3be797..1bf3a52a8b 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx @@ -64,13 +64,13 @@ describe('TimePickerOptions', () => { it('should render selected hour in the list', () => { const props = createOptionsProps({ selectedTime: dayjs('2024-01-01 05:30:00') }) render() - const selectedHour = screen.getAllByRole('listitem').find(item => item.textContent === '05') + const selectedHour = screen.getAllByRole('button').find(item => item.textContent === '05') expect(selectedHour)!.toHaveClass('bg-components-button-ghost-bg-hover') }) it('should render selected minute in the list', () => { const props = createOptionsProps({ selectedTime: dayjs('2024-01-01 05:30:00') }) render() - const selectedMinute = screen.getAllByRole('listitem').find(item => item.textContent === '30') + const selectedMinute = screen.getAllByRole('button').find(item => item.textContent === '30') expect(selectedMinute)!.toHaveClass('bg-components-button-ghost-bg-hover') }) diff --git a/web/app/components/base/date-and-time-picker/time-picker/options.tsx b/web/app/components/base/date-and-time-picker/time-picker/options.tsx index 6f6e5e9c68..10e94f983d 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/options.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/options.tsx @@ -1,6 +1,7 @@ import type { FC } from 'react' import type { TimeOptionsProps } from '../types' import * as React from 'react' +import OptionList from '../common/option-list' import OptionListItem from '../common/option-list-item' import { useTimeOptions } from '../hooks' @@ -16,7 +17,7 @@ const Options: FC = ({ return (
    {/* Hour */} -
      + { hourOptions.map((hour) => { const isSelected = selectedTime?.format('hh') === hour @@ -31,9 +32,9 @@ const Options: FC = ({ ) }) } -
    + {/* Minute */} -
      + { (minuteFilter ? minuteFilter(minuteOptions) : minuteOptions).map((minute) => { const isSelected = selectedTime?.format('mm') === minute @@ -48,9 +49,9 @@ const Options: FC = ({ ) }) } -
    + {/* Period */} -
      + { periodOptions.map((period) => { const isSelected = selectedTime?.format('A') === period @@ -66,7 +67,7 @@ const Options: FC = ({ ) }) } -
    +
    ) } diff --git a/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx b/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx index 2288925579..2e162472f4 100644 --- a/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx +++ b/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx @@ -1,6 +1,7 @@ import type { FC } from 'react' import type { YearAndMonthPickerOptionsProps } from '../types' import * as React from 'react' +import OptionList from '../common/option-list' import OptionListItem from '../common/option-list-item' import { useMonths, useYearOptions } from '../hooks' @@ -16,7 +17,7 @@ const Options: FC = ({ return (
    {/* Month Picker */} -
      + { months.map((month, index) => { const isSelected = selectedMonth === index @@ -31,9 +32,9 @@ const Options: FC = ({ ) }) } -
    + {/* Year Picker */} -
      + { yearOptions.map((year) => { const isSelected = selectedYear === year @@ -48,7 +49,7 @@ const Options: FC = ({ ) }) } -
    +
    ) } diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx index ed3c457aaf..0ee9efc416 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx @@ -2,6 +2,7 @@ import type { AutoUpdateConfig } from '../types' import type { PluginDeclaration, PluginDetail } from '@/app/components/plugins/types' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { fireEvent, render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import dayjs from 'dayjs' import timezone from 'dayjs/plugin/timezone' import utc from 'dayjs/plugin/utc' @@ -803,165 +804,103 @@ describe('auto-update-setting', () => { }) describe('StrategyPicker (strategy-picker.tsx)', () => { - const defaultProps = { - value: AUTO_UPDATE_STRATEGY.disabled, - onChange: vi.fn(), + const i18nKeyByStrategy: Record = { + [AUTO_UPDATE_STRATEGY.disabled]: 'disabled', + [AUTO_UPDATE_STRATEGY.fixOnly]: 'fixOnly', + [AUTO_UPDATE_STRATEGY.latest]: 'latest', + } + + const triggerName = (strategy: AUTO_UPDATE_STRATEGY) => + new RegExp(`plugin\\.autoUpdate\\.strategy\\.${i18nKeyByStrategy[strategy]}\\.name`, 'i') + + const findOption = async (key: 'disabled' | 'fixOnly' | 'latest') => { + const options = await screen.findAllByRole('menuitemradio') + const option = options.find(item => + item.textContent?.includes(`plugin.autoUpdate.strategy.${key}.name`), + ) + if (!option) + throw new Error(`Strategy option "${key}" not found`) + return option } describe('Rendering', () => { it('should render trigger button with current strategy label', () => { - // Act - render() + render() - // Assert - expect(screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.disabled\.name/i })).toBeInTheDocument() + expect(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) })).toBeInTheDocument() }) it('should not render dropdown content when closed', () => { - // Act - render() + render() - // Assert - expect(screen.queryByTestId('portal-content')).not.toBeInTheDocument() + expect(screen.queryByRole('menu')).not.toBeInTheDocument() }) - it('should render all strategy options when open', () => { - // Arrange - mockPortalOpen = true + it('should render all strategy options when open', async () => { + const user = userEvent.setup() + render() - // Act - render() - fireEvent.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) })) - // Wait for portal to open - if (mockPortalOpen) { - // Assert all options visible (use getAllByText for strategy name as it appears in both trigger and dropdown) - expect(screen.getAllByText('plugin.autoUpdate.strategy.disabled.name').length).toBeGreaterThanOrEqual(1) - expect(screen.getByText('plugin.autoUpdate.strategy.fixOnly.name')).toBeInTheDocument() - expect(screen.getByText('plugin.autoUpdate.strategy.latest.name')).toBeInTheDocument() - } + const options = await screen.findAllByRole('menuitemradio') + expect(options).toHaveLength(3) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.disabled.name'))).toBe(true) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.fixOnly.name'))).toBe(true) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.latest.name'))).toBe(true) }) }) describe('User Interactions', () => { - it('should toggle dropdown when trigger is clicked', () => { - // Act - render() - - // Assert - initially closed - expect(mockPortalOpen).toBe(false) - - // Act - click trigger - fireEvent.click(screen.getByTestId('portal-trigger')) - - // Assert - portal trigger element should still be in document - expect(screen.getByTestId('portal-trigger')).toBeInTheDocument() - }) - - it('should call onChange with fixOnly when Bug Fixes Only option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Bug Fixes Only" option - const fixOnlyOption = screen.getByText('plugin.autoUpdate.strategy.fixOnly.name').closest('div[class*="cursor-pointer"]') - expect(fixOnlyOption).toBeInTheDocument() - fireEvent.click(fixOnlyOption!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.fixOnly) - }) - - it('should call onChange with latest when Latest Version option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Latest Version" option - const latestOption = screen.getByText('plugin.autoUpdate.strategy.latest.name').closest('div[class*="cursor-pointer"]') - expect(latestOption).toBeInTheDocument() - fireEvent.click(latestOption!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.latest) - }) - - it('should call onChange with disabled when Disabled option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Disabled" option - need to find the one in the dropdown, not the button - const disabledOptions = screen.getAllByText('plugin.autoUpdate.strategy.disabled.name') - // The second one should be in the dropdown - const dropdownOption = disabledOptions.find(el => el.closest('div[class*="cursor-pointer"]')) - expect(dropdownOption).toBeInTheDocument() - fireEvent.click(dropdownOption!.closest('div[class*="cursor-pointer"]')!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.disabled) - }) - - it('should stop event propagation when option is clicked', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - const onChange = vi.fn() - const parentClickHandler = vi.fn() - - // Act - render( -
    - -
    , - ) - - // Click an option - const fixOnlyOption = screen.getByText('plugin.autoUpdate.strategy.fixOnly.name').closest('div[class*="cursor-pointer"]') - fireEvent.click(fixOnlyOption!) - - // Assert - onChange is called but parent click handler should not propagate - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.fixOnly) - }) - - it('should render check icon for currently selected option', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - - // Act - render with fixOnly selected - render() - - // Assert - RiCheckLine should be rendered (check icon) - // Find all "Bug Fixes Only" texts and get the one in the dropdown (has cursor-pointer parent) - const allFixOnlyTexts = screen.getAllByText('plugin.autoUpdate.strategy.fixOnly.name') - const dropdownOption = allFixOnlyTexts.find(el => el.closest('div[class*="cursor-pointer"]')) - const optionContainer = dropdownOption?.closest('div[class*="cursor-pointer"]') - expect(optionContainer).toBeInTheDocument() - // The check icon SVG should exist within the option - expect(optionContainer?.querySelector('svg')).toBeInTheDocument() - }) - - it('should not render check icon for non-selected options', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - - // Act - render with disabled selected + it('should open and close the menu when the trigger is clicked', async () => { + const user = userEvent.setup() render() - // Assert - check the Latest Version option should not have check icon - const latestOption = screen.getByText('plugin.autoUpdate.strategy.latest.name').closest('div[class*="cursor-pointer"]') - // The svg should only be in selected option, not in non-selected - const checkIconContainer = latestOption?.querySelector('div.mr-1') - // Non-selected option should have empty check icon container - expect(checkIconContainer?.querySelector('svg')).toBeNull() + const trigger = screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) }) + expect(screen.queryByRole('menu')).not.toBeInTheDocument() + + await user.click(trigger) + expect(await screen.findByRole('menu')).toBeInTheDocument() + }) + + it.each<[AUTO_UPDATE_STRATEGY, 'disabled' | 'fixOnly' | 'latest', AUTO_UPDATE_STRATEGY]>([ + [AUTO_UPDATE_STRATEGY.disabled, 'fixOnly', AUTO_UPDATE_STRATEGY.fixOnly], + [AUTO_UPDATE_STRATEGY.disabled, 'latest', AUTO_UPDATE_STRATEGY.latest], + [AUTO_UPDATE_STRATEGY.fixOnly, 'disabled', AUTO_UPDATE_STRATEGY.disabled], + ])('should call onChange with %s -> %s when option is selected', async (initial, optionKey, expected) => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await user.click(screen.getByRole('button', { name: triggerName(initial) })) + await user.click(await findOption(optionKey)) + + expect(onChange).toHaveBeenCalledWith(expected) + }) + + it('should mark only the currently selected option with aria-checked', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.fixOnly) })) + + const options = await screen.findAllByRole('menuitemradio') + const checked = options.filter(o => o.getAttribute('aria-checked') === 'true') + + expect(checked).toHaveLength(1) + expect(checked[0]).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + }) + + it('should render the check indicator inside the selected option only', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.fixOnly) })) + + const fixOnlyOption = await findOption('fixOnly') + const latestOption = await findOption('latest') + + expect(fixOnlyOption.querySelector('.i-ri-check-line')).toBeInTheDocument() + expect(latestOption.querySelector('.i-ri-check-line')).toBeNull() }) }) }) @@ -1280,7 +1219,9 @@ describe('auto-update-setting', () => { render() // Assert - expect(screen.getByTestId('portal-elem')).toBeInTheDocument() + expect( + screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.fixOnly\.name/i }), + ).toBeInTheDocument() }) it('should show time picker when strategy is not disabled', () => { @@ -1407,16 +1348,27 @@ describe('auto-update-setting', () => { }) describe('User Interactions', () => { - it('should call onChange with updated strategy when strategy changes', () => { + it('should call onChange with updated strategy when strategy changes', async () => { // Arrange + const user = userEvent.setup() const onChange = vi.fn() - const payload = createMockAutoUpdateConfig() + const payload = createMockAutoUpdateConfig({ strategy_setting: AUTO_UPDATE_STRATEGY.fixOnly }) // Act render() - // Assert - component renders with strategy picker - expect(screen.getByTestId('portal-elem')).toBeInTheDocument() + await user.click( + screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.fixOnly\.name/i }), + ) + const latestOption = (await screen.findAllByRole('menuitemradio')).find(item => + item.textContent?.includes('plugin.autoUpdate.strategy.latest.name'), + )! + await user.click(latestOption) + + // Assert + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining({ strategy_setting: AUTO_UPDATE_STRATEGY.latest }), + ) }) it('should call onChange with updated time when time changes', () => { diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx index e287e48985..9fe089c34e 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx @@ -1,62 +1,12 @@ -import { fireEvent, render, screen } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { describe, expect, it, vi } from 'vitest' import StrategyPicker from '../strategy-picker' import { AUTO_UPDATE_STRATEGY } from '../types' -let portalOpen = false - -vi.mock('@langgenius/dify-ui/button', () => ({ - Button: ({ - children, - }: { - children: React.ReactNode - }) => {children}, -})) - -vi.mock('@/app/components/base/portal-to-follow-elem', async () => { - const _React = await import('react') - return { - PortalToFollowElem: ({ - open, - children, - }: { - open: boolean - children: React.ReactNode - }) => { - portalOpen = open - return
    {children}
    - }, - PortalToFollowElemTrigger: ({ - children, - onClick, - }: { - children: React.ReactNode - onClick: (event: { stopPropagation: () => void, nativeEvent: { stopImmediatePropagation: () => void } }) => void - }) => ( - - ), - PortalToFollowElemContent: ({ - children, - }: { - children: React.ReactNode - }) => portalOpen ?
    {children}
    : null, - } -}) +const triggerName = (key: string) => new RegExp(`plugin\\.autoUpdate\\.strategy\\.${key}\\.name`, 'i') describe('StrategyPicker', () => { - beforeEach(() => { - vi.clearAllMocks() - portalOpen = false - }) - it('renders the selected strategy label in the trigger', () => { render( { />, ) - expect(screen.getByTestId('trigger')).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + expect(screen.getByRole('button', { name: triggerName('fixOnly') })).toBeInTheDocument() + expect(screen.queryByRole('menu')).not.toBeInTheDocument() }) - it('opens the option list when the trigger is clicked', () => { + it('opens the option list when the trigger is clicked', async () => { + const user = userEvent.setup() render( { />, ) - fireEvent.click(screen.getByTestId('trigger')) + await user.click(screen.getByRole('button', { name: triggerName('disabled') })) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - expect(screen.getByTestId('portal-content').querySelectorAll('svg')).toHaveLength(1) + const options = await screen.findAllByRole('menuitemradio') + expect(options).toHaveLength(3) expect(screen.getByText('plugin.autoUpdate.strategy.latest.description')).toBeInTheDocument() }) - it('calls onChange when a new strategy is selected', () => { + it('marks only the currently selected strategy as checked', async () => { + const user = userEvent.setup() + render( + , + ) + + await user.click(screen.getByRole('button', { name: triggerName('fixOnly') })) + + const checkedOptions = (await screen.findAllByRole('menuitemradio')) + .filter(item => item.getAttribute('aria-checked') === 'true') + + expect(checkedOptions).toHaveLength(1) + expect(checkedOptions[0]).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + }) + + it('calls onChange and closes the menu when a new strategy is selected', async () => { + const user = userEvent.setup() const onChange = vi.fn() render( { />, ) - fireEvent.click(screen.getByTestId('trigger')) - fireEvent.click(screen.getByText('plugin.autoUpdate.strategy.latest.name')) + await user.click(screen.getByRole('button', { name: triggerName('disabled') })) + const latestOption = (await screen.findAllByRole('menuitemradio')) + .find(item => item.textContent?.includes('plugin.autoUpdate.strategy.latest.name'))! + await user.click(latestOption) expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.latest) + expect(await screen.findByRole('button', { name: triggerName('disabled') })).toBeInTheDocument() }) }) diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx index 22bfa6a30b..5247352f9b 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx @@ -1,15 +1,14 @@ import { Button } from '@langgenius/dify-ui/button' import { - RiArrowDownSLine, - RiCheckLine, -} from '@remixicon/react' + DropdownMenu, + DropdownMenuContent, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuRadioItemIndicator, + DropdownMenuTrigger, +} from '@langgenius/dify-ui/dropdown-menu' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { AUTO_UPDATE_STRATEGY } from './types' const i18nPrefix = 'autoUpdate.strategy' @@ -42,58 +41,48 @@ const StrategyPicker = ({ }, ] const selectedOption = options.find(option => option.value === value) + const handleValueChange = (nextValue: string) => { + onChange(nextValue as AUTO_UPDATE_STRATEGY) + setOpen(false) + } return ( - - { - e.stopPropagation() - e.nativeEvent.stopImmediatePropagation() - setOpen(v => !v) - }} + }> + {selectedOption?.label} + + + - - - -
    - { - options.map(option => ( -
    { - e.stopPropagation() - e.nativeEvent.stopImmediatePropagation() - onChange(option.value) - setOpen(false) - }} - > -
    - { - value === option.value && ( - - ) - } -
    -
    -
    {option.label}
    -
    {option.description}
    -
    + {options.map(option => ( + +
    +
    - )) - } -
    - - +
    +
    {option.label}
    +
    {option.description}
    +
    + + ))} + + + ) }