From ad96501e091cb614b7dc287d761a7637989f5391 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Fri, 12 Jun 2026 18:31:33 +0800 Subject: [PATCH] fix(ui): keep loading buttons focusable (#37383) --- packages/dify-ui/README.md | 6 ++ .../src/button/__tests__/index.spec.tsx | 55 +++++++++++++++++-- packages/dify-ui/src/button/index.stories.tsx | 10 +++- packages/dify-ui/src/button/index.tsx | 2 + .../__tests__/app-info-modals.spec.tsx | 3 +- .../sidebar/__tests__/index.spec.tsx | 3 +- .../sidebar/__tests__/rename-modal.spec.tsx | 4 +- .../create/step-two/__tests__/index.spec.tsx | 5 +- .../website/base/__tests__/url-input.spec.tsx | 4 +- .../base/__tests__/url-input.spec.tsx | 4 +- .../base/options/__tests__/index.spec.tsx | 24 +++----- .../query-input/__tests__/index.spec.tsx | 4 +- .../settings/form/__tests__/index.spec.tsx | 4 +- .../explore/sidebar/__tests__/index.spec.tsx | 3 +- .../__tests__/compliance.spec.tsx | 5 +- .../__tests__/header-modals.spec.tsx | 3 +- .../plugin-item/__tests__/action.spec.tsx | 5 +- .../__tests__/index.spec.tsx | 20 ++----- .../__tests__/actions.spec.tsx | 3 +- .../__tests__/index.spec.tsx | 40 ++------------ .../__tests__/create-snippet-dialog.spec.tsx | 3 +- .../save-before-leaving-dialog.spec.tsx | 3 +- .../snippet-header/__tests__/index.spec.tsx | 3 +- .../dsl-export-confirm-modal.spec.tsx | 3 +- web/app/install/installForm.spec.tsx | 3 +- web/test/button.ts | 8 +++ 26 files changed, 130 insertions(+), 100 deletions(-) create mode 100644 web/test/button.ts diff --git a/packages/dify-ui/README.md b/packages/dify-ui/README.md index b392c5adecc..06db2d0f418 100644 --- a/packages/dify-ui/README.md +++ b/packages/dify-ui/README.md @@ -61,6 +61,12 @@ Utilities: - `./cn` — `clsx` + `tailwind-merge` wrapper. Use this for conditional class composition. - `./styles.css` — the one CSS entry that ships the design tokens, theme variables, and project utilities/components. Import it once from the app root. +## Button loading and disabled contract + +`Button` keeps normal `disabled` controls native-disabled by default so unavailable actions are removed from the keyboard focus order. + +When `loading` is true, `Button` defaults `focusableWhenDisabled` to true. Loading represents an action that has already been triggered and is temporarily pending, so the button remains focusable while Base UI still suppresses click, pointer, keyboard activation, and submit-button activation. Pass `focusableWhenDisabled={false}` only when a loading button should use native disabled behavior. + ## Segmented control contract `SegmentedControl` is Dify's design-system primitive for mode, filter, and view selection. It is built on Base UI `ToggleGroup` + `Toggle`, so use `Tabs` instead when the UI needs `tablist` / `tabpanel` semantics. diff --git a/packages/dify-ui/src/button/__tests__/index.spec.tsx b/packages/dify-ui/src/button/__tests__/index.spec.tsx index f3694c29af7..cb7dd4c9d9c 100644 --- a/packages/dify-ui/src/button/__tests__/index.spec.tsx +++ b/packages/dify-ui/src/button/__tests__/index.spec.tsx @@ -1,4 +1,6 @@ +import type { FormEvent } from 'react' import { render } from 'vitest-browser-react' +import { userEvent } from 'vitest/browser' import { Button } from '../index' const asHTMLElement = (element: HTMLElement | SVGElement) => element as HTMLElement @@ -106,9 +108,16 @@ describe('Button', () => { expect(screen.getByRole('button').element().querySelector('[aria-hidden="true"]')).not.toBeInTheDocument() }) - it('auto-disables when loading', async () => { + it('keeps loading buttons focusable by default', async () => { const screen = await render() - await expect.element(screen.getByRole('button')).toBeDisabled() + const button = screen.getByRole('button').element() + + expect(button).not.toHaveAttribute('disabled') + expect((button as HTMLButtonElement).disabled).toBe(false) + expect(button).toHaveAttribute('aria-disabled', 'true') + + asHTMLElement(button).focus() + expect(button).toHaveFocus() }) it('sets aria-busy when loading', async () => { @@ -128,10 +137,17 @@ describe('Button', () => { await expect.element(screen.getByRole('button')).toBeDisabled() }) - it('keeps focusable when loading with focusableWhenDisabled', async () => { - const screen = await render() + it('does not keep normal disabled buttons focusable by default', async () => { + const screen = await render() const button = screen.getByRole('button').element() - expect(button).toHaveAttribute('aria-disabled', 'true') + + expect(button).toBeDisabled() + expect(button).not.toHaveAttribute('aria-disabled') + }) + + it('allows loading focusability to be opted out', async () => { + const screen = await render() + await expect.element(screen.getByRole('button')).toBeDisabled() }) }) @@ -156,6 +172,35 @@ describe('Button', () => { asHTMLElement(screen.getByRole('button').element()).click() expect(onClick).not.toHaveBeenCalled() }) + + it('does not submit a form when a loading submit button is clicked', async () => { + const onSubmit = vi.fn((event: FormEvent) => event.preventDefault()) + const screen = await render( +
+ +
, + ) + + asHTMLElement(screen.getByRole('button', { name: 'Submit' }).element()).click() + + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('does not implicitly submit a form through a loading submit button', async () => { + const onSubmit = vi.fn((event: FormEvent) => event.preventDefault()) + const screen = await render( +
+ + + +
, + ) + + asHTMLElement(screen.getByRole('textbox', { name: 'Name' }).element()).focus() + await userEvent.keyboard('{Enter}') + + expect(onSubmit).not.toHaveBeenCalled() + }) }) describe('className merging', () => { diff --git a/packages/dify-ui/src/button/index.stories.tsx b/packages/dify-ui/src/button/index.stories.tsx index b70a76f431f..9540c2b18a8 100644 --- a/packages/dify-ui/src/button/index.stories.tsx +++ b/packages/dify-ui/src/button/index.stories.tsx @@ -11,6 +11,7 @@ const meta = { tags: ['autodocs'], argTypes: { loading: { control: 'boolean' }, + focusableWhenDisabled: { control: 'boolean' }, tone: { control: 'select', options: ['default', 'destructive'], @@ -90,6 +91,13 @@ export const Loading: Story = { loading: true, children: 'Loading Button', }, + parameters: { + docs: { + description: { + story: 'Loading buttons remain focusable by default so focus is not lost after activation. Pass `focusableWhenDisabled={false}` to opt out.', + }, + }, + }, } export const Destructive: Story = { @@ -105,7 +113,7 @@ export const WithIcon: Story = { variant: 'primary', children: ( <> - + Launch ), diff --git a/packages/dify-ui/src/button/index.tsx b/packages/dify-ui/src/button/index.tsx index 03e5c4a937a..a7f82fe4683 100644 --- a/packages/dify-ui/src/button/index.tsx +++ b/packages/dify-ui/src/button/index.tsx @@ -112,6 +112,7 @@ export function Button({ tone, loading, disabled, + focusableWhenDisabled, type = 'button', children, ...props @@ -121,6 +122,7 @@ export function Button({ type={type} className={cn(buttonVariants({ variant, size, tone, className }))} disabled={disabled || loading} + focusableWhenDisabled={focusableWhenDisabled ?? loading} aria-busy={loading || undefined} {...props} > diff --git a/web/app/components/app-sidebar/app-info/__tests__/app-info-modals.spec.tsx b/web/app/components/app-sidebar/app-info/__tests__/app-info-modals.spec.tsx index 218d4b94e62..ebe1dd51d33 100644 --- a/web/app/components/app-sidebar/app-info/__tests__/app-info-modals.spec.tsx +++ b/web/app/components/app-sidebar/app-info/__tests__/app-info-modals.spec.tsx @@ -2,6 +2,7 @@ import type { App, AppSSO } from '@/types/app' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' +import { expectLoadingButton } from '@/test/button' import { AppModeEnum } from '@/types/app' import AppInfoModals from '../app-info-modals' @@ -265,7 +266,7 @@ describe('AppInfoModals', () => { const firstClick = user.click(confirmButton) await waitFor(() => { - expect(confirmButton).toBeDisabled() + expectLoadingButton(confirmButton) expect(confirmButton).toHaveTextContent('common.operation.exporting') }) await user.click(confirmButton) diff --git a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/index.spec.tsx b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/index.spec.tsx index 19513deff1f..2c6454b461f 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/index.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/index.spec.tsx @@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event' import * as React from 'react' import * as ReactI18next from 'react-i18next' import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features' +import { expectLoadingButton } from '@/test/button' import { useChatWithHistoryContext } from '../../context' import Sidebar from '../index' import RenameModal from '../rename-modal' @@ -550,7 +551,7 @@ describe('Sidebar Index', () => { render() await user.click(screen.getByTestId('rename-1')) const saveButton = screen.getByText('common.operation.save').closest('button') - expect(saveButton).toBeDisabled() + expectLoadingButton(saveButton) }) it('should handle rename for different items', async () => { diff --git a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/rename-modal.spec.tsx b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/rename-modal.spec.tsx index 9c741beb0e2..8a8e53254c0 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/rename-modal.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/rename-modal.spec.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from 'react' import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as ReactI18next from 'react-i18next' +import { expectLoadingButton } from '@/test/button' import RenameModal from '../rename-modal' vi.mock('@langgenius/dify-ui/dialog', () => ({ @@ -72,8 +73,7 @@ describe('RenameModal', () => { it('shows loading state when saveLoading is true', () => { render() const saveButton = screen.getByRole('button', { name: 'common.operation.save' }) - expect(saveButton).toBeDisabled() - expect(saveButton).toHaveAttribute('aria-busy', 'true') + expectLoadingButton(saveButton) expect(saveButton.querySelector('.animate-spin')).toBeInTheDocument() }) diff --git a/web/app/components/datasets/create/step-two/__tests__/index.spec.tsx b/web/app/components/datasets/create/step-two/__tests__/index.spec.tsx index a5318430be3..a1610064181 100644 --- a/web/app/components/datasets/create/step-two/__tests__/index.spec.tsx +++ b/web/app/components/datasets/create/step-two/__tests__/index.spec.tsx @@ -13,6 +13,7 @@ import type { RetrievalConfig } from '@/types/app' import { act, cleanup, fireEvent, render, renderHook, screen } from '@testing-library/react' import { ConfigurationMethodEnum, ModelStatusEnum, ModelTypeEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { ChunkingMode, DataSourceType, ProcessMode } from '@/models/datasets' +import { expectLoadingButton } from '@/test/button' import { RETRIEVE_METHOD } from '@/types/app' import { PreviewPanel } from '../components/preview-panel' import { StepTwoFooter } from '../components/step-two-footer' @@ -1773,14 +1774,14 @@ describe('StepTwoFooter', () => { render() const nextButton = screen.getByText(/nextStep/i).closest('button') - expect(nextButton)!.toBeDisabled() + expectLoadingButton(nextButton) }) it('should show loading state on Save button when creating in setting mode', () => { render() const saveButton = screen.getByText(/save/i).closest('button') - expect(saveButton)!.toBeDisabled() + expectLoadingButton(saveButton) }) }) }) diff --git a/web/app/components/datasets/create/website/base/__tests__/url-input.spec.tsx b/web/app/components/datasets/create/website/base/__tests__/url-input.spec.tsx index 749d1110455..6ee8481de0a 100644 --- a/web/app/components/datasets/create/website/base/__tests__/url-input.spec.tsx +++ b/web/app/components/datasets/create/website/base/__tests__/url-input.spec.tsx @@ -1,6 +1,7 @@ import { fireEvent, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' // Component Imports (after mocks) @@ -51,8 +52,7 @@ describe('UrlInput', () => { it('should show loading state on button when running', () => { render() const button = screen.getByRole('button') - expect(button).toBeDisabled() - expect(button).toHaveAttribute('aria-busy', 'true') + expectLoadingButton(button) expect(button.querySelector('.animate-spin')).toBeInTheDocument() }) diff --git a/web/app/components/datasets/create/website/jina-reader/base/__tests__/url-input.spec.tsx b/web/app/components/datasets/create/website/jina-reader/base/__tests__/url-input.spec.tsx index 84fd00d2671..ee34f411d27 100644 --- a/web/app/components/datasets/create/website/jina-reader/base/__tests__/url-input.spec.tsx +++ b/web/app/components/datasets/create/website/jina-reader/base/__tests__/url-input.spec.tsx @@ -1,6 +1,7 @@ import { fireEvent, render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' // Component Imports (after mocks) @@ -49,8 +50,7 @@ describe('UrlInput (jina-reader)', () => { it('should show loading state on button when running', () => { render() const button = screen.getByRole('button') - expect(button).toBeDisabled() - expect(button).toHaveAttribute('aria-busy', 'true') + expectLoadingButton(button) expect(button.querySelector('.animate-spin')).toBeInTheDocument() }) diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/website-crawl/base/options/__tests__/index.spec.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/website-crawl/base/options/__tests__/index.spec.tsx index 2ab8ad6d4b3..c78739adbf4 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/website-crawl/base/options/__tests__/index.spec.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/website-crawl/base/options/__tests__/index.spec.tsx @@ -4,6 +4,7 @@ import * as React from 'react' import { BaseFieldType } from '@/app/components/base/form/form-scenarios/base/types' import { CrawlStep } from '@/models/datasets' import { PipelineInputVarType } from '@/models/pipeline' +import { expectLoadingButton } from '@/test/button' import Options from '../index' const { mockToastError } = vi.hoisted(() => ({ @@ -257,12 +258,12 @@ describe('Options', () => { expect(screen.getByText(/running/i)).toBeInTheDocument() }) - it('should disable button when step is running', () => { + it('should keep button loading-disabled when step is running', () => { const props = createDefaultProps({ step: CrawlStep.running }) render() - expect(screen.getByRole('button')).toBeDisabled() + expectLoadingButton(screen.getByRole('button')) }) it('should enable button when step is finished', () => { @@ -272,16 +273,6 @@ describe('Options', () => { expect(screen.getByRole('button')).not.toBeDisabled() }) - - it('should show loading state on button when step is running', () => { - const props = createDefaultProps({ step: CrawlStep.running }) - - render() - - // Assert - Button should have loading prop which disables it - const button = screen.getByRole('button') - expect(button).toBeDisabled() - }) }) describe('runDisabled prop', () => { @@ -306,7 +297,7 @@ describe('Options', () => { render() - expect(screen.getByRole('button')).toBeDisabled() + expectLoadingButton(screen.getByRole('button')) }) it('should default runDisabled to undefined (falsy)', () => { @@ -495,9 +486,8 @@ describe('Options', () => { render() - // Assert - Button should be in loading state const button = screen.getByRole('button') - expect(button).toBeDisabled() + expectLoadingButton(button) expect(screen.getByText(/running/i)).toBeInTheDocument() }) @@ -772,7 +762,9 @@ describe('Options', () => { render() const button = screen.getByRole('button') - if (expectedDisabled) + if (propVariation.step === CrawlStep.running) + expectLoadingButton(button) + else if (expectedDisabled) expect(button).toBeDisabled() else expect(button).not.toBeDisabled() diff --git a/web/app/components/datasets/hit-testing/components/query-input/__tests__/index.spec.tsx b/web/app/components/datasets/hit-testing/components/query-input/__tests__/index.spec.tsx index d9427f5117a..271db6bf127 100644 --- a/web/app/components/datasets/hit-testing/components/query-input/__tests__/index.spec.tsx +++ b/web/app/components/datasets/hit-testing/components/query-input/__tests__/index.spec.tsx @@ -3,6 +3,7 @@ import type { Query } from '@/models/datasets' import type { RetrievalConfig } from '@/types/app' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' import QueryInput from '../index' // Capture onChange callback so tests can trigger handleImageChange @@ -123,8 +124,7 @@ describe('QueryInput', () => { it('should show loading state on submit button when loading', () => { render() const submitButton = screen.getByRole('button', { name: /input\.testing/ }) - expect(submitButton)!.toBeDisabled() - expect(submitButton)!.toHaveAttribute('aria-busy', 'true') + expectLoadingButton(submitButton) expect(submitButton.querySelector('.animate-spin'))!.toBeInTheDocument() }) diff --git a/web/app/components/datasets/settings/form/__tests__/index.spec.tsx b/web/app/components/datasets/settings/form/__tests__/index.spec.tsx index 9d3545d019d..3e5ae330a22 100644 --- a/web/app/components/datasets/settings/form/__tests__/index.spec.tsx +++ b/web/app/components/datasets/settings/form/__tests__/index.spec.tsx @@ -2,6 +2,7 @@ import type { DataSet } from '@/models/datasets' import type { RetrievalConfig } from '@/types/app' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { ChunkingMode, DatasetPermission, DataSourceType } from '@/models/datasets' +import { expectLoadingButton } from '@/test/button' import { RETRIEVE_METHOD } from '@/types/app' import { IndexingType } from '../../../create/step-two' import Form from '../index' @@ -381,9 +382,8 @@ describe('Form', () => { const saveButton = screen.getByRole('button', { name: /form\.save/i }) fireEvent.click(saveButton) - // Button should be disabled during loading await waitFor(() => { - expect(saveButton).toBeDisabled() + expectLoadingButton(saveButton) }) }) diff --git a/web/app/components/explore/sidebar/__tests__/index.spec.tsx b/web/app/components/explore/sidebar/__tests__/index.spec.tsx index cce42d6322b..2bf21d8e1ad 100644 --- a/web/app/components/explore/sidebar/__tests__/index.spec.tsx +++ b/web/app/components/explore/sidebar/__tests__/index.spec.tsx @@ -1,6 +1,7 @@ import type { InstalledApp } from '@/models/explore' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { MediaType } from '@/hooks/use-breakpoints' +import { expectLoadingButton } from '@/test/button' import { AppModeEnum } from '@/types/app' import SideBar from '../index' @@ -224,7 +225,7 @@ describe('SideBar', () => { fireEvent.click(await screen.findByText('explore.sidebar.action.delete')) expect(screen.getByText('common.operation.cancel')).toBeDisabled() - expect(screen.getByText('common.operation.confirm')).toBeDisabled() + expectLoadingButton(screen.getByText('common.operation.confirm').closest('button')) }) }) diff --git a/web/app/components/header/account-dropdown/__tests__/compliance.spec.tsx b/web/app/components/header/account-dropdown/__tests__/compliance.spec.tsx index b4a700e22d1..e59c7378b9a 100644 --- a/web/app/components/header/account-dropdown/__tests__/compliance.spec.tsx +++ b/web/app/components/header/account-dropdown/__tests__/compliance.spec.tsx @@ -8,6 +8,7 @@ import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/con import { useModalContext } from '@/context/modal-context' import { baseProviderContextValue, useProviderContext } from '@/context/provider-context' import { getDocDownloadUrl } from '@/service/common' +import { expectLoadingButton } from '@/test/button' import { downloadUrl } from '@/utils/download' import Compliance from '../compliance' @@ -253,7 +254,7 @@ describe('Compliance', () => { await waitFor(() => { const busyButton = menuItem!.querySelector('button[aria-busy="true"]') expect(busyButton).not.toBeNull() - expect(busyButton)!.toBeDisabled() + expectLoadingButton(busyButton) expect(busyButton!.querySelector('.animate-spin')).not.toBeNull() }, { timeout: 10000 }) @@ -288,7 +289,7 @@ describe('Compliance', () => { await waitFor(() => { const busyButton = menuItem!.querySelector('button[aria-busy="true"]') expect(busyButton).not.toBeNull() - expect(busyButton)!.toBeDisabled() + expectLoadingButton(busyButton) expect(getDocDownloadUrl).toHaveBeenCalledTimes(1) }, { timeout: 10000 }) diff --git a/web/app/components/plugins/plugin-detail-panel/detail-header/components/__tests__/header-modals.spec.tsx b/web/app/components/plugins/plugin-detail-panel/detail-header/components/__tests__/header-modals.spec.tsx index 004e6b94f04..79511071ade 100644 --- a/web/app/components/plugins/plugin-detail-panel/detail-header/components/__tests__/header-modals.spec.tsx +++ b/web/app/components/plugins/plugin-detail-panel/detail-header/components/__tests__/header-modals.spec.tsx @@ -2,6 +2,7 @@ import type { PluginDetail } from '../../../../types' import type { ModalStates, VersionTarget } from '../../hooks' import { fireEvent, render, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' import { PluginSource } from '../../../../types' import HeaderModals from '../header-modals' @@ -301,7 +302,7 @@ describe('HeaderModals', () => { />, ) - expect(screen.getByRole('button', { name: /common\.operation\.confirm/ })).toBeDisabled() + expectLoadingButton(screen.getByRole('button', { name: /common\.operation\.confirm/ })) }) }) diff --git a/web/app/components/plugins/plugin-item/__tests__/action.spec.tsx b/web/app/components/plugins/plugin-item/__tests__/action.spec.tsx index 32e2e8065ce..8a0e11ffc7d 100644 --- a/web/app/components/plugins/plugin-item/__tests__/action.spec.tsx +++ b/web/app/components/plugins/plugin-item/__tests__/action.spec.tsx @@ -1,6 +1,7 @@ import type { MetaData, PluginCategoryEnum } from '../../types' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' // ==================== Imports (after mocks) ==================== @@ -408,7 +409,7 @@ describe('Action Component', () => { // Assert - Loading state await waitFor(() => { - expect(getDeleteConfirmButton())!.toBeDisabled() + expectLoadingButton(getDeleteConfirmButton()) }) // Resolve and check modal closes @@ -865,7 +866,7 @@ describe('Action Component', () => { // The confirm button should be disabled during deletion // The confirm button should be disabled during deletion - expect(getDeleteConfirmButton())!.toBeDisabled() + expectLoadingButton(getDeleteConfirmButton()) // Resolve the deletion resolveFirst!({ success: true }) diff --git a/web/app/components/plugins/plugin-mutation-model/__tests__/index.spec.tsx b/web/app/components/plugins/plugin-mutation-model/__tests__/index.spec.tsx index d36cf12f11f..9508e97f951 100644 --- a/web/app/components/plugins/plugin-mutation-model/__tests__/index.spec.tsx +++ b/web/app/components/plugins/plugin-mutation-model/__tests__/index.spec.tsx @@ -2,6 +2,7 @@ import type { Plugin } from '../../types' import { fireEvent, render, screen } from '@testing-library/react' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { expectLoadingButton } from '@/test/button' import { PluginCategoryEnum } from '../../types' import PluginMutationModal from '../index' @@ -433,13 +434,11 @@ describe('PluginMutationModal', () => { render() const confirmButton = screen.getByRole('button', { name: /Confirm/i }) - expect(confirmButton).toBeDisabled() + expectLoadingButton(confirmButton) fireEvent.click(confirmButton) - // Button is disabled, so mutate might still be called depending on implementation - // The important thing is the button has disabled attribute - expect(confirmButton).toHaveAttribute('disabled') + expect(mutate).not.toHaveBeenCalled() }) }) @@ -468,18 +467,7 @@ describe('PluginMutationModal', () => { render() const confirmButton = screen.getByRole('button', { name: /Confirm/i }) - expect(confirmButton).toBeDisabled() - }) - - it('should disable confirm button', () => { - const props = createDefaultProps({ - mutation: createMockMutation({ isPending: true }), - }) - - render() - - const confirmButton = screen.getByRole('button', { name: /Confirm/i }) - expect(confirmButton).toBeDisabled() + expectLoadingButton(confirmButton) }) }) diff --git a/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/actions.spec.tsx b/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/actions.spec.tsx index 69f576eae79..4389aa1c8c8 100644 --- a/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/actions.spec.tsx +++ b/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/actions.spec.tsx @@ -1,6 +1,7 @@ import type { CustomActionsProps } from '@/app/components/base/form/components/form/actions' import { fireEvent, render, screen } from '@testing-library/react' import { WorkflowRunningStatus } from '@/app/components/workflow/types' +import { expectLoadingButton } from '@/test/button' import Actions from '../actions' let mockWorkflowRunningData: { result: { status: WorkflowRunningStatus } } | undefined @@ -62,6 +63,6 @@ describe('Document processing actions', () => { />, ) - expect(screen.getByRole('button', { name: /datasetPipeline\.operations\.process/i })).toBeDisabled() + expectLoadingButton(screen.getByRole('button', { name: /datasetPipeline\.operations\.process/i })) }) }) diff --git a/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/index.spec.tsx b/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/index.spec.tsx index 3eae324ef47..e0ca1b2cc4c 100644 --- a/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/index.spec.tsx +++ b/web/app/components/rag-pipeline/components/panel/test-run/preparation/document-processing/__tests__/index.spec.tsx @@ -8,6 +8,7 @@ import * as React from 'react' import { BaseFieldType } from '@/app/components/base/form/form-scenarios/base/types' import { WorkflowRunningStatus } from '@/app/components/workflow/types' import { PipelineInputVarType } from '@/models/pipeline' +import { expectLoadingButton } from '@/test/button' import Actions from '../actions' import DocumentProcessing from '../index' import Options from '../options' @@ -566,7 +567,7 @@ describe('Actions', () => { ) const processButton = screen.getByText('datasetPipeline.operations.process').closest('button') - expect(processButton).toBeDisabled() + expectLoadingButton(processButton) }) it('should disable process button when canSubmit is false', () => { @@ -597,7 +598,7 @@ describe('Actions', () => { ) const processButton = screen.getByText('datasetPipeline.operations.process').closest('button') - expect(processButton).toBeDisabled() + expectLoadingButton(processButton) }) it('should enable process button when all conditions are met', () => { @@ -653,39 +654,6 @@ describe('Actions', () => { }) }) - describe('Loading State', () => { - it('should show loading state when isSubmitting', () => { - const mockFormParams = createMockFormParams({ isSubmitting: true }) - - render( - , - ) - - const processButton = screen.getByText('datasetPipeline.operations.process').closest('button') - expect(processButton).toBeDisabled() - }) - - it('should show loading state when workflow is running', () => { - mockWorkflowRunningData = { - result: { status: WorkflowRunningStatus.Running }, - } - const mockFormParams = createMockFormParams() - - render( - , - ) - - const processButton = screen.getByText('datasetPipeline.operations.process').closest('button') - expect(processButton).toBeDisabled() - }) - }) - describe('Edge Cases', () => { it('should handle undefined runDisabled prop', () => { const mockFormParams = createMockFormParams() @@ -1297,7 +1265,7 @@ describe('DocumentProcessing Integration', () => { ) const processButton = screen.getByText('datasetPipeline.operations.process').closest('button') - expect(processButton).toBeDisabled() + expectLoadingButton(processButton) }) it('should update when fetching params status changes', () => { diff --git a/web/app/components/snippets/__tests__/create-snippet-dialog.spec.tsx b/web/app/components/snippets/__tests__/create-snippet-dialog.spec.tsx index a947e7dc4ab..c3c65fb3c61 100644 --- a/web/app/components/snippets/__tests__/create-snippet-dialog.spec.tsx +++ b/web/app/components/snippets/__tests__/create-snippet-dialog.spec.tsx @@ -2,6 +2,7 @@ import type { SnippetCanvasData, SnippetInputField } from '@/models/snippet' import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { PipelineInputVarType } from '@/models/pipeline' +import { expectLoadingButton } from '@/test/button' import CreateSnippetDialog from '../create-snippet-dialog' let capturedKeyPressHandler: (() => void) | undefined @@ -182,6 +183,6 @@ describe('CreateSnippetDialog', () => { expect(screen.getByPlaceholderText('workflow.snippet.namePlaceholder')).toBeDisabled() expect(screen.getByPlaceholderText('workflow.snippet.descriptionPlaceholder')).toBeDisabled() expect(screen.getByRole('button', { name: 'common.operation.cancel' })).toBeDisabled() - expect(screen.getByRole('button', { name: 'workflow.snippet.confirm' })).toBeDisabled() + expectLoadingButton(screen.getByRole('button', { name: 'workflow.snippet.confirm' })) }) }) diff --git a/web/app/components/snippets/components/__tests__/save-before-leaving-dialog.spec.tsx b/web/app/components/snippets/components/__tests__/save-before-leaving-dialog.spec.tsx index 2db550d2a51..fcff9180549 100644 --- a/web/app/components/snippets/components/__tests__/save-before-leaving-dialog.spec.tsx +++ b/web/app/components/snippets/components/__tests__/save-before-leaving-dialog.spec.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { expectLoadingButton } from '@/test/button' import SaveBeforeLeavingDialog from '../save-before-leaving-dialog' describe('SaveBeforeLeavingDialog', () => { @@ -40,6 +41,6 @@ describe('SaveBeforeLeavingDialog', () => { expect(screen.getByRole('button', { name: 'snippet.continueEditing' })).toBeDisabled() expect(screen.getByRole('button', { name: 'snippet.doNotSave' })).toBeDisabled() - expect(screen.getByRole('button', { name: 'snippet.saveAndExit' })).toBeDisabled() + expectLoadingButton(screen.getByRole('button', { name: 'snippet.saveAndExit' })) }) }) diff --git a/web/app/components/snippets/components/snippet-header/__tests__/index.spec.tsx b/web/app/components/snippets/components/snippet-header/__tests__/index.spec.tsx index 7ae6f4af181..b099d095267 100644 --- a/web/app/components/snippets/components/snippet-header/__tests__/index.spec.tsx +++ b/web/app/components/snippets/components/snippet-header/__tests__/index.spec.tsx @@ -1,6 +1,7 @@ import type { ReactNode } from 'react' import type { HeaderProps } from '@/app/components/workflow/header' import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { expectLoadingButton } from '@/test/button' import SnippetHeader from '..' vi.mock('@langgenius/dify-ui/alert-dialog', () => ({ @@ -223,7 +224,7 @@ describe('SnippetHeader', () => { ) expect(screen.getByRole('button', { name: 'snippet.exitEditing' })).toBeDisabled() - expect(screen.getByRole('button', { name: /^snippet\.save$/i })).toBeDisabled() + expectLoadingButton(screen.getByRole('button', { name: /^snippet\.save$/i })) expect(screen.getByRole('button', { name: 'snippet.doNotSave' })).toBeDisabled() }) diff --git a/web/app/components/workflow/__tests__/dsl-export-confirm-modal.spec.tsx b/web/app/components/workflow/__tests__/dsl-export-confirm-modal.spec.tsx index d7753d163ea..d0db0f02672 100644 --- a/web/app/components/workflow/__tests__/dsl-export-confirm-modal.spec.tsx +++ b/web/app/components/workflow/__tests__/dsl-export-confirm-modal.spec.tsx @@ -1,5 +1,6 @@ import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { expectLoadingButton } from '@/test/button' import DSLExportConfirmModal from '../dsl-export-confirm-modal' const envList = [ @@ -125,7 +126,7 @@ describe('DSLExportConfirmModal', () => { const firstClick = user.click(confirmButton) await waitFor(() => { - expect(confirmButton).toBeDisabled() + expectLoadingButton(confirmButton) expect(confirmButton).toHaveTextContent('common.operation.exporting') expect(screen.getByRole('button', { name: 'common.operation.cancel' })).toBeDisabled() }) diff --git a/web/app/install/installForm.spec.tsx b/web/app/install/installForm.spec.tsx index 0b22c1cc70e..d23c9c8d9ab 100644 --- a/web/app/install/installForm.spec.tsx +++ b/web/app/install/installForm.spec.tsx @@ -3,6 +3,7 @@ import type { InitValidateStatusResponse, SetupStatusResponse } from '@/models/c import { fireEvent, screen, waitFor } from '@testing-library/react' import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features' import { fetchInitValidateStatus, fetchSetupStatus, login, setup } from '@/service/common' +import { expectLoadingButton } from '@/test/button' import { encryptPassword } from '@/utils/encryption' import InstallForm from './installForm' @@ -135,7 +136,7 @@ describe('InstallForm', () => { fireEvent.click(button) await waitFor(() => { - expect(button).toBeDisabled() + expectLoadingButton(button) }) fireEvent.click(button) diff --git a/web/test/button.ts b/web/test/button.ts new file mode 100644 index 00000000000..ab2f036ec51 --- /dev/null +++ b/web/test/button.ts @@ -0,0 +1,8 @@ +import { expect } from 'vitest' + +export const expectLoadingButton = (button: Element | null) => { + expect(button).toBeInstanceOf(HTMLButtonElement) + expect(button).toHaveAttribute('aria-busy', 'true') + expect(button).toHaveAttribute('aria-disabled', 'true') + expect(button).not.toHaveAttribute('disabled') +}