diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 27cc56c59c0..83f6ce67f84 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -2041,14 +2041,6 @@ "count": 2 } }, - "web/app/components/header/account-setting/model-provider-page/model-auth/authorized/credential-item.tsx": { - "jsx-a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx-a11y/no-static-element-interactions": { - "count": 1 - } - }, "web/app/components/header/account-setting/model-provider-page/model-auth/authorized/index.tsx": { "jsx-a11y/click-events-have-key-events": { "count": 1 @@ -2057,14 +2049,6 @@ "count": 1 } }, - "web/app/components/header/account-setting/model-provider-page/model-auth/credential-selector.tsx": { - "jsx-a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx-a11y/no-static-element-interactions": { - "count": 1 - } - }, "web/app/components/header/account-setting/model-provider-page/model-auth/hooks/index.ts": { "no-barrel-files/no-barrel-files": { "count": 6 @@ -2106,12 +2090,6 @@ } }, "web/app/components/header/account-setting/model-provider-page/provider-added-card/model-load-balancing-configs.tsx": { - "jsx-a11y/click-events-have-key-events": { - "count": 2 - }, - "jsx-a11y/no-static-element-interactions": { - "count": 2 - }, "typescript/no-explicit-any": { "count": 5 } @@ -2120,12 +2098,6 @@ "eslint-react/set-state-in-effect": { "count": 1 }, - "jsx-a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx-a11y/no-static-element-interactions": { - "count": 1 - }, "typescript/no-explicit-any": { "count": 3 } diff --git a/web/app/components/header/account-setting/model-provider-page/model-auth/__tests__/credential-selector.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-auth/__tests__/credential-selector.spec.tsx index 63ac5d84e59..568a87558fb 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-auth/__tests__/credential-selector.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-auth/__tests__/credential-selector.spec.tsx @@ -60,6 +60,9 @@ describe('CredentialSelector', () => { render() await user.click(screen.getByText(/modelProvider.auth.selectModelCredential/)) + expect( + screen.getByRole('dialog', { name: /modelProvider.auth.selectModelCredential/ }), + ).toBeInTheDocument() await user.click(screen.getByRole('button', { name: 'Key 2' })) expect(mockOnSelect).toHaveBeenCalledWith(mockCredentials[1]) @@ -80,6 +83,21 @@ describe('CredentialSelector', () => { ) }) + it('allows adding a credential with the keyboard', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByText(/modelProvider.auth.selectModelCredential/)) + const addButton = screen.getByRole('button', { + name: /modelProvider.auth.addNewModelCredential/, + }) + addButton.focus() + expect(addButton).toHaveFocus() + await user.keyboard('{Enter}') + + expect(mockOnSelect).toHaveBeenCalledWith(expect.objectContaining({ addNewCredential: true })) + }) + it('should not open options when disabled is true', async () => { const user = userEvent.setup() render() diff --git a/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/__tests__/credential-item.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/__tests__/credential-item.spec.tsx index bcbb5a04215..66050cb3c23 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/__tests__/credential-item.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/__tests__/credential-item.spec.tsx @@ -1,5 +1,6 @@ import type { Credential } from '../../../declarations' import { fireEvent, render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import CredentialItem from '../credential-item' vi.mock('@langgenius/dify-ui/status-dot', () => ({ @@ -31,6 +32,18 @@ describe('CredentialItem', () => { expect(screen.getByText('Enterprise')).toBeInTheDocument() }) + it('selects an enterprise credential when its badge is clicked', async () => { + const user = userEvent.setup() + const onItemClick = vi.fn() + const enterpriseCredential = { ...credential, from_enterprise: true } + + render() + + await user.click(screen.getByText('Enterprise')) + + expect(onItemClick).toHaveBeenCalledWith(enterpriseCredential) + }) + it('should call onItemClick when list item is clicked', () => { const onItemClick = vi.fn() @@ -41,6 +54,47 @@ describe('CredentialItem', () => { expect(onItemClick).toHaveBeenCalledWith(credential) }) + it('selects a credential with the keyboard and exposes its selected state', async () => { + const user = userEvent.setup() + const onItemClick = vi.fn() + + render( + , + ) + + const option = screen.getByRole('button', { name: 'Test API Key' }) + expect(option).toHaveAttribute('aria-pressed', 'true') + await user.tab() + expect(option).toHaveFocus() + await user.keyboard('{Enter}') + expect(onItemClick).toHaveBeenCalledWith(credential) + }) + + it('keeps edit and delete reachable by keyboard without hovering', async () => { + const user = userEvent.setup() + const onEdit = vi.fn() + const onDelete = vi.fn() + + render() + + await user.tab() + const editButton = screen.getByRole('button', { name: 'common.operation.edit' }) + expect(editButton).toHaveFocus() + await user.keyboard('{Enter}') + expect(onEdit).toHaveBeenCalledWith(credential) + + await user.tab() + const deleteButton = screen.getByRole('button', { name: 'common.operation.delete' }) + expect(deleteButton).toHaveFocus() + await user.keyboard(' ') + expect(onDelete).toHaveBeenCalledWith(credential) + }) + it('should not call onItemClick when credential is unavailable', () => { const onItemClick = vi.fn() @@ -86,7 +140,8 @@ describe('CredentialItem', () => { expect(onDelete).toHaveBeenCalledWith(credential) }) - it('should block delete action for the currently selected credential when delete is disabled', () => { + it('keeps the selected credential delete action focusable but disabled', async () => { + const user = userEvent.setup() const onDelete = vi.fn() render( @@ -103,7 +158,11 @@ describe('CredentialItem', () => { .getAllByRole('button') .find((b) => b.querySelector('.i-ri-delete-bin-line'))! - fireEvent.click(deleteButton) + expect(deleteButton).toHaveAttribute('aria-disabled', 'true') + await user.tab() + await user.tab() + expect(deleteButton).toHaveFocus() + await user.click(deleteButton) expect(onDelete).not.toHaveBeenCalled() }) @@ -146,6 +205,7 @@ describe('CredentialItem', () => { const deleteButton = screen .getAllByRole('button') .find((b) => b.querySelector('.i-ri-delete-bin-line'))! + expect(deleteButton).toBeDisabled() fireEvent.click(deleteButton) expect(onDelete).not.toHaveBeenCalled() diff --git a/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/credential-item.tsx b/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/credential-item.tsx index 14ba6ae3408..13a5fb436b5 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/credential-item.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-auth/authorized/credential-item.tsx @@ -43,47 +43,65 @@ const CredentialItem = ({ return disableDeleteButShowAction && selectedCredentialId === credential.credential_id }, [disableDeleteButShowAction, selectedCredentialId, credential.credential_id]) const isUnavailable = !!credential.not_allowed_to_use + const canSelect = !!onItemClick && !isUnavailable + + const content = ( + <> + + {showSelectedIcon && ( + + {selectedCredentialId === credential.credential_id && ( + + )} + + )} + + + {credential.credential_name} + + + {isUnavailable && ( + + {t(($) => $['modelProvider.card.unavailable'], { ns: 'modelProvider' })} + + )} + + ) const Item = (
{ - if (disabled || isUnavailable) return - onItemClick?.(credential) - }} > -
- {showSelectedIcon && ( -
- {selectedCredentialId === credential.credential_id && ( - - )} -
- )} - -
onItemClick?.(credential)} > - {credential.credential_name} -
-
- {credential.from_enterprise && Enterprise} - {isUnavailable && ( -
- {t(($) => $['modelProvider.card.unavailable'], { ns: 'modelProvider' })} -
+ {content} + {credential.from_enterprise && ( + + Enterprise + + )} + + ) : ( +
{content}
)} + {!canSelect && credential.from_enterprise && Enterprise} {showAction && !credential.from_enterprise && !isUnavailable && ( -
+
{!disableEdit && ( $['operation.delete'], { ns: 'common' })} className="hover:bg-transparent" + disabled={disabled || disableDeleteWhenSelected} + focusableWhenDisabled={!!disableDeleteWhenSelected && !disabled} onClick={(e) => { - if (disabled || disableDeleteWhenSelected) return e.stopPropagation() onDelete?.(credential) }} diff --git a/web/app/components/header/account-setting/model-provider-page/model-auth/credential-selector.tsx b/web/app/components/header/account-setting/model-provider-page/model-auth/credential-selector.tsx index 38d844a1f9e..9a5269359c0 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-auth/credential-selector.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-auth/credential-selector.tsx @@ -1,10 +1,9 @@ import type { Credential } from '@/app/components/header/account-setting/model-provider-page/declarations' -import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' +import { Popover, PopoverContent, PopoverTitle, PopoverTrigger } from '@langgenius/dify-ui/popover' import { StatusDot } from '@langgenius/dify-ui/status-dot' import { RiAddLine, RiArrowDownSLine } from '@remixicon/react' import { memo, useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' -import Badge from '@/app/components/base/badge' import CredentialItem from './authorized/credential-item' type CredentialSelectorProps = { @@ -43,28 +42,34 @@ const CredentialSelector = ({ return ( + )} diff --git a/web/app/components/header/account-setting/model-provider-page/model-modal/__tests__/index.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-modal/__tests__/index.spec.tsx index 08055240b9c..9fafb857421 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-modal/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-modal/__tests__/index.spec.tsx @@ -232,8 +232,8 @@ describe('ModelModal', () => { const predefined = renderModal() expect( - screen.getByText('modelProvider.modelProvider.auth.apiKeyModal.title'), - )!.toBeInTheDocument() + screen.getByRole('dialog', { name: 'modelProvider.modelProvider.auth.apiKeyModal.title' }), + ).toBeInTheDocument() expect( screen.getByText('modelProvider.modelProvider.auth.apiKeyModal.desc'), )!.toBeInTheDocument() diff --git a/web/app/components/header/account-setting/model-provider-page/model-modal/index.tsx b/web/app/components/header/account-setting/model-provider-page/model-modal/index.tsx index f0574f0caf3..5f9aa16426c 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-modal/index.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-modal/index.tsx @@ -15,7 +15,13 @@ import { AlertDialogTitle, } from '@langgenius/dify-ui/alert-dialog' import { Button } from '@langgenius/dify-ui/button' -import { Dialog, DialogClose, DialogContent } from '@langgenius/dify-ui/dialog' +import { + Dialog, + DialogClose, + DialogContent, + DialogDescription, + DialogTitle, +} from '@langgenius/dify-ui/dialog' import { IconButton } from '@langgenius/dify-ui/icon-button' import { memo, useCallback, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -206,15 +212,15 @@ const ModelModal: FC = ({ else label = t(($) => $['modelProvider.auth.addModelCredential'], { ns: 'modelProvider' }) } - return
{label}
+ return {label} }, [t, mode, credential]) const modalDesc = useMemo(() => { if (providerFormSchemaPredefined) { return ( -
+ {t(($) => $['modelProvider.auth.apiKeyModal.desc'], { ns: 'modelProvider' })} -
+ ) } diff --git a/web/app/components/header/account-setting/model-provider-page/model-name/index.tsx b/web/app/components/header/account-setting/model-provider-page/model-name/index.tsx index 9807acdc3ad..95747657f02 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-name/index.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-name/index.tsx @@ -9,6 +9,7 @@ import { modelTypeFormat, sizeFormat } from '../utils' type ModelNameProps = PropsWithChildren<{ modelItem?: ModelSelectorModel + nameId?: string className?: string nameClassName?: string showModelType?: boolean @@ -22,6 +23,7 @@ type ModelNameProps = PropsWithChildren<{ }> const ModelName: FC = ({ modelItem, + nameId, className, nameClassName, showModelType, @@ -47,7 +49,7 @@ const ModelName: FC = ({ className, )} > - + {label} diff --git a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-list-item.spec.tsx b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-list-item.spec.tsx index d33df01fdea..c212fef3a99 100644 --- a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-list-item.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-list-item.spec.tsx @@ -42,8 +42,17 @@ vi.mock('../../model-icon', () => ({ })) vi.mock('../../model-name', () => ({ - default: ({ children, nameClassName }: { children: React.ReactNode; nameClassName?: string }) => ( + default: ({ + children, + nameClassName, + nameId, + }: { + children: React.ReactNode + nameClassName?: string + nameId?: string + }) => (
+ gpt-4 {children}
), @@ -92,6 +101,7 @@ describe('ModelListItem', () => { }) expect(screen.getByTestId('model-icon')).toBeInTheDocument() expect(screen.getByTestId('model-name')).toBeInTheDocument() + expect(screen.getByRole('switch', { name: 'gpt-4' })).toBeInTheDocument() }) it('should disable an active model when switch is clicked', async () => { diff --git a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-configs.spec.tsx b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-configs.spec.tsx index 454d74db34c..d924b77fdb4 100644 --- a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-configs.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-configs.spec.tsx @@ -165,12 +165,14 @@ describe('ModelLoadBalancingConfigs', () => { expect(container.firstChild).toBeNull() }) - it('should enable load balancing by clicking the main panel when disabled and without switch', async () => { + it('should enable load balancing from the named mode button with the keyboard', async () => { const user = userEvent.setup() render() - const panel = screen.getByTestId('load-balancing-main-panel') - await user.click(panel) + const modeButton = screen.getByTestId('load-balancing-select-mode') + expect(modeButton).toHaveAccessibleName('modelProvider.modelProvider.loadBalancing') + modeButton.focus() + await user.keyboard('[Space]') expect(screen.getByText('Key 1')).toBeInTheDocument() }) @@ -178,8 +180,9 @@ describe('ModelLoadBalancingConfigs', () => { const user = userEvent.setup() render() - const removeBtn = screen.getByTestId('load-balancing-remove-cfg-1') - await user.click(removeBtn) + const removeBtn = screen.getByRole('button', { name: 'common.operation.remove Key 1' }) + removeBtn.focus() + await user.keyboard('{Enter}') expect(screen.queryByText('Key 1')).not.toBeInTheDocument() }) @@ -189,6 +192,7 @@ describe('ModelLoadBalancingConfigs', () => { render() const entrySwitch = screen.getByTestId('load-balancing-switch-cfg-1') + expect(screen.getByRole('switch', { name: 'Key 1' })).toBe(entrySwitch) await user.click(entrySwitch) // Internal state transitions are verified by successful interactions }) @@ -198,6 +202,9 @@ describe('ModelLoadBalancingConfigs', () => { render() const mainSwitch = screen.getByTestId('load-balancing-switch-main') + expect(screen.getByRole('switch', { name: 'modelProvider.modelProvider.loadBalancing' })).toBe( + mainSwitch, + ) await user.click(mainSwitch) // Check if description is still there (it should be) expect( @@ -475,14 +482,12 @@ describe('ModelLoadBalancingConfigs', () => { it('should not toggle load balancing when modelLoadBalancingEnabled=false and clicking panel to enable', async () => { // Arrange: load balancing not enabled in context, draftConfig.enabled=false (so panel is clickable) - const user = userEvent.setup() mockModelLoadBalancingEnabled = false render() // Act: clicking the panel calls toggleModalBalancing(true) // but (modelLoadBalancingEnabled || !enabled) = (false || false) = false → condition fails - const panel = screen.getByTestId('load-balancing-main-panel') - await user.click(panel) + expect(screen.queryByTestId('load-balancing-select-mode')).not.toBeInTheDocument() expect(screen.queryByText('Key 1')).not.toBeInTheDocument() }) diff --git a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-modal.spec.tsx b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-modal.spec.tsx index 3880d6d2a40..93ef2603728 100644 --- a/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-modal.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/provider-added-card/__tests__/model-load-balancing-modal.spec.tsx @@ -601,7 +601,7 @@ describe('ModelLoadBalancingModal', () => { }) }) - it('should toggle modal from enabled to disabled when clicking the card', async () => { + it('should toggle to model credentials from the named mode button with the keyboard', async () => { renderModal( { // draftConfig.enabled=true → title shows configLoadBalancing expect(screen.getByText(/modelProvider\.auth\.configLoadBalancing/))!.toBeInTheDocument() - // Clicking the card when enabled=true toggles to disabled - const card = screen - .getByText(/modelProvider\.auth\.providerManaged$/) - .closest('div[class]')! - .closest('div[class]')! - await user.click(card) + const modeButton = screen.getByRole('button', { + name: 'modelProvider.modelProvider.auth.providerManaged', + }) + modeButton.focus() + await user.keyboard('{Enter}') // After toggling, title should show configModel (disabled state) // After toggling, title should show configModel (disabled state) diff --git a/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/__tests__/index.spec.tsx b/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/__tests__/index.spec.tsx index 3e0bfdcb7ce..b22ff59c73e 100644 --- a/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/__tests__/index.spec.tsx @@ -277,6 +277,7 @@ describe('ModelAuthDropdown', () => { await waitFor(() => { expect(screen.getByText('Key 1')).toBeInTheDocument() }) + expect(screen.getByRole('dialog', { name: /config/i })).toBeInTheDocument() }) it('should load provider detail on first click and reuse it on reopen', async () => { diff --git a/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/index.tsx b/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/index.tsx index 4096c5bca2d..5e42c04f625 100644 --- a/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/index.tsx +++ b/web/app/components/header/account-setting/model-provider-page/provider-added-card/model-auth-dropdown/index.tsx @@ -2,7 +2,7 @@ import type { ModelProviderSummaryResponse } from '@dify/contracts/api/console/w import type { ModelProvider, PreferredProviderTypeEnum } from '../../declarations' import type { CredentialPanelState } from '../use-credential-panel-state' import { Button } from '@langgenius/dify-ui/button' -import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' +import { Popover, PopoverContent, PopoverTitle, PopoverTrigger } from '@langgenius/dify-ui/popover' import { memo, useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import { useLazyModelProviderDetail } from '../../hooks' @@ -74,6 +74,7 @@ function ModelAuthDropdown({ } /> + {buttonConfig.text} {currentProvider ? ( { const { t } = useTranslation(['modelProvider']) + const modelNameId = useId() const deploymentEdition = useAtomValue(deploymentEditionAtom) const { data: features } = useQuery( consoleQuery.features.get.queryOptions({ @@ -119,6 +120,7 @@ const ModelListItem = ({ - +
} /> @@ -171,6 +173,7 @@ const ModelListItem = ({ ) : ( canConfigureModels && (
toggleModalBalancing(true) : undefined} data-testid="load-balancing-main-panel" > + {!withSwitch && !draftConfig.enabled && modelLoadBalancingEnabled && ( +