From 594c21d98c9e43dc7b130efe95fdc8335f7c7fde Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:24:27 +0800 Subject: [PATCH] fix(web): split model selection and settings actions (#38797) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- eslint-suppressions.json | 4 +- .../__tests__/derive-trigger-status.spec.ts | 112 ------ .../__tests__/index.spec.tsx | 57 +++- .../__tests__/trigger.spec.tsx | 322 ------------------ .../derive-trigger-status.ts | 9 - .../model-parameter-modal/index.tsx | 87 +++-- .../model-parameter-modal/trigger.tsx | 138 -------- .../model-parameter-modal/types.ts | 13 + .../model-selector/__tests__/index.spec.tsx | 184 +++++----- .../model-selector/index.tsx | 105 +++--- 10 files changed, 273 insertions(+), 758 deletions(-) delete mode 100644 web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/derive-trigger-status.spec.ts delete mode 100644 web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/trigger.spec.tsx delete mode 100644 web/app/components/header/account-setting/model-provider-page/model-parameter-modal/derive-trigger-status.ts delete mode 100644 web/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger.tsx create mode 100644 web/app/components/header/account-setting/model-provider-page/model-parameter-modal/types.ts diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 27a40b17491..d767bfc4301 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3704,10 +3704,10 @@ }, "web/app/components/plugins/plugin-detail-panel/model-selector/__tests__/index.spec.tsx": { "jsx-a11y/click-events-have-key-events": { - "count": 3 + "count": 2 }, "jsx-a11y/no-static-element-interactions": { - "count": 3 + "count": 2 } }, "web/app/components/plugins/plugin-detail-panel/model-selector/index.tsx": { diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/derive-trigger-status.spec.ts b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/derive-trigger-status.spec.ts deleted file mode 100644 index 3186199524c..00000000000 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/derive-trigger-status.spec.ts +++ /dev/null @@ -1,112 +0,0 @@ -import type { ModelItem, ModelProvider } from '../../declarations' -import type { CredentialPanelState } from '../../provider-added-card/use-credential-panel-state' -import { ModelStatusEnum } from '../../declarations' -import { deriveTriggerStatus } from '../derive-trigger-status' - -const baseCredentialState: CredentialPanelState = { - variant: 'api-active', - priority: 'apiKey', - supportsCredits: true, - showPrioritySwitcher: true, - hasCredentials: true, - isCreditsExhausted: false, - credentialName: 'Primary Key', - credits: 10, -} - -const mockProvider = { provider: 'openai' } as ModelProvider -const mockModel = { model: 'gpt-4', status: ModelStatusEnum.active } as ModelItem - -describe('deriveTriggerStatus', () => { - it('returns empty when modelId is missing', () => { - expect(deriveTriggerStatus(undefined, 'openai', mockProvider, mockModel, baseCredentialState)).toBe('empty') - }) - - it('returns empty when providerName is missing', () => { - expect(deriveTriggerStatus('gpt-4', undefined, mockProvider, mockModel, baseCredentialState)).toBe('empty') - }) - - it('returns incompatible when provider plugin is not installed', () => { - expect(deriveTriggerStatus('gpt-4', 'openai', undefined, mockModel, baseCredentialState)).toBe('incompatible') - }) - - it('returns credits-exhausted when credits priority and exhausted', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - priority: 'credits', - isCreditsExhausted: true, - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, state)).toBe('credits-exhausted') - }) - - it('returns active when credits priority but not exhausted', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - priority: 'credits', - isCreditsExhausted: false, - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, state)).toBe('active') - }) - - it('returns api-key-unavailable when variant is api-unavailable', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - variant: 'api-unavailable', - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, state)).toBe('api-key-unavailable') - }) - - it('returns incompatible when currentModel is missing (deprecated)', () => { - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, undefined, baseCredentialState)).toBe('incompatible') - }) - - it('returns credits-exhausted when currentModel is missing and AI credits are exhausted without api key', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - priority: 'apiKey', - hasCredentials: false, - isCreditsExhausted: true, - credentialName: undefined, - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, undefined, state)).toBe('credits-exhausted') - }) - - it('returns configure-required when model status is no-configure', () => { - const model = { ...mockModel, status: ModelStatusEnum.noConfigure } as ModelItem - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, model, baseCredentialState)).toBe('configure-required') - }) - - it('returns incompatible when model status is noPermission', () => { - const model = { ...mockModel, status: ModelStatusEnum.noPermission } as ModelItem - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, model, baseCredentialState)).toBe('incompatible') - }) - - it('returns disabled when model status is disabled', () => { - const model = { ...mockModel, status: ModelStatusEnum.disabled } as ModelItem - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, model, baseCredentialState)).toBe('disabled') - }) - - it('returns active when all conditions are satisfied', () => { - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, baseCredentialState)).toBe('active') - }) - - it('prioritises credits-exhausted over api-unavailable', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - priority: 'credits', - isCreditsExhausted: true, - variant: 'api-unavailable', - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, state)).toBe('credits-exhausted') - }) - - it('does not return credits-exhausted when supportsCredits is false', () => { - const state: CredentialPanelState = { - ...baseCredentialState, - priority: 'credits', - isCreditsExhausted: true, - supportsCredits: false, - } - expect(deriveTriggerStatus('gpt-4', 'openai', mockProvider, mockModel, state)).toBe('active') - }) -}) diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/index.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/index.spec.tsx index b3f1ee9be13..fbb5e9336af 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/index.spec.tsx @@ -123,6 +123,7 @@ vi.mock('@/config', async (importOriginal) => { }) describe('ModelParameterModal', () => { + const openSettings = () => fireEvent.click(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })) const defaultProps = { isAdvancedMode: false, modelId: 'gpt-3.5-turbo', @@ -179,14 +180,42 @@ describe('ModelParameterModal', () => { it('should render trigger and open modal content when trigger is clicked', () => { render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.getByTestId('model-selector')).toBeInTheDocument() expect(screen.getByTestId('param-temperature')).toBeInTheDocument() }) + it('should keep model selection and model settings as separate actions', () => { + render() + + expect(screen.getByTestId('model-selector')).toBeInTheDocument() + expect(screen.queryByTestId('param-temperature')).not.toBeInTheDocument() + + fireEvent.click(screen.getByText('Select GPT-4.1')) + + expect(defaultProps.setModel).toHaveBeenCalledWith({ + modelId: 'gpt-4.1', + provider: 'openai', + mode: 'chat', + features: ['vision', 'tool-call'], + }) + expect(screen.queryByTestId('param-temperature')).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })) + + expect(screen.getByTestId('param-temperature')).toBeInTheDocument() + }) + + it('should disable model settings when no model is selected', () => { + render() + + expect(screen.getByTestId('model-selector')).toBeInTheDocument() + expect(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })).toBeDisabled() + }) + it('should call onCompletionParamsChange when parameter changes and switch actions happen', () => { render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() fireEvent.click(screen.getByText('Change')) expect(defaultProps.onCompletionParamsChange).toHaveBeenCalledWith({ @@ -206,7 +235,7 @@ describe('ModelParameterModal', () => { it('should call onCompletionParamsChange when preset is selected', () => { render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() fireEvent.click(screen.getByText('Preset 1')) expect(defaultProps.onCompletionParamsChange).toHaveBeenCalledWith({ ...defaultProps.completionParams, @@ -227,14 +256,14 @@ describe('ModelParameterModal', () => { ] render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.queryByText('Preset 1')).not.toBeInTheDocument() }) it('should call setModel when model selector picks another model', () => { render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() fireEvent.click(screen.getByText('Select GPT-4.1')) expect(defaultProps.setModel).toHaveBeenCalledWith({ @@ -247,7 +276,7 @@ describe('ModelParameterModal', () => { it('should toggle debug mode when debug footer is clicked', () => { render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() fireEvent.click(screen.getByText(/debugAsMultipleModel/i)) expect(defaultProps.onDebugWithMultipleModelChange).toHaveBeenCalled() }) @@ -256,7 +285,7 @@ describe('ModelParameterModal', () => { isRulesLoading = true isRulesPending = true render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.getByRole('status')).toBeInTheDocument() }) @@ -271,7 +300,7 @@ describe('ModelParameterModal', () => { modelId="" />, ) - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.queryByRole('status')).not.toBeInTheDocument() expect(screen.getByTestId('model-selector')).toBeInTheDocument() @@ -279,14 +308,14 @@ describe('ModelParameterModal', () => { it('should not open content when readonly is true', () => { render() - fireEvent.click(screen.getByText('Open Settings')) - expect(screen.queryByTestId('model-selector')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })).toBeDisabled() + expect(screen.queryByTestId('param-temperature')).not.toBeInTheDocument() }) it('should render no parameter items when rules are undefined', () => { parameterRules = undefined render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.queryByTestId('param-temperature')).not.toBeInTheDocument() expect(screen.getByTestId('model-selector')).toBeInTheDocument() }) @@ -304,7 +333,7 @@ describe('ModelParameterModal', () => { />, ) - fireEvent.click(screen.getByText('Open Settings')) + openSettings() const paramEl = screen.getByTestId('param-temperature') expect(paramEl).toHaveAttribute('data-has-nodes-output-vars', 'true') @@ -343,7 +372,7 @@ describe('ModelParameterModal', () => { />, ) - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.getByTestId('param-stop')).toBeInTheDocument() expect(screen.getByText(/debugAsSingleModel/i)).toBeInTheDocument() @@ -355,7 +384,7 @@ describe('ModelParameterModal', () => { isRulesPending = true render() - fireEvent.click(screen.getByText('Open Settings')) + openSettings() expect(screen.getByRole('status')).toBeInTheDocument() expect(screen.queryByTestId('param-temperature')).not.toBeInTheDocument() diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/trigger.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/trigger.spec.tsx deleted file mode 100644 index 7cf5cf75876..00000000000 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/__tests__/trigger.spec.tsx +++ /dev/null @@ -1,322 +0,0 @@ -import type { ComponentProps } from 'react' -import { render, screen } from '@testing-library/react' -import Trigger from '../trigger' - -const mockUseCredentialPanelState = vi.fn() - -vi.mock('../../hooks', () => ({ - useLanguage: () => 'en_US', -})) - -vi.mock('@/context/provider-context', () => ({ - useProviderContext: () => ({ - modelProviders: [{ - provider: 'openai', - label: { en_US: 'OpenAI', zh_Hans: 'OpenAI' }, - }], - }), -})) - -vi.mock('../../provider-added-card/use-credential-panel-state', () => ({ - useCredentialPanelState: () => mockUseCredentialPanelState(), -})) - -vi.mock('../../model-icon', () => ({ - default: () =>
Icon
, -})) - -vi.mock('../../model-name', () => ({ - default: ({ - modelItem, - showMode, - showFeatures, - }: { - modelItem: { model: string } - showMode?: boolean - showFeatures?: boolean - }) => ( -
- {modelItem.model} - {showMode && mode} - {showFeatures && features} -
- ), -})) - -const activeCredentialState = { - variant: 'api-active' as const, - supportsCredits: true, - isCreditsExhausted: false, - priority: 'apiKey' as const, - showPrioritySwitcher: true, - hasCredentials: true, - credentialName: 'Primary Key', - credits: 10, -} - -describe('Trigger', () => { - const currentProvider = { - provider: 'openai', - label: { en_US: 'OpenAI', zh_Hans: 'OpenAI' }, - } as unknown as ComponentProps['currentProvider'] - - const currentModel = { - model: 'gpt-4', - status: 'active', - } as unknown as ComponentProps['currentModel'] - - beforeEach(() => { - vi.clearAllMocks() - mockUseCredentialPanelState.mockReturnValue(activeCredentialState) - }) - - describe('Rendering', () => { - it('should render active state with model features in non-workflow mode', () => { - render( - , - ) - - expect(screen.getByText('gpt-4')).toBeInTheDocument() - expect(screen.getByTestId('model-icon')).toBeInTheDocument() - expect(screen.getByTestId('model-name-mode')).toBeInTheDocument() - expect(screen.getByTestId('model-name-features')).toBeInTheDocument() - }) - - it('should render fallback model id when current model is missing', () => { - render( - , - ) - - expect(screen.getByText('gpt-4')).toBeInTheDocument() - }) - - it('should render split layout with workflow styles when workflow mode is enabled', () => { - const { container } = render( - , - ) - - const leftPanel = container.querySelector('.rounded-l-lg') - expect(leftPanel).toBeInTheDocument() - expect(leftPanel).toHaveClass('border-workflow-block-parma-bg') - const rightPanel = container.querySelector('.rounded-r-lg') - expect(rightPanel).toBeInTheDocument() - expect(rightPanel).toHaveClass('border-workflow-block-parma-bg') - }) - - it('should render empty state when no provider or model is selected', () => { - render() - - expect(screen.getByText('workflow.errorMsg.configureModel')).toBeInTheDocument() - }) - - it('should render non-workflow empty state with warning border', () => { - const { container } = render() - - expect(screen.getByText('workflow.errorMsg.configureModel')).toBeInTheDocument() - expect(container.firstChild).toHaveClass('border-text-warning') - }) - }) - - describe('Status badges', () => { - it('should render credits exhausted badge in non-workflow mode', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'credits-exhausted', - isCreditsExhausted: true, - priority: 'credits', - }) - - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.creditsExhausted')).toBeInTheDocument() - expect(screen.queryByTestId('model-name-mode')).not.toBeInTheDocument() - expect(screen.queryByTestId('model-name-features')).not.toBeInTheDocument() - }) - - it('should render api unavailable badge in non-workflow mode', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'api-unavailable', - }) - - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.apiKeyUnavailable')).toBeInTheDocument() - }) - - it('should render credits exhausted badge in workflow mode', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'credits-exhausted', - isCreditsExhausted: true, - priority: 'credits', - }) - - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.creditsExhausted')).toBeInTheDocument() - }) - - it('should render api unavailable badge in workflow mode', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'api-unavailable', - }) - - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.apiKeyUnavailable')).toBeInTheDocument() - }) - - it('should render incompatible badge when model is deprecated (currentModel missing)', () => { - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.incompatible')).toBeInTheDocument() - }) - - it('should render credits exhausted badge when model is missing and AI credits are exhausted without api key', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'no-usage', - priority: 'apiKey', - hasCredentials: false, - isCreditsExhausted: true, - credentialName: undefined, - }) - - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.creditsExhausted')).toBeInTheDocument() - }) - - it('should render configure required badge when model status is no-configure', () => { - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.configureRequired')).toBeInTheDocument() - }) - - it('should render disabled badge when model status is disabled', () => { - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.disabled')).toBeInTheDocument() - }) - - it('should render incompatible badge when provider plugin is not installed', () => { - render( - , - ) - - expect(screen.getByText('common.modelProvider.selector.incompatible')).toBeInTheDocument() - }) - }) - - describe('Split layout', () => { - it('should use split layout with settings button in non-workflow mode', () => { - const { container } = render( - , - ) - - const splitContainer = container.querySelector('.rounded-l-lg') - expect(splitContainer).toBeInTheDocument() - const settingsButton = container.querySelector('.rounded-r-lg') - expect(settingsButton).toBeInTheDocument() - }) - - it('should use split layout for error states in non-workflow mode', () => { - mockUseCredentialPanelState.mockReturnValue({ - ...activeCredentialState, - variant: 'api-unavailable', - }) - - const { container } = render( - , - ) - - const splitContainer = container.querySelector('.rounded-l-lg') - expect(splitContainer).toBeInTheDocument() - }) - }) -}) diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/derive-trigger-status.ts b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/derive-trigger-status.ts deleted file mode 100644 index 08614604dd5..00000000000 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/derive-trigger-status.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { - DERIVED_MODEL_STATUS_BADGE_I18N, - DERIVED_MODEL_STATUS_TOOLTIP_I18N, - deriveModelStatus, -} from '../derive-model-status' - -export const deriveTriggerStatus = deriveModelStatus -export const TRIGGER_STATUS_BADGE_I18N = DERIVED_MODEL_STATUS_BADGE_I18N -export const TRIGGER_STATUS_TOOLTIP_I18N = DERIVED_MODEL_STATUS_TOOLTIP_I18N diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/index.tsx b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/index.tsx index dca1b31ff68..32d96053718 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/index.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/index.tsx @@ -8,7 +8,7 @@ import type { ModelParameterRule, } from '../declarations' import type { ParameterValue } from './parameter-item' -import type { TriggerProps } from './trigger' +import type { TriggerProps } from './types' import type { Node, NodeOutPutVar, @@ -20,7 +20,7 @@ import { PopoverContent, PopoverTrigger, } from '@langgenius/dify-ui/popover' -import { useMemo, useRef, useState } from 'react' +import { useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { ArrowNarrowLeft } from '@/app/components/base/icons/src/vender/line/arrows' import Loading from '@/app/components/base/loading' @@ -33,7 +33,6 @@ import ModelSelector from '../model-selector' import ParameterItem from './parameter-item' import PresetsParameter from './presets-parameter' import { getSupportedPresetConfig } from './presets-parameter-utils' -import Trigger from './trigger' export type ModelParameterModalProps = { popupClassName?: string @@ -73,7 +72,6 @@ const ModelParameterModal: FC = ({ }) => { const { t } = useTranslation() const [open, setOpen] = useState(false) - const settingsIconRef = useRef(null) const { data: parameterRulesData, isLoading, @@ -134,6 +132,8 @@ const ModelParameterModal: FC = ({ }) } + const hasSelectedModel = !!provider && !!modelId + return ( = ({ setOpen(newOpen) }} > - - { - renderTrigger - ? renderTrigger({ + {renderTrigger + ? ( + + {renderTrigger({ open, currentProvider, currentModel, providerName: provider, modelId, - }) - : ( - - ) - } - - )} - /> + })} + + )} + /> + ) + : ( +
+
+ +
+ $['modelProvider.modelSettings'], { ns: 'common' })} + disabled={readonly || !hasSelectedModel} + className={cn( + 'flex size-8 shrink-0 items-center justify-center rounded-l-none rounded-r-lg border-0 bg-components-button-tertiary-bg p-0 text-text-tertiary outline-hidden hover:bg-components-button-tertiary-bg-hover hover:text-text-secondary focus-visible:ring-2 focus-visible:ring-state-accent-solid disabled:cursor-not-allowed disabled:text-text-disabled', + isInWorkflow && 'border border-workflow-block-parma-bg bg-workflow-block-parma-bg hover:bg-workflow-block-parma-bg', + )} + > + + +
+ )}
@@ -184,17 +199,19 @@ const ModelParameterModal: FC = ({
-
- setOpen(false)} - /> -
+ {renderTrigger && ( +
+ setOpen(false)} + /> +
+ )} { !!parameterRules.length && ( -
+
{t($ => $['modelProvider.parameters'], { ns: 'common' })}
{ diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger.tsx b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger.tsx deleted file mode 100644 index d7baa0aa0c8..00000000000 --- a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger.tsx +++ /dev/null @@ -1,138 +0,0 @@ -import type { FC, Ref } from 'react' -import type { - Model, - ModelItem, - ModelProvider, -} from '../declarations' -import { cn } from '@langgenius/dify-ui/cn' -import { Tooltip, TooltipContent, TooltipTrigger } from '@langgenius/dify-ui/tooltip' -import { useTranslation } from 'react-i18next' -import { useProviderContext } from '@/context/provider-context' -import ModelIcon from '../model-icon' -import ModelName from '../model-name' -import { useCredentialPanelState } from '../provider-added-card/use-credential-panel-state' -import { - deriveTriggerStatus, - TRIGGER_STATUS_BADGE_I18N, - TRIGGER_STATUS_TOOLTIP_I18N, -} from './derive-trigger-status' - -export type TriggerProps = { - open?: boolean - currentProvider?: ModelProvider | Model - currentModel?: ModelItem - providerName?: string - modelId?: string - isInWorkflow?: boolean - settingsRef?: Ref -} - -const Trigger: FC = ({ - currentProvider, - currentModel, - providerName, - modelId, - isInWorkflow, - settingsRef, -}) => { - const { t } = useTranslation() - const { modelProviders } = useProviderContext() - const currentModelProvider = modelProviders.find(p => p.provider === providerName) - const credentialState = useCredentialPanelState(currentModelProvider) - const status = deriveTriggerStatus(modelId, providerName, currentModelProvider, currentModel, credentialState) - const badgeKey = TRIGGER_STATUS_BADGE_I18N[status as keyof typeof TRIGGER_STATUS_BADGE_I18N] - const tooltipKey = TRIGGER_STATUS_TOOLTIP_I18N[status as keyof typeof TRIGGER_STATUS_TOOLTIP_I18N] - const badgeLabel = badgeKey ? t($ => $[badgeKey], { ns: 'common' }) : null - const tooltipLabel = tooltipKey ? t($ => $[tooltipKey], { ns: 'common' }) : null - const isActive = status === 'active' - const iconProvider = currentProvider || modelProviders.find(item => item.provider === providerName) - - if (status === 'empty') { - return ( -
-
-
- -
-
-
- {t($ => $['errorMsg.configureModel'], { ns: 'workflow' })} -
- -
- ) - } - - return ( -
-
- -
- {currentModel - ? ( - - ) - :
{modelId}
} -
- {badgeKey && ( - tooltipLabel - ? ( - - -
- - - {badgeLabel} - -
-
- )} - /> - - {tooltipLabel} - - - ) - : ( -
-
- - - {badgeLabel} - -
-
- ) - )} - {!badgeKey && ( -
- -
- )} -
-
- -
-
- ) -} - -export default Trigger diff --git a/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/types.ts b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/types.ts new file mode 100644 index 00000000000..37aa56b2821 --- /dev/null +++ b/web/app/components/header/account-setting/model-provider-page/model-parameter-modal/types.ts @@ -0,0 +1,13 @@ +import type { + Model, + ModelItem, + ModelProvider, +} from '../declarations' + +export type TriggerProps = { + open?: boolean + currentProvider?: ModelProvider | Model + currentModel?: ModelItem + providerName?: string + modelId?: string +} diff --git a/web/app/components/plugins/plugin-detail-panel/model-selector/__tests__/index.spec.tsx b/web/app/components/plugins/plugin-detail-panel/model-selector/__tests__/index.spec.tsx index 00ed9eb4f1f..2dd3b6631c0 100644 --- a/web/app/components/plugins/plugin-detail-panel/model-selector/__tests__/index.spec.tsx +++ b/web/app/components/plugins/plugin-detail-panel/model-selector/__tests__/index.spec.tsx @@ -70,49 +70,41 @@ vi.mock('@/utils/completion-params', () => ({ // Mock child components vi.mock('@/app/components/header/account-setting/model-provider-page/model-selector', () => ({ - default: ({ defaultModel, modelList, scopeFeatures, onSelect }: { + default: ({ defaultModel, modelList, scopeFeatures, triggerClassName, readonly, onSelect }: { defaultModel?: { provider?: string, model?: string } modelList?: Model[] scopeFeatures?: string[] + triggerClassName?: string + readonly?: boolean onSelect?: (model: { provider: string, model: string }) => void - }) => ( -
onSelect?.({ provider: 'openai', model: 'gpt-4' })} - > - Model Selector -
- ), -})) - -vi.mock('@/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger', () => ({ - default: ({ currentProvider, currentModel, providerName, modelId, isInWorkflow }: { - currentProvider?: Model - currentModel?: ModelItem - providerName?: string - modelId?: string - isInWorkflow?: boolean }) => { - const hasDeprecated = !currentProvider || !currentModel + const currentProvider = modelList?.find(model => model.provider === defaultModel?.provider) + const currentModel = currentProvider?.models.find(model => model.model === defaultModel?.model) + const hasDeprecated = !!defaultModel && (!currentProvider || !currentModel) const modelDisabled = currentModel?.status !== ModelStatusEnum.active - const disabled = !mockProviderContextValue.isAPIKeySet || hasDeprecated || modelDisabled return (
- Trigger +
) }, @@ -255,6 +247,8 @@ const setupModelLists = (config: { // ==================== Tests ==================== describe('ModelParameterModal', () => { + const openSettings = () => fireEvent.click(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })) + beforeEach(() => { vi.clearAllMocks() mockProviderContextValue.isAPIKeySet = true @@ -287,6 +281,16 @@ describe('ModelParameterModal', () => { expect(screen.getByTestId('trigger')).toBeInTheDocument() }) + it('should keep model selection and model settings as separate actions', () => { + const props = createDefaultProps() + + render() + + expect(screen.getByTestId('model-selector')).toBeInTheDocument() + expect(screen.queryByTestId('llm-params-panel')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })).toBeDisabled() + }) + it('should render agent model trigger when isAgentStrategy is true', () => { // Arrange const props = createDefaultProps({ isAgentStrategy: true }) @@ -339,20 +343,25 @@ describe('ModelParameterModal', () => { render() // Assert - expect(screen.queryByTestId('model-selector')).not.toBeInTheDocument() + expect(screen.queryByTestId('llm-params-panel')).not.toBeInTheDocument() }) - it('should render model selector inside portal content when open', async () => { + it('should render model settings inside portal content when open', async () => { // Arrange - const props = createDefaultProps() + const model = createModel({ + provider: 'openai', + models: [createModelItem({ model: 'gpt-4' })], + }) + setupModelLists({ textGeneration: [model] }) + const props = createDefaultProps({ value: { provider: 'openai', model: 'gpt-4' } }) // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { - expect(screen.getByTestId('model-selector')).toBeInTheDocument() + expect(screen.getByTestId('llm-params-panel')).toBeInTheDocument() }) }) }) @@ -383,11 +392,19 @@ describe('ModelParameterModal', () => { it('should apply popupClassName to portal content', async () => { // Arrange - const props = createDefaultProps({ popupClassName: 'custom-popup-class' }) + const model = createModel({ + provider: 'openai', + models: [createModelItem({ model: 'gpt-4' })], + }) + setupModelLists({ textGeneration: [model] }) + const props = createDefaultProps({ + popupClassName: 'custom-popup-class', + value: { provider: 'openai', model: 'gpt-4' }, + }) // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -403,7 +420,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert const selector = screen.getByTestId('model-selector') @@ -413,19 +430,24 @@ describe('ModelParameterModal', () => { // ==================== State Management ==================== describe('State Management', () => { - it('should toggle open state when trigger is clicked', async () => { + it('should toggle model settings when the settings button is clicked', async () => { // Arrange - const props = createDefaultProps() + const model = createModel({ + provider: 'openai', + models: [createModelItem({ model: 'gpt-4' })], + }) + setupModelLists({ textGeneration: [model] }) + const props = createDefaultProps({ value: { provider: 'openai', model: 'gpt-4' } }) // Act render() - expect(screen.queryByTestId('model-selector')).not.toBeInTheDocument() + expect(screen.queryByTestId('llm-params-panel')).not.toBeInTheDocument() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { - expect(screen.getByTestId('model-selector')).toBeInTheDocument() + expect(screen.getByTestId('llm-params-panel')).toBeInTheDocument() }) }) @@ -434,16 +456,10 @@ describe('ModelParameterModal', () => { const props = createDefaultProps({ readonly: true }) // Act - const { rerender } = render() - expect(screen.queryByTestId('model-selector')).not.toBeInTheDocument() + render() - fireEvent.click(screen.getByTestId('trigger')) - - // Force a re-render to ensure state is stable - rerender() - - // Assert - open state should remain false due to readonly - expect(screen.queryByTestId('model-selector')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: /modelProvider\.modelSettings/i })).toBeDisabled() + expect(screen.queryByTestId('llm-params-panel')).not.toBeInTheDocument() }) }) @@ -455,7 +471,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -470,7 +486,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -493,7 +509,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -511,7 +527,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -528,7 +544,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -545,7 +561,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -562,7 +578,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -579,7 +595,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -596,7 +612,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -613,7 +629,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -735,7 +751,7 @@ describe('ModelParameterModal', () => { }) describe('Memoization - disabled', () => { - it('should set disabled to true when isAPIKeySet is false', () => { + it('should keep model selection available when isAPIKeySet is false', () => { // Arrange mockProviderContextValue.isAPIKeySet = false const model = createModel({ @@ -749,7 +765,7 @@ describe('ModelParameterModal', () => { render() // Assert - expect(screen.getByTestId('trigger')).toHaveAttribute('data-disabled', 'true') + expect(screen.getByTestId('trigger')).toHaveAttribute('data-disabled', 'false') }) it('should set disabled to true when hasDeprecated is true', () => { @@ -812,7 +828,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { fireEvent.click(screen.getByTestId('model-selector')) @@ -837,7 +853,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { fireEvent.click(screen.getByTestId('model-selector')) @@ -869,7 +885,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { fireEvent.click(screen.getByTestId('model-selector')) @@ -894,7 +910,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { fireEvent.click(screen.getByTestId('model-selector')) @@ -928,7 +944,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { const panel = screen.getByTestId('llm-params-panel') @@ -965,7 +981,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { const panel = screen.getByTestId('tts-params-panel') @@ -1002,7 +1018,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1028,7 +1044,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1054,7 +1070,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1063,7 +1079,7 @@ describe('ModelParameterModal', () => { expect(screen.queryByTestId('llm-params-panel')).not.toBeInTheDocument() }) - it('should render divider when model type is textGeneration or tts', async () => { + it('should not render a selector divider inside split model settings', async () => { // Arrange const textGenModel = createModel({ provider: 'openai', @@ -1081,11 +1097,11 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { - expect(document.querySelector('.bg-divider-subtle')).toBeInTheDocument() + expect(document.querySelector('.bg-divider-subtle')).not.toBeInTheDocument() }) }) }) @@ -1101,7 +1117,7 @@ describe('ModelParameterModal', () => { // Assert expect(screen.getByTestId('trigger')).toBeInTheDocument() - expect(screen.getByTestId('trigger')).toHaveAttribute('data-has-deprecated', 'true') + expect(screen.getByTestId('trigger')).toHaveAttribute('data-has-deprecated', 'false') }) it('should handle undefined value', () => { @@ -1122,7 +1138,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1161,7 +1177,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1240,7 +1256,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert await waitFor(() => { @@ -1256,7 +1272,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert - defaultModel is created with undefined provider await waitFor(() => { @@ -1273,7 +1289,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert - defaultModel is created with undefined model await waitFor(() => { @@ -1290,7 +1306,7 @@ describe('ModelParameterModal', () => { // Act render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() // Assert - when defaultModel is undefined, attribute is not set (returns null) await waitFor(() => { @@ -1326,7 +1342,7 @@ describe('ModelParameterModal', () => { // Act const { rerender } = render() - fireEvent.click(screen.getByTestId('trigger')) + openSettings() await waitFor(() => { expect(screen.getByTestId('model-selector')).toHaveAttribute('data-model-list-count', '1') @@ -1341,7 +1357,7 @@ describe('ModelParameterModal', () => { }) }) - it('should update disabled state when isAPIKeySet changes', () => { + it('should keep selector state independent from isAPIKeySet changes', () => { // Arrange const model = createModel({ provider: 'openai', @@ -1359,7 +1375,7 @@ describe('ModelParameterModal', () => { rerender() // Assert - expect(screen.getByTestId('trigger')).toHaveAttribute('data-disabled', 'true') + expect(screen.getByTestId('trigger')).toHaveAttribute('data-disabled', 'false') }) }) diff --git a/web/app/components/plugins/plugin-detail-panel/model-selector/index.tsx b/web/app/components/plugins/plugin-detail-panel/model-selector/index.tsx index 0b51c3c2398..9f7f33f2d6d 100644 --- a/web/app/components/plugins/plugin-detail-panel/model-selector/index.tsx +++ b/web/app/components/plugins/plugin-detail-panel/model-selector/index.tsx @@ -7,7 +7,7 @@ import type { FormValue, ModelFeatureEnum, } from '@/app/components/header/account-setting/model-provider-page/declarations' -import type { TriggerProps } from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger' +import type { TriggerProps } from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/types' import { cn } from '@langgenius/dify-ui/cn' import { Popover, @@ -22,7 +22,6 @@ import { useModelList, } from '@/app/components/header/account-setting/model-provider-page/hooks' import AgentModelTrigger from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/agent-model-trigger' -import Trigger from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/trigger' import ModelSelector from '@/app/components/header/account-setting/model-provider-page/model-selector' import { useProviderContext } from '@/context/provider-context' import { fetchAndMergeValidCompletionParams } from '@/utils/completion-params' @@ -174,6 +173,9 @@ const ModelParameterModal: FC = ({ }) } + const hasSelectedModel = !!value?.provider && !!value?.model + const isSplitTrigger = !renderTrigger && !isAgentStrategy + return ( = ({ }} >
- - { - renderTrigger - ? renderTrigger({ - open, - currentProvider, - currentModel, - providerName: value?.provider, - modelId: value?.model, - }) - : (isAgentStrategy - ? ( + {isSplitTrigger + ? ( +
+
+ +
+ $['modelProvider.modelSettings'], { ns: 'common' })} + disabled={readonly || !hasSelectedModel} + className={cn( + 'flex size-8 shrink-0 items-center justify-center rounded-l-none rounded-r-lg border-0 bg-components-button-tertiary-bg p-0 text-text-tertiary outline-hidden hover:bg-components-button-tertiary-bg-hover hover:text-text-secondary focus-visible:ring-2 focus-visible:ring-state-accent-solid disabled:cursor-not-allowed disabled:text-text-disabled', + isInWorkflow && 'border border-workflow-block-parma-bg bg-workflow-block-parma-bg hover:bg-workflow-block-parma-bg', + )} + > + + +
+ ) + : ( + + {renderTrigger + ? renderTrigger({ + open, + currentProvider, + currentModel, + providerName: value?.provider, + modelId: value?.model, + }) + : ( = ({ modelId={value?.model} scope={scope} /> - ) - : ( - - ) - ) - } - - )} - /> + )} + + )} + /> + )}
-
-
- {t($ => $['modelProvider.model'], { ns: 'common' }).toLocaleUpperCase()} + {!isSplitTrigger && ( +
+
+ {t($ => $['modelProvider.model'], { ns: 'common' }).toLocaleUpperCase()} +
+
- -
- {(currentModel?.model_type === ModelTypeEnum.textGeneration || currentModel?.model_type === ModelTypeEnum.tts) && ( + )} + {!isSplitTrigger && (currentModel?.model_type === ModelTypeEnum.textGeneration || currentModel?.model_type === ModelTypeEnum.tts) && (
)} {currentModel?.model_type === ModelTypeEnum.textGeneration && (