diff --git a/api/controllers/inner_api/plugin/wraps.py b/api/controllers/inner_api/plugin/wraps.py index a5846e2815..2f309262cb 100644 --- a/api/controllers/inner_api/plugin/wraps.py +++ b/api/controllers/inner_api/plugin/wraps.py @@ -20,10 +20,13 @@ class TenantUserPayload(BaseModel): def get_user(tenant_id: str, user_id: str | None) -> EndUser: """ - Get current user + Get current user. NOTE: user_id is not trusted, it could be maliciously set to any value. - As a result, it could only be considered as an end user id. + As a result, it could only be considered as an end user id. Even when a + concrete end-user ID is supplied, lookups must stay tenant-scoped so one + tenant cannot bind another tenant's user record into the plugin request + context. """ if not user_id: user_id = DefaultEndUserSessionID.DEFAULT_SESSION_ID @@ -42,7 +45,14 @@ def get_user(tenant_id: str, user_id: str | None) -> EndUser: .limit(1) ) else: - user_model = session.get(EndUser, user_id) + user_model = session.scalar( + select(EndUser) + .where( + EndUser.id == user_id, + EndUser.tenant_id == tenant_id, + ) + .limit(1) + ) if not user_model: user_model = EndUser( diff --git a/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py b/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py index 0895fac3a4..d1b09c3a58 100644 --- a/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py +++ b/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py @@ -41,17 +41,22 @@ class TestTenantUserPayload: class TestGetUser: """Test get_user function""" + @patch("controllers.inner_api.plugin.wraps.select") @patch("controllers.inner_api.plugin.wraps.EndUser") @patch("controllers.inner_api.plugin.wraps.sessionmaker") @patch("controllers.inner_api.plugin.wraps.db") - def test_should_return_existing_user_by_id(self, mock_db, mock_sessionmaker, mock_enduser_class, app: Flask): + def test_should_return_existing_user_by_id( + self, mock_db, mock_sessionmaker, mock_enduser_class, mock_select, app: Flask + ): """Test returning existing user when found by ID""" # Arrange mock_user = MagicMock() mock_user.id = "user123" mock_session = MagicMock() mock_sessionmaker.return_value.begin.return_value.__enter__.return_value = mock_session - mock_session.get.return_value = mock_user + mock_session.scalar.return_value = mock_user + mock_query = MagicMock() + mock_select.return_value.where.return_value.limit.return_value = mock_query # Act with app.app_context(): @@ -59,13 +64,45 @@ class TestGetUser: # Assert assert result == mock_user - mock_session.get.assert_called_once() + mock_session.scalar.assert_called_once() + @patch("controllers.inner_api.plugin.wraps.select") + @patch("controllers.inner_api.plugin.wraps.EndUser") + @patch("controllers.inner_api.plugin.wraps.sessionmaker") + @patch("controllers.inner_api.plugin.wraps.db") + def test_should_not_resolve_non_anonymous_users_across_tenants( + self, + mock_db, + mock_sessionmaker, + mock_enduser_class, + mock_select, + app: Flask, + ): + """Test that explicit user IDs remain scoped to the current tenant.""" + # Arrange + mock_session = MagicMock() + mock_sessionmaker.return_value.begin.return_value.__enter__.return_value = mock_session + mock_session.scalar.return_value = None + mock_new_user = MagicMock() + mock_new_user.tenant_id = "tenant-current" + mock_enduser_class.return_value = mock_new_user + + # Act + with app.app_context(): + result = get_user("tenant-current", "foreign-user-id") + + # Assert + assert result == mock_new_user + mock_session.get.assert_not_called() + mock_session.scalar.assert_called_once() + mock_session.add.assert_called_once_with(mock_new_user) + + @patch("controllers.inner_api.plugin.wraps.select") @patch("controllers.inner_api.plugin.wraps.EndUser") @patch("controllers.inner_api.plugin.wraps.sessionmaker") @patch("controllers.inner_api.plugin.wraps.db") def test_should_return_existing_anonymous_user_by_session_id( - self, mock_db, mock_sessionmaker, mock_enduser_class, app: Flask + self, mock_db, mock_sessionmaker, mock_enduser_class, mock_select, app: Flask ): """Test returning existing anonymous user by session_id""" # Arrange @@ -73,8 +110,9 @@ class TestGetUser: mock_user.session_id = "anonymous_session" mock_session = MagicMock() mock_sessionmaker.return_value.begin.return_value.__enter__.return_value = mock_session - # non-anonymous path uses session.get(); anonymous uses session.scalar() - mock_session.get.return_value = mock_user + mock_session.scalar.return_value = mock_user + mock_query = MagicMock() + mock_select.return_value.where.return_value.limit.return_value = mock_query # Act with app.app_context(): @@ -83,17 +121,22 @@ class TestGetUser: # Assert assert result == mock_user + @patch("controllers.inner_api.plugin.wraps.select") @patch("controllers.inner_api.plugin.wraps.EndUser") @patch("controllers.inner_api.plugin.wraps.sessionmaker") @patch("controllers.inner_api.plugin.wraps.db") - def test_should_create_new_user_when_not_found(self, mock_db, mock_sessionmaker, mock_enduser_class, app: Flask): + def test_should_create_new_user_when_not_found( + self, mock_db, mock_sessionmaker, mock_enduser_class, mock_select, app: Flask + ): """Test creating new user when not found in database""" # Arrange mock_session = MagicMock() mock_sessionmaker.return_value.begin.return_value.__enter__.return_value = mock_session - mock_session.get.return_value = None + mock_session.scalar.return_value = None mock_new_user = MagicMock() mock_enduser_class.return_value = mock_new_user + mock_query = MagicMock() + mock_select.return_value.where.return_value.limit.return_value = mock_query # Act with app.app_context(): @@ -134,7 +177,7 @@ class TestGetUser: # Arrange mock_session = MagicMock() mock_sessionmaker.return_value.begin.return_value.__enter__.return_value = mock_session - mock_session.get.side_effect = Exception("Database error") + mock_session.scalar.side_effect = Exception("Database error") # Act & Assert with app.app_context(): diff --git a/e2e/features/apps/create-agent-app.feature b/e2e/features/apps/create-agent-app.feature new file mode 100644 index 0000000000..75d8dc3a77 --- /dev/null +++ b/e2e/features/apps/create-agent-app.feature @@ -0,0 +1,11 @@ +@apps @authenticated @core @mode-matrix +Feature: Create Agent app + Scenario: Create a new Agent app and redirect to the configuration page + Given I am signed in as the default E2E admin + When I open the apps console + And I start creating a blank app + And I expand the beginner app types + And I select the "Agent" app type + And I enter a unique E2E app name + And I confirm app creation + Then I should land on the app configuration page diff --git a/e2e/features/apps/create-chatflow-app.feature b/e2e/features/apps/create-chatflow-app.feature new file mode 100644 index 0000000000..364cccc494 --- /dev/null +++ b/e2e/features/apps/create-chatflow-app.feature @@ -0,0 +1,10 @@ +@apps @authenticated @core @mode-matrix +Feature: Create Chatflow app + Scenario: Create a new Chatflow app and redirect to the workflow editor + Given I am signed in as the default E2E admin + When I open the apps console + And I start creating a blank app + And I select the "Chatflow" app type + And I enter a unique E2E app name + And I confirm app creation + Then I should land on the workflow editor diff --git a/e2e/features/apps/create-text-generator-app.feature b/e2e/features/apps/create-text-generator-app.feature new file mode 100644 index 0000000000..aec0436644 --- /dev/null +++ b/e2e/features/apps/create-text-generator-app.feature @@ -0,0 +1,11 @@ +@apps @authenticated @core @mode-matrix +Feature: Create Text Generator app + Scenario: Create a new Text Generator app and redirect to the configuration page + Given I am signed in as the default E2E admin + When I open the apps console + And I start creating a blank app + And I expand the beginner app types + And I select the "Text Generator" app type + And I enter a unique E2E app name + And I confirm app creation + Then I should land on the app configuration page diff --git a/e2e/features/apps/delete-app.feature b/e2e/features/apps/delete-app.feature new file mode 100644 index 0000000000..49326ba098 --- /dev/null +++ b/e2e/features/apps/delete-app.feature @@ -0,0 +1,11 @@ +@apps @authenticated @core +Feature: Delete app + Scenario: Delete an existing app from the apps console + Given I am signed in as the default E2E admin + And there is an existing E2E app available for testing + When I open the apps console + And I open the options menu for the last created E2E app + And I click "Delete" in the app options menu + And I type the app name in the deletion confirmation + And I confirm the deletion + Then the app should no longer appear in the apps console diff --git a/e2e/features/apps/duplicate-app.feature b/e2e/features/apps/duplicate-app.feature new file mode 100644 index 0000000000..3645a7d172 --- /dev/null +++ b/e2e/features/apps/duplicate-app.feature @@ -0,0 +1,10 @@ +@apps @authenticated @core +Feature: Duplicate app + Scenario: Duplicate an existing app and open the copy in the editor + Given I am signed in as the default E2E admin + And there is an existing E2E app available for testing + When I open the apps console + And I open the options menu for the last created E2E app + And I click "Duplicate" in the app options menu + And I confirm the app duplication + Then I should land on the app editor diff --git a/e2e/features/apps/export-app.feature b/e2e/features/apps/export-app.feature new file mode 100644 index 0000000000..d6d040fb00 --- /dev/null +++ b/e2e/features/apps/export-app.feature @@ -0,0 +1,9 @@ +@apps @authenticated @core +Feature: Export app DSL + Scenario: Export the DSL file for an existing app + Given I am signed in as the default E2E admin + And there is an existing E2E completion app available for testing + When I open the apps console + And I open the options menu for the last created E2E app + And I click "Export DSL" in the app options menu + Then a YAML file named after the app should be downloaded diff --git a/e2e/features/apps/switch-app-mode.feature b/e2e/features/apps/switch-app-mode.feature new file mode 100644 index 0000000000..5cdc6341fb --- /dev/null +++ b/e2e/features/apps/switch-app-mode.feature @@ -0,0 +1,10 @@ +@apps @authenticated @core +Feature: Switch app mode + Scenario: Switch a Completion app to Workflow Orchestrate + Given I am signed in as the default E2E admin + And there is an existing E2E completion app available for testing + When I open the apps console + And I open the options menu for the last created E2E app + And I click "Switch to Workflow Orchestrate" in the app options menu + And I confirm the app switch + Then I should land on the switched app diff --git a/e2e/features/step-definitions/apps/create-app.steps.ts b/e2e/features/step-definitions/apps/create-app.steps.ts index e444b97dc8..931d4662a2 100644 --- a/e2e/features/step-definitions/apps/create-app.steps.ts +++ b/e2e/features/step-definitions/apps/create-app.steps.ts @@ -11,7 +11,7 @@ When('I start creating a blank app', async function (this: DifyWorld) { When('I enter a unique E2E app name', async function (this: DifyWorld) { const appName = `E2E App ${Date.now()}` - + this.lastCreatedAppName = appName await this.getPage().getByPlaceholder('Give your app a name').fill(appName) }) @@ -26,10 +26,15 @@ When('I confirm app creation', async function (this: DifyWorld) { When('I select the {string} app type', async function (this: DifyWorld, appType: string) { const dialog = this.getPage().getByRole('dialog') - const appTypeTitle = dialog.getByText(appType, { exact: true }) + // The modal defaults to ADVANCED_CHAT, so the preview panel immediately renders + //

Chatflow

alongside the card's
Chatflow
. + // locator('div').getByText(...) would still match the

because getByText + // searches inside each div for any descendant. Use :text-is() instead, which + // targets only
elements whose own normalised text equals appType exactly. + const appTypeCard = dialog.locator(`div:text-is("${appType}")`) - await expect(appTypeTitle).toBeVisible() - await appTypeTitle.click() + await expect(appTypeCard).toBeVisible() + await appTypeCard.click() }) When('I expand the beginner app types', async function (this: DifyWorld) { diff --git a/e2e/features/step-definitions/apps/delete-app.steps.ts b/e2e/features/step-definitions/apps/delete-app.steps.ts new file mode 100644 index 0000000000..e5da626645 --- /dev/null +++ b/e2e/features/step-definitions/apps/delete-app.steps.ts @@ -0,0 +1,35 @@ +import type { DifyWorld } from '../../support/world' +import { Then, When } from '@cucumber/cucumber' +import { expect } from '@playwright/test' + +When('I type the app name in the deletion confirmation', async function (this: DifyWorld) { + const appName = this.lastCreatedAppName + if (!appName) { + throw new Error( + 'No app name stored. Run "there is an existing E2E app available for testing" first.', + ) + } + + const page = this.getPage() + const dialog = page.getByRole('alertdialog') + await expect(dialog).toBeVisible() + await dialog.getByPlaceholder('Enter app name…').fill(appName) +}) + +When('I confirm the deletion', async function (this: DifyWorld) { + const dialog = this.getPage().getByRole('alertdialog') + await dialog.getByRole('button', { name: 'Confirm' }).click() +}) + +Then('the app should no longer appear in the apps console', async function (this: DifyWorld) { + const appName = this.lastCreatedAppName + if (!appName) { + throw new Error( + 'No app name stored. Run "there is an existing E2E app available for testing" first.', + ) + } + + await expect(this.getPage().getByTitle(appName)).not.toBeVisible({ + timeout: 10_000, + }) +}) diff --git a/e2e/features/step-definitions/apps/duplicate-app.steps.ts b/e2e/features/step-definitions/apps/duplicate-app.steps.ts new file mode 100644 index 0000000000..e5e3694e4d --- /dev/null +++ b/e2e/features/step-definitions/apps/duplicate-app.steps.ts @@ -0,0 +1,36 @@ +import type { DifyWorld } from '../../support/world' +import { Given, When } from '@cucumber/cucumber' +import { createTestApp } from '../../../support/api' + +Given('there is an existing E2E app available for testing', async function (this: DifyWorld) { + const name = `E2E Test App ${Date.now()}` + const app = await createTestApp(name, 'completion') + this.lastCreatedAppName = app.name + this.createdAppIds.push(app.id) +}) + +When('I open the options menu for the last created E2E app', async function (this: DifyWorld) { + const appName = this.lastCreatedAppName + if (!appName) + throw new Error('No app name stored. Run "I enter a unique E2E app name" first.') + + const page = this.getPage() + // Scope to the specific card: the card root is the innermost div that contains + // both the unique app name text and a More button (they are in separate branches, + // so no child div satisfies both). .last() picks the deepest match in DOM order. + const appCard = page + .locator('div') + .filter({ has: page.getByText(appName, { exact: true }) }) + .filter({ has: page.getByRole('button', { name: 'More' }) }) + .last() + await appCard.hover() + await appCard.getByRole('button', { name: 'More' }).click() +}) + +When('I click {string} in the app options menu', async function (this: DifyWorld, label: string) { + await this.getPage().getByRole('menuitem', { name: label }).click() +}) + +When('I confirm the app duplication', async function (this: DifyWorld) { + await this.getPage().getByRole('button', { name: 'Duplicate' }).click() +}) diff --git a/e2e/features/step-definitions/apps/export-app.steps.ts b/e2e/features/step-definitions/apps/export-app.steps.ts new file mode 100644 index 0000000000..4ebeecb507 --- /dev/null +++ b/e2e/features/step-definitions/apps/export-app.steps.ts @@ -0,0 +1,19 @@ +import type { DifyWorld } from '../../support/world' +import { Then } from '@cucumber/cucumber' +import { expect } from '@playwright/test' + +Then('a YAML file named after the app should be downloaded', async function (this: DifyWorld) { + const appName = this.lastCreatedAppName + if (!appName) { + throw new Error( + 'No app name stored. Run "there is an existing E2E app available for testing" first.', + ) + } + + // The export triggers an async API call before the blob download fires. + // Poll until the download event is captured by the page listener in DifyWorld. + await expect.poll(() => this.capturedDownloads.length, { timeout: 10_000 }).toBeGreaterThan(0) + + const download = this.capturedDownloads.at(-1)! + expect(download.suggestedFilename()).toBe(`${appName}.yml`) +}) diff --git a/e2e/features/step-definitions/apps/switch-app-mode.steps.ts b/e2e/features/step-definitions/apps/switch-app-mode.steps.ts new file mode 100644 index 0000000000..55ad1ab02c --- /dev/null +++ b/e2e/features/step-definitions/apps/switch-app-mode.steps.ts @@ -0,0 +1,28 @@ +import type { DifyWorld } from '../../support/world' +import { Given, Then, When } from '@cucumber/cucumber' +import { expect } from '@playwright/test' +import { createTestApp } from '../../../support/api' + +Given( + 'there is an existing E2E completion app available for testing', + async function (this: DifyWorld) { + const name = `E2E Test App ${Date.now()}` + const app = await createTestApp(name, 'completion') + this.lastCreatedAppName = app.name + this.createdAppIds.push(app.id) + }, +) + +When('I confirm the app switch', async function (this: DifyWorld) { + await this.getPage().getByRole('button', { name: 'Start switch' }).click() +}) + +Then('I should land on the switched app', async function (this: DifyWorld) { + const page = this.getPage() + await expect(page).toHaveURL(/\/app\/[^/]+\/workflow(?:\?.*)?$/, { timeout: 15_000 }) + + // Capture the new app's ID so the After hook can clean it up + const match = page.url().match(/\/app\/([^/]+)\/workflow/) + if (match?.[1]) + this.createdAppIds.push(match[1]) +}) diff --git a/e2e/features/support/hooks.ts b/e2e/features/support/hooks.ts index 33b337fb93..c1a535ee2c 100644 --- a/e2e/features/support/hooks.ts +++ b/e2e/features/support/hooks.ts @@ -6,6 +6,7 @@ import { fileURLToPath } from 'node:url' import { After, AfterAll, Before, BeforeAll, setDefaultTimeout, Status } from '@cucumber/cucumber' import { chromium } from '@playwright/test' import { AUTH_BOOTSTRAP_TIMEOUT_MS, ensureAuthenticatedState } from '../../fixtures/auth' +import { deleteTestApp } from '../../support/api' import { baseURL, cucumberHeadless, cucumberSlowMo } from '../../test-env' const e2eRoot = fileURLToPath(new URL('../..', import.meta.url)) @@ -88,6 +89,8 @@ After(async function (this: DifyWorld, { pickle, result }) { `[e2e] end ${pickle.name} status=${status}${elapsedMs ? ` durationMs=${elapsedMs}` : ''}`, ) + for (const id of this.createdAppIds) await deleteTestApp(id).catch(() => {}) + await this.closeSession() }) diff --git a/e2e/features/support/world.ts b/e2e/features/support/world.ts index 0e9c4b9c84..986f79c8f9 100644 --- a/e2e/features/support/world.ts +++ b/e2e/features/support/world.ts @@ -1,12 +1,8 @@ import type { IWorldOptions } from '@cucumber/cucumber' -import type { Browser, BrowserContext, ConsoleMessage, Page } from '@playwright/test' +import type { Browser, BrowserContext, ConsoleMessage, Download, Page } from '@playwright/test' import type { AuthSessionMetadata } from '../../fixtures/auth' import { setWorldConstructor, World } from '@cucumber/cucumber' -import { - - authStatePath, - readAuthSessionMetadata, -} from '../../fixtures/auth' +import { authStatePath, readAuthSessionMetadata } from '../../fixtures/auth' import { baseURL, defaultLocale } from '../../test-env' export class DifyWorld extends World { @@ -16,6 +12,9 @@ export class DifyWorld extends World { pageErrors: string[] = [] scenarioStartedAt: number | undefined session: AuthSessionMetadata | undefined + lastCreatedAppName: string | undefined + createdAppIds: string[] = [] + capturedDownloads: Download[] = [] constructor(options: IWorldOptions) { super(options) @@ -25,6 +24,9 @@ export class DifyWorld extends World { resetScenarioState() { this.consoleErrors = [] this.pageErrors = [] + this.lastCreatedAppName = undefined + this.createdAppIds = [] + this.capturedDownloads = [] } async startSession(browser: Browser, authenticated: boolean) { @@ -45,6 +47,9 @@ export class DifyWorld extends World { this.page.on('pageerror', (error) => { this.pageErrors.push(error.message) }) + this.page.on('download', (dl) => { + this.capturedDownloads.push(dl) + }) } async startAuthenticatedSession(browser: Browser) { diff --git a/e2e/support/api.ts b/e2e/support/api.ts new file mode 100644 index 0000000000..c6d6c98bde --- /dev/null +++ b/e2e/support/api.ts @@ -0,0 +1,54 @@ +import { readFile } from 'node:fs/promises' +import { request } from '@playwright/test' +import { authStatePath } from '../fixtures/auth' +import { apiURL } from '../test-env' + +type StorageState = { + cookies: Array<{ name: string, value: string }> +} + +async function createApiContext() { + const state = JSON.parse(await readFile(authStatePath, 'utf8')) as StorageState + const csrfToken = state.cookies.find(c => c.name.endsWith('csrf_token'))?.value ?? '' + + return request.newContext({ + baseURL: apiURL, + extraHTTPHeaders: { 'X-CSRF-Token': csrfToken }, + storageState: authStatePath, + }) +} + +export type AppSeed = { + id: string + name: string +} + +export async function createTestApp(name: string, mode = 'workflow'): Promise { + const ctx = await createApiContext() + try { + const response = await ctx.post('/console/api/apps', { + data: { + name, + mode, + icon_type: 'emoji', + icon: '🤖', + icon_background: '#FFEAD5', + }, + }) + const body = (await response.json()) as AppSeed + return body + } + finally { + await ctx.dispose() + } +} + +export async function deleteTestApp(id: string): Promise { + const ctx = await createApiContext() + try { + await ctx.delete(`/console/api/apps/${id}`) + } + finally { + await ctx.dispose() + } +} diff --git a/e2e/vite.config.ts b/e2e/vite.config.ts index 2329b534b4..f3dd7bbb0b 100644 --- a/e2e/vite.config.ts +++ b/e2e/vite.config.ts @@ -1,5 +1,3 @@ import { defineConfig } from 'vite-plus' -export default defineConfig({ - -}) +export default defineConfig({}) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e4831c4e98..763d94af9a 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -218,36 +218,20 @@ } }, "web/app/components/app-sidebar/app-info/app-operations.tsx": { - "no-restricted-imports": { - "count": 1 - }, "react/set-state-in-effect": { "count": 4 } }, - "web/app/components/app-sidebar/app-sidebar-dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/app-sidebar/basic.tsx": { "no-restricted-imports": { "count": 1 } }, "web/app/components/app-sidebar/dataset-info/dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - }, "ts/no-explicit-any": { "count": 1 } }, - "web/app/components/app-sidebar/dataset-sidebar-dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/app-sidebar/index.tsx": { "ts/no-explicit-any": { "count": 1 @@ -338,11 +322,6 @@ "count": 5 } }, - "web/app/components/app/app-publisher/publish-with-multiple-model.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/app/app-publisher/version-info-modal.tsx": { "no-restricted-imports": { "count": 1 @@ -575,11 +554,6 @@ "count": 6 } }, - "web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx": { - "no-restricted-imports": { - "count": 2 - } - }, "web/app/components/app/configuration/debug/debug-with-multiple-model/index.tsx": { "ts/no-explicit-any": { "count": 2 @@ -802,9 +776,6 @@ "web/app/components/base/action-button/index.tsx": { "erasable-syntax-only/enums": { "count": 1 - }, - "react-refresh/only-export-components": { - "count": 1 } }, "web/app/components/base/agent-log-modal/detail.tsx": { @@ -2594,11 +2565,6 @@ "count": 1 } }, - "web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/list/item.tsx": { "no-restricted-imports": { "count": 1 @@ -3022,9 +2988,6 @@ } }, "web/app/components/explore/item-operation/index.tsx": { - "no-restricted-imports": { - "count": 1 - }, "react/set-state-in-effect": { "count": 1 } @@ -3168,11 +3131,6 @@ "count": 1 } }, - "web/app/components/header/account-setting/data-source-page-new/operator.tsx": { - "no-restricted-imports": { - "count": 2 - } - }, "web/app/components/header/account-setting/data-source-page-new/types.ts": { "ts/no-explicit-any": { "count": 2 @@ -3196,11 +3154,6 @@ "count": 3 } }, - "web/app/components/header/account-setting/members-page/operation/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/header/account-setting/members-page/transfer-ownership-modal/index.tsx": { "erasable-syntax-only/enums": { "count": 1 @@ -3490,11 +3443,6 @@ "count": 1 } }, - "web/app/components/plugins/marketplace/sort-dropdown/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/plugins/plugin-auth/authorize/add-oauth-button.tsx": { "ts/no-explicit-any": { "count": 2 @@ -3851,9 +3799,6 @@ } }, "web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - }, "react/set-state-in-effect": { "count": 2 } @@ -3868,11 +3813,6 @@ "count": 1 } }, - "web/app/components/plugins/plugin-page/plugin-tasks/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/plugins/readme-panel/index.tsx": { "react/unsupported-syntax": { "count": 1 @@ -4091,9 +4031,6 @@ } }, "web/app/components/share/text-generation/menu-dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - }, "react/set-state-in-effect": { "count": 1 } @@ -4175,11 +4112,6 @@ "count": 3 } }, - "web/app/components/tools/mcp/detail/operation-dropdown.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/tools/mcp/detail/tool-item.tsx": { "no-restricted-imports": { "count": 1 @@ -4349,9 +4281,6 @@ } }, "web/app/components/workflow/block-selector/market-place-plugin/action.tsx": { - "no-restricted-imports": { - "count": 1 - }, "react/set-state-in-effect": { "count": 1 } @@ -4467,9 +4396,6 @@ "erasable-syntax-only/enums": { "count": 1 }, - "no-restricted-imports": { - "count": 1 - }, "react-refresh/only-export-components": { "count": 1 } @@ -4753,11 +4679,6 @@ "count": 1 } }, - "web/app/components/workflow/nodes/_base/components/next-step/operator.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/nodes/_base/components/node-control.tsx": { "no-restricted-imports": { "count": 1 @@ -4773,11 +4694,6 @@ "count": 1 } }, - "web/app/components/workflow/nodes/_base/components/panel-operator/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/nodes/_base/components/prompt/editor.tsx": { "no-restricted-imports": { "count": 1 @@ -5001,11 +4917,6 @@ "count": 1 } }, - "web/app/components/workflow/nodes/assigner/components/operation-selector.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/nodes/assigner/default.ts": { "ts/no-explicit-any": { "count": 1 @@ -6010,11 +5921,6 @@ "count": 1 } }, - "web/app/components/workflow/note-node/note-editor/toolbar/operator.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/note-node/note-editor/utils.ts": { "regexp/no-useless-quantifier": { "count": 1 @@ -6030,11 +5936,6 @@ "count": 1 } }, - "web/app/components/workflow/operator/more-actions.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/operator/tip-popup.tsx": { "no-restricted-imports": { "count": 1 @@ -6043,9 +5944,6 @@ "web/app/components/workflow/operator/zoom-in-out.tsx": { "erasable-syntax-only/enums": { "count": 1 - }, - "no-restricted-imports": { - "count": 1 } }, "web/app/components/workflow/panel/chat-record/index.tsx": { @@ -6141,11 +6039,6 @@ "count": 4 } }, - "web/app/components/workflow/panel/version-history-panel/context-menu/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/panel/version-history-panel/delete-confirm-modal.tsx": { "no-restricted-imports": { "count": 1 @@ -6166,11 +6059,6 @@ "count": 2 } }, - "web/app/components/workflow/run/agent-log/agent-log-nav-more.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/workflow/run/agent-log/index.tsx": { "no-barrel-files/no-barrel-files": { "count": 2 @@ -6450,9 +6338,6 @@ "web/app/components/workflow/workflow-preview/components/zoom-in-out.tsx": { "erasable-syntax-only/enums": { "count": 1 - }, - "no-restricted-imports": { - "count": 1 } }, "web/app/education-apply/expire-notice-modal.tsx": { diff --git a/web/__tests__/app-sidebar/dataset-info-flow.test.tsx b/web/__tests__/app-sidebar/dataset-info-flow.test.tsx index d1ca233d96..3093b2809d 100644 --- a/web/__tests__/app-sidebar/dataset-info-flow.test.tsx +++ b/web/__tests__/app-sidebar/dataset-info-flow.test.tsx @@ -194,7 +194,7 @@ describe('App Sidebar Dataset Info Flow', () => { openDropdown() fireEvent.click(await screen.findByText('common.operation.edit')) - expect(screen.getByTestId('rename-dataset-modal')).toBeInTheDocument() + expect(await screen.findByTestId('rename-dataset-modal')).toBeInTheDocument() fireEvent.click(screen.getByRole('button', { name: 'rename-success' })) diff --git a/web/__tests__/app-sidebar/sidebar-shell-flow.test.tsx b/web/__tests__/app-sidebar/sidebar-shell-flow.test.tsx index 3e3edba5dd..a7c660105d 100644 --- a/web/__tests__/app-sidebar/sidebar-shell-flow.test.tsx +++ b/web/__tests__/app-sidebar/sidebar-shell-flow.test.tsx @@ -181,7 +181,7 @@ describe('App Sidebar Shell Flow', () => { expect(mockSetAppSidebarExpand).toHaveBeenCalledWith('collapse') }) - it('switches to the workflow fullscreen dropdown shell and opens its navigation menu', () => { + it('switches to the workflow fullscreen dropdown shell and opens its navigation menu', async () => { mockPathname = '/app/app-1/workflow' mockSelectedSegment = 'workflow' localStorage.setItem('workflow-canvas-maximize', 'true') @@ -190,9 +190,9 @@ describe('App Sidebar Shell Flow', () => { expect(screen.queryByTestId('app-info')).not.toBeInTheDocument() - fireEvent.click(screen.getByTestId('portal-trigger')) + fireEvent.click(screen.getByRole('button', { name: 'operation.more' })) - expect(screen.getByText('Demo App')).toBeInTheDocument() + expect(await screen.findByText('Demo App')).toBeInTheDocument() expect(screen.getByRole('link', { name: /Overview/i })).toBeInTheDocument() expect(screen.getByRole('link', { name: /Logs/i })).toBeInTheDocument() }) diff --git a/web/app/components/app-sidebar/__tests__/app-sidebar-dropdown.spec.tsx b/web/app/components/app-sidebar/__tests__/app-sidebar-dropdown.spec.tsx index 5018709da1..5e18bbc343 100644 --- a/web/app/components/app-sidebar/__tests__/app-sidebar-dropdown.spec.tsx +++ b/web/app/components/app-sidebar/__tests__/app-sidebar-dropdown.spec.tsx @@ -19,17 +19,40 @@ vi.mock('@/context/app-context', () => ({ }), })) -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) => ( -
{children}
- ), - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) => ( -
{children}
- ), - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), -})) +vi.mock('@/app/components/base/ui/dropdown-menu', () => { + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: React.MouseEventHandler }) => { + const { isOpen, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + } +}) vi.mock('../../base/app-icon', () => ({ default: ({ size, icon }: { size: string, icon: string }) => ( @@ -128,11 +151,11 @@ describe('AppSidebarDropdown', () => { const user = userEvent.setup() render() - const trigger = screen.getByTestId('portal-trigger') + const trigger = screen.getByTestId('dropdown-trigger') await user.click(trigger) - const portal = screen.getByTestId('portal-elem') - expect(portal).toHaveAttribute('data-open', 'true') + const dropdown = screen.getByTestId('dropdown-menu') + expect(dropdown).toHaveAttribute('data-open', 'true') }) it('should render divider between app info and navigation', () => { diff --git a/web/app/components/app-sidebar/__tests__/dataset-sidebar-dropdown.spec.tsx b/web/app/components/app-sidebar/__tests__/dataset-sidebar-dropdown.spec.tsx index 1f3a5f9ad8..5060987cda 100644 --- a/web/app/components/app-sidebar/__tests__/dataset-sidebar-dropdown.spec.tsx +++ b/web/app/components/app-sidebar/__tests__/dataset-sidebar-dropdown.spec.tsx @@ -21,17 +21,40 @@ vi.mock('@/hooks/use-knowledge', () => ({ }), })) -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) => ( -
{children}
- ), - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) => ( -
{children}
- ), - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), -})) +vi.mock('@/app/components/base/ui/dropdown-menu', () => { + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: React.MouseEventHandler }) => { + const { isOpen, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + } +}) vi.mock('../../base/app-icon', () => ({ default: ({ size, icon }: { size: string, icon: string }) => ( @@ -173,10 +196,10 @@ describe('DatasetSidebarDropdown', () => { const user = userEvent.setup() render() - const trigger = screen.getByTestId('portal-trigger') + const trigger = screen.getByTestId('dropdown-trigger') await user.click(trigger) - expect(screen.getByTestId('portal-elem')).toHaveAttribute('data-open', 'true') + expect(screen.getByTestId('dropdown-menu')).toHaveAttribute('data-open', 'true') }) it('should render divider', () => { diff --git a/web/app/components/app-sidebar/app-info/__tests__/app-operations.spec.tsx b/web/app/components/app-sidebar/app-info/__tests__/app-operations.spec.tsx index 2c5b133a74..461cedc20c 100644 --- a/web/app/components/app-sidebar/app-info/__tests__/app-operations.spec.tsx +++ b/web/app/components/app-sidebar/app-info/__tests__/app-operations.spec.tsx @@ -30,17 +30,67 @@ vi.mock('../../../base/ui/button', () => ({ ), })) -vi.mock('../../../base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) => ( -
{children}
- ), - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) => ( -
{children}
- ), - PortalToFollowElemContent: ({ children, className }: { children: React.ReactNode, className?: string }) => ( -
{children}
- ), -})) +vi.mock('../../../base/ui/dropdown-menu', () => { + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + onClick, + render, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + render?: React.ReactElement + }) => { + const { isOpen, setOpen } = useDropdownMenuContext() + const handleClick = (e: React.MouseEvent) => { + onClick?.(e) + setOpen(!isOpen) + } + + if (render) + return React.cloneElement(render, { 'data-testid': 'dropdown-trigger', 'onClick': handleClick } as Record, children) + + return + }, + DropdownMenuContent: ({ children, popupClassName }: { children: React.ReactNode, popupClassName?: string }) => { + const { isOpen } = useDropdownMenuContext() + if (!isOpen) + return null + + return
{children}
+ }, + DropdownMenuItem: ({ children, onClick }: { children: React.ReactNode, onClick?: React.MouseEventHandler }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuSeparator: () =>
, + } +}) const createOperation = (id: string, title: string, type?: 'divider'): Operation => ({ id, @@ -169,7 +219,7 @@ describe('AppOperations', () => { render() - const trigger = screen.queryByTestId('portal-trigger') + const trigger = screen.queryByTestId('dropdown-trigger') if (trigger) await user.click(trigger) diff --git a/web/app/components/app-sidebar/app-info/app-operations.tsx b/web/app/components/app-sidebar/app-info/app-operations.tsx index a3e67c8a59..095fb31206 100644 --- a/web/app/components/app-sidebar/app-info/app-operations.tsx +++ b/web/app/components/app-sidebar/app-info/app-operations.tsx @@ -1,9 +1,15 @@ import type { JSX } from 'react' import { RiMoreLine } from '@remixicon/react' -import { cloneElement, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { cloneElement, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { Button } from '@/app/components/base/ui/button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '../../base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '../../base/ui/dropdown-menu' export type Operation = { id: string @@ -33,9 +39,6 @@ const AppOperations = ({ const [moreOperations, setMoreOperations] = useState([]) const [showMore, setShowMore] = useState(false) const navRef = useRef(null) - const handleTriggerMore = useCallback(() => { - setShowMore(true) - }, [setShowMore]) const primaryOps = useMemo(() => { if (operations) @@ -169,43 +172,44 @@ const AppOperations = ({ ))} {shouldShowMoreButton && ( - - - - - -
- {moreOperations.map(item => item.type === 'divider' - ? ( -
- ) - : ( -
- {cloneElement(item.icon, { className: 'h-4 w-4 text-text-tertiary' })} - {item.title} -
- ))} -
- - + + + + {moreOperations.map(item => item.type === 'divider' + ? ( + + ) + : ( + + {cloneElement(item.icon, { className: 'h-4 w-4 text-text-tertiary' })} + {item.title} + + ))} + + )}
diff --git a/web/app/components/app-sidebar/app-sidebar-dropdown.tsx b/web/app/components/app-sidebar/app-sidebar-dropdown.tsx index 361fc94d69..617d14f426 100644 --- a/web/app/components/app-sidebar/app-sidebar-dropdown.tsx +++ b/web/app/components/app-sidebar/app-sidebar-dropdown.tsx @@ -5,14 +5,14 @@ import { RiMenuLine, } from '@remixicon/react' import * as React from 'react' -import { useCallback, useRef, useState } from 'react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' import { useStore as useAppStore } from '@/app/components/app/store' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useAppContext } from '@/context/app-context' import AppIcon from '../base/app-icon' import Divider from '../base/divider' @@ -34,16 +34,7 @@ const AppSidebarDropdown = ({ navigation }: Props) => { const { isCurrentWorkspaceEditor } = useAppContext() const appDetail = useAppStore(state => state.appDetail) const [detailExpand, setDetailExpand] = useState(false) - - const [open, doSetOpen] = useState(false) - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - doSetOpen(v) - openRef.current = v - }, [doSetOpen]) - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) + const [open, setOpen] = useState(false) if (!appDetail) return null @@ -51,27 +42,28 @@ const AppSidebarDropdown = ({ navigation }: Props) => { return ( <>
- - -
- - -
- - + + + + + +
{ })}
- - + +
diff --git a/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx b/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx index 6ed10609e9..b514b6e095 100644 --- a/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx +++ b/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx @@ -137,14 +137,6 @@ vi.mock('@/app/components/datasets/rename-modal', () => ({ }, })) -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children }: { children: React.ReactNode }) =>
{children}
, - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) => ( -
{children}
- ), - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) =>
{children}
, -})) - describe('Dropdown callback coverage', () => { beforeEach(() => { vi.clearAllMocks() @@ -159,7 +151,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.edit')) expect(screen.getByTestId('rename-modal')).toBeInTheDocument() @@ -175,7 +167,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.edit')) expect(screen.getByTestId('rename-modal')).toBeInTheDocument() @@ -190,7 +182,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { @@ -210,7 +202,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { @@ -224,7 +216,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('datasetPipeline.operations.exportPipeline')) await waitFor(() => { @@ -232,6 +224,27 @@ describe('Dropdown callback coverage', () => { }) }) + it('should not attempt export when the dataset has no pipeline id', async () => { + const user = userEvent.setup() + mockDataset = createDataset({ pipeline_id: '' }) + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('datasetPipeline.operations.exportPipeline')) + + expect(mockExportPipeline).not.toHaveBeenCalled() + }) + + it('should render and open correctly when collapsed', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + + expect(screen.getByText('common.operation.edit')).toBeInTheDocument() + }) + it('should surface the backend message when checking app usage fails', async () => { const user = userEvent.setup() mockCheckIsUsedInApp.mockRejectedValueOnce({ @@ -240,7 +253,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { diff --git a/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx b/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx index bb85e00c14..e6d3f94e2a 100644 --- a/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx +++ b/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx @@ -1,5 +1,4 @@ import type { DataSet } from '@/models/datasets' -import { RiEditLine } from '@remixicon/react' import { createEvent, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' @@ -22,6 +21,7 @@ const mockInvalidDatasetDetail = vi.fn() const mockExportPipeline = vi.fn() const mockCheckIsUsedInApp = vi.fn() const mockDeleteDataset = vi.fn() +const TestEditIcon = () => const createDataset = (overrides: Partial = {}): DataSet => ({ id: 'dataset-1', @@ -210,7 +210,7 @@ describe('MenuItem', () => { const user = userEvent.setup() const handleClick = vi.fn() // Arrange - render() + render() // Act await user.click(screen.getByText('Edit')) @@ -225,7 +225,7 @@ describe('MenuItem', () => { render(
- +
, ) @@ -236,7 +236,7 @@ describe('MenuItem', () => { }) it('should not crash when no click handler is provided', () => { - render() + render() const event = createEvent.click(screen.getByText('Edit')) fireEvent(screen.getByText('Edit'), event) diff --git a/web/app/components/app-sidebar/dataset-info/dropdown.tsx b/web/app/components/app-sidebar/dataset-info/dropdown.tsx index 9abebfcc88..e69b3d7e32 100644 --- a/web/app/components/app-sidebar/dataset-info/dropdown.tsx +++ b/web/app/components/app-sidebar/dataset-info/dropdown.tsx @@ -1,6 +1,5 @@ import type { DataSet } from '@/models/datasets' import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import * as React from 'react' import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -14,7 +13,6 @@ import { useInvalid } from '@/service/use-base' import { useExportPipelineDSL } from '@/service/use-pipeline' import { downloadBlob } from '@/utils/download' import ActionButton from '../../base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '../../base/portal-to-follow-elem' import { AlertDialog, AlertDialogActions, @@ -24,6 +22,11 @@ import { AlertDialogDescription, AlertDialogTitle, } from '../../base/ui/alert-dialog' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '../../base/ui/dropdown-menu' import RenameDatasetModal from '../../datasets/rename-modal' import Menu from './menu' @@ -44,10 +47,6 @@ const DropDown = ({ const isCurrentWorkspaceDatasetOperator = useAppContextWithSelector(state => state.isCurrentWorkspaceDatasetOperator) const dataset = useDatasetDetailContextWithSelector(state => state.dataset) as DataSet - const handleTrigger = useCallback(() => { - setOpen(prev => !prev) - }, []) - const invalidDatasetList = useInvalidDatasetList() const invalidDatasetDetail = useInvalid([...datasetDetailQueryKeyPrefix, dataset.id]) @@ -57,9 +56,11 @@ const DropDown = ({ }, [invalidDatasetDetail, invalidDatasetList]) const openRenameModal = useCallback(() => { - setShowRenameModal(true) - handleTrigger() - }, [handleTrigger]) + setOpen(false) + queueMicrotask(() => { + setShowRenameModal(true) + }) + }, []) const { mutateAsync: exportPipelineConfig } = useExportPipelineDSL() @@ -67,7 +68,7 @@ const DropDown = ({ const { pipeline_id, name } = dataset if (!pipeline_id) return - handleTrigger() + setOpen(false) try { const { data } = await exportPipelineConfig({ pipelineId: pipeline_id, @@ -79,9 +80,10 @@ const DropDown = ({ catch { toast(t('exportFailed', { ns: 'app' }), { type: 'error' }) } - }, [dataset, exportPipelineConfig, handleTrigger, t]) + }, [dataset, exportPipelineConfig, t]) const detectIsUsedByApp = useCallback(async () => { + setOpen(false) try { const { is_using: isUsedByApp } = await checkIsUsedInApp(dataset.id) setConfirmMessage(isUsedByApp ? t('datasetUsedByApp', { ns: 'dataset' })! : t('deleteDatasetConfirmContent', { ns: 'dataset' })!) @@ -91,10 +93,7 @@ const DropDown = ({ const res = await e.json() toast(res?.message || 'Unknown error', { type: 'error' }) } - finally { - handleTrigger() - } - }, [dataset.id, handleTrigger, t]) + }, [dataset.id, t]) const onConfirmDelete = useCallback(async () => { try { @@ -109,32 +108,27 @@ const DropDown = ({ }, [dataset.id, replace, invalidDatasetList, t]) return ( - - - - + }> + + - - + + - + {showRenameModal && ( - + ) } diff --git a/web/app/components/app-sidebar/dataset-sidebar-dropdown.tsx b/web/app/components/app-sidebar/dataset-sidebar-dropdown.tsx index de2563b377..3968a0df6f 100644 --- a/web/app/components/app-sidebar/dataset-sidebar-dropdown.tsx +++ b/web/app/components/app-sidebar/dataset-sidebar-dropdown.tsx @@ -5,13 +5,13 @@ import { RiMenuLine, } from '@remixicon/react' import * as React from 'react' -import { useCallback, useRef, useState } from 'react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useDatasetDetailContextWithSelector } from '@/context/dataset-detail' import { useKnowledge } from '@/hooks/use-knowledge' import { DOC_FORM_TEXT } from '@/models/datasets' @@ -41,15 +41,7 @@ const DatasetSidebarDropdown = ({ const { data: relatedApps } = useDatasetRelatedApps(dataset.id) - const [open, doSetOpen] = useState(false) - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - doSetOpen(v) - openRef.current = v - }, [doSetOpen]) - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) + const [open, setOpen] = useState(false) const iconInfo = dataset.icon_info || { icon: '📙', @@ -66,32 +58,28 @@ const DatasetSidebarDropdown = ({ return ( <>
- - -
- - -
- - + + + + + +
@@ -155,8 +143,8 @@ const DatasetSidebarDropdown = ({ documentCount={dataset.document_count} />
- - + +
) diff --git a/web/app/components/app/app-publisher/__tests__/publish-with-multiple-model.spec.tsx b/web/app/components/app/app-publisher/__tests__/publish-with-multiple-model.spec.tsx index f476d8b188..465252c6c4 100644 --- a/web/app/components/app/app-publisher/__tests__/publish-with-multiple-model.spec.tsx +++ b/web/app/components/app/app-publisher/__tests__/publish-with-multiple-model.spec.tsx @@ -22,24 +22,57 @@ vi.mock('../../header/account-setting/model-provider-page/model-icon', () => ({ default: ({ modelName }: { modelName: string }) => {modelName}, })) -vi.mock('@/app/components/base/portal-to-follow-elem', async () => { +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { const ReactModule = await vi.importActual('react') - const OpenContext = ReactModule.createContext(false) + const OpenContext = ReactModule.createContext<{ open: boolean, setOpen: (nextOpen: boolean) => void } | null>(null) + + const useOpenContext = () => { + const context = ReactModule.use(OpenContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } return { - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) => ( - + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( +
{children}
 ), - PortalToFollowElemTrigger: ({ children, onClick, className }: { children: React.ReactNode, onClick?: () => void, className?: string }) => ( -
- {children} -
- ), - PortalToFollowElemContent: ({ children, className }: { children: React.ReactNode, className?: string }) => { - const open = ReactModule.useContext(OpenContext) - return open ?
{children}
: null + DropdownMenuTrigger: ({ + children, + render, + }: { + children: React.ReactNode + render?: React.ReactElement + }) => { + const { open, setOpen } = useOpenContext() + + if (render) { + return ReactModule.cloneElement(render, { + onClick: () => setOpen(!open), + } as Record, children) + } + + return + }, + DropdownMenuContent: ({ children, popupClassName }: { children: React.ReactNode, popupClassName?: string }) => { + const context = useOpenContext() + return context.open ?
{children}
: null + }, + DropdownMenuItem: ({ children, onClick }: { children: React.ReactNode, onClick?: React.MouseEventHandler }) => { + const { setOpen } = useOpenContext() + return ( + + ) }, } }) diff --git a/web/app/components/app/app-publisher/publish-with-multiple-model.tsx b/web/app/components/app/app-publisher/publish-with-multiple-model.tsx index b4ad0423b0..fbff371577 100644 --- a/web/app/components/app/app-publisher/publish-with-multiple-model.tsx +++ b/web/app/components/app/app-publisher/publish-with-multiple-model.tsx @@ -4,12 +4,13 @@ import type { Model, ModelItem } from '@/app/components/header/account-setting/m import { RiArrowDownSLine } from '@remixicon/react' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { Button } from '@/app/components/base/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useLanguage } from '@/app/components/header/account-setting/model-provider-page/hooks' import { useProviderContext } from '@/context/provider-context' import ModelIcon from '../../header/account-setting/model-provider-page/model-icon' @@ -50,61 +51,57 @@ const PublishWithMultipleModel: FC = ({ } }) - const handleToggle = () => { - if (validModelConfigs.length) - setOpen(v => !v) - } - - const handleSelect = (item: ModelAndParameter) => { - onSelect(item) - setOpen(false) - } - return ( - - - - - -
-
- {t('publishAs', { ns: 'appDebug' })} -
- { - validModelConfigs.map((item, index) => ( -
handleSelect(item)} - > - - # - {index + 1} - - -
- {item.modelItem.label[language]} -
-
- )) - } + + + +
+ {t('publishAs', { ns: 'appDebug' })}
- - + { + validModelConfigs.map((item, index) => ( + onSelect(item)} + > + + # + {index + 1} + + +
+ {item.modelItem.label[language]} +
+ + )) + } + + ) } diff --git a/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx b/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx index 747ea4efcb..1d54ae17bb 100644 --- a/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx +++ b/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx @@ -1,7 +1,7 @@ import type { CSSProperties } from 'react' import type { ModelAndParameter } from '../../types' -import type { Item } from '@/app/components/base/dropdown' -import { fireEvent, render, screen } from '@testing-library/react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { ModelStatusEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { AppModeEnum } from '@/types/app' import DebugItem from '../debug-item' @@ -10,12 +10,6 @@ const mockUseDebugConfigurationContext = vi.fn() const mockUseDebugWithMultipleModelContext = vi.fn() const mockUseProviderContext = vi.fn() -let capturedDropdownProps: { - onSelect: (item: Item) => void - items: Item[] - secondItems?: Item[] -} | null = null - let capturedModelParameterTriggerProps: { modelAndParameter: ModelAndParameter } | null = null @@ -51,34 +45,6 @@ vi.mock('../model-parameter-trigger', () => ({ }, })) -vi.mock('@/app/components/base/dropdown', () => ({ - default: (props: { onSelect: (item: Item) => void, items: Item[], secondItems?: Item[] }) => { - capturedDropdownProps = props - return ( -
- {props.items.map(item => ( - - ))} - {props.secondItems?.map(item => ( - - ))} -
- ) - }, -})) - const createModelAndParameter = (overrides: Partial = {}): ModelAndParameter => ({ id: 'model-1', model: 'gpt-3.5-turbo', @@ -117,7 +83,6 @@ const renderComponent = (props: Partial = {}) => { describe('DebugItem', () => { beforeEach(() => { vi.clearAllMocks() - capturedDropdownProps = null capturedModelParameterTriggerProps = null mockUseDebugConfigurationContext.mockReturnValue({ @@ -137,12 +102,18 @@ describe('DebugItem', () => { }) }) + const openMenu = async () => { + const user = userEvent.setup() + await user.click(screen.getByRole('button')) + return user + } + describe('rendering', () => { it('should render with basic props', () => { renderComponent() - expect(screen.getByTestId('model-parameter-trigger'))!.toBeInTheDocument() - expect(screen.getByTestId('dropdown'))!.toBeInTheDocument() + expect(screen.getByTestId('model-parameter-trigger')).toBeInTheDocument() + expect(screen.getByRole('button')).toBeInTheDocument() }) it('should display correct index number', () => { @@ -280,7 +251,7 @@ describe('DebugItem', () => { }) describe('dropdown menu', () => { - it('should show duplicate option when less than 4 models', () => { + it('should show duplicate option when less than 4 models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [createModelAndParameter()], onMultipleModelConfigsChange: vi.fn(), @@ -288,13 +259,12 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.items).toContainEqual( - expect.objectContaining({ value: 'duplicate' }), - ) + expect(screen.getByText('appDebug.duplicateModel')).toBeInTheDocument() }) - it('should hide duplicate option when 4 or more models', () => { + it('should hide duplicate option when 4 or more models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -307,52 +277,48 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'duplicate' }), - ) + expect(screen.queryByText('appDebug.duplicateModel')).not.toBeInTheDocument() }) - it('should show debug-as-single-model option when provider and model are set', () => { + it('should show debug-as-single-model option when provider and model are set', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: 'openai', model: 'gpt-3.5-turbo', }), }) + await openMenu() - expect(capturedDropdownProps?.items).toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.getByText('appDebug.debugAsSingleModel')).toBeInTheDocument() }) - it('should hide debug-as-single-model option when provider is missing', () => { + it('should hide debug-as-single-model option when provider is missing', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: '', model: 'gpt-3.5-turbo', }), }) + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.queryByText('appDebug.debugAsSingleModel')).not.toBeInTheDocument() }) - it('should hide debug-as-single-model option when model is missing', () => { + it('should hide debug-as-single-model option when model is missing', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: 'openai', model: '', }), }) + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.queryByText('appDebug.debugAsSingleModel')).not.toBeInTheDocument() }) - it('should show remove option in secondItems when more than 2 models', () => { + it('should show remove option in secondItems when more than 2 models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -364,13 +330,12 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.secondItems).toContainEqual( - expect.objectContaining({ value: 'remove' }), - ) + expect(screen.getByText('common.operation.remove')).toBeInTheDocument() }) - it('should not show remove option when 2 or fewer models', () => { + it('should not show remove option when 2 or fewer models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -381,13 +346,14 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.secondItems).toBeUndefined() + expect(screen.queryByText('common.operation.remove')).not.toBeInTheDocument() }) }) describe('dropdown actions', () => { - it('should duplicate model when duplicate is selected', () => { + it('should duplicate model when duplicate is selected', async () => { const onMultipleModelConfigsChange = vi.fn() const originalModel = createModelAndParameter({ id: 'original' }) @@ -399,7 +365,8 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter: originalModel }) - fireEvent.click(screen.getByTestId('dropdown-item-duplicate')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.duplicateModel')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, @@ -414,7 +381,7 @@ describe('DebugItem', () => { ) }) - it('should not duplicate when already at 4 models', () => { + it('should not duplicate when already at 4 models', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -430,14 +397,13 @@ describe('DebugItem', () => { }) renderComponent({ modelAndParameter: models[0] }) - - // Since duplicate is not shown when >= 4 models, we need to manually call handleSelect - capturedDropdownProps?.onSelect({ value: 'duplicate', text: 'Duplicate' }) + await openMenu() expect(onMultipleModelConfigsChange).not.toHaveBeenCalled() + expect(screen.queryByText('appDebug.duplicateModel')).not.toBeInTheDocument() }) - it('should call onDebugWithMultipleModelChange when debug-as-single-model is selected', () => { + it('should call onDebugWithMultipleModelChange when debug-as-single-model is selected', async () => { const onDebugWithMultipleModelChange = vi.fn() const modelAndParameter = createModelAndParameter() @@ -449,12 +415,13 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter }) - fireEvent.click(screen.getByTestId('dropdown-item-debug-as-single-model')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.debugAsSingleModel')) expect(onDebugWithMultipleModelChange).toHaveBeenCalledWith(modelAndParameter) }) - it('should remove model when remove is selected', () => { + it('should remove model when remove is selected', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -470,7 +437,8 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter: models[1] }) - fireEvent.click(screen.getByTestId('dropdown-second-item-remove')) + const user = await openMenu() + await user.click(screen.getByText('common.operation.remove')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, @@ -478,7 +446,7 @@ describe('DebugItem', () => { ) }) - it('should insert duplicated model at correct position', () => { + it('should insert duplicated model at correct position', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -495,7 +463,8 @@ describe('DebugItem', () => { // Duplicate the second model renderComponent({ modelAndParameter: models[1] }) - fireEvent.click(screen.getByTestId('dropdown-item-duplicate')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.duplicateModel')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, diff --git a/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx b/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx index 7693c3ab43..2e535baeac 100644 --- a/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx +++ b/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx @@ -1,9 +1,15 @@ import type { CSSProperties, FC } from 'react' import type { ModelAndParameter } from '../types' -import type { Item } from '@/app/components/base/dropdown' -import { memo } from 'react' +import { memo, useState } from 'react' import { useTranslation } from 'react-i18next' -import Dropdown from '@/app/components/base/dropdown' +import ActionButton from '@/app/components/base/action-button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { ModelStatusEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { useDebugConfigurationContext } from '@/context/debug-configuration' import { useProviderContext } from '@/context/provider-context' @@ -35,34 +41,43 @@ const DebugItem: FC = ({ const index = multipleModelConfigs.findIndex(v => v.id === modelAndParameter.id) const currentProvider = textGenerationModelList.find(item => item.provider === modelAndParameter.provider) const currentModel = currentProvider?.models.find(item => item.model === modelAndParameter.model) + const [open, setOpen] = useState(false) - const handleSelect = (item: Item) => { - if (item.value === 'duplicate') { - if (multipleModelConfigs.length >= 4) - return + const handleDuplicate = () => { + setOpen(false) + if (multipleModelConfigs.length >= 4) + return - onMultipleModelConfigsChange( - true, - [ - ...multipleModelConfigs.slice(0, index + 1), - { - ...modelAndParameter, - id: `${Date.now()}`, - }, - ...multipleModelConfigs.slice(index + 1), - ], - ) - } - if (item.value === 'debug-as-single-model') - onDebugWithMultipleModelChange(modelAndParameter) - if (item.value === 'remove') { - onMultipleModelConfigsChange( - true, - multipleModelConfigs.filter(item => item.id !== modelAndParameter.id), - ) - } + onMultipleModelConfigsChange( + true, + [ + ...multipleModelConfigs.slice(0, index + 1), + { + ...modelAndParameter, + id: `${Date.now()}`, + }, + ...multipleModelConfigs.slice(index + 1), + ], + ) } + const handleDebugAsSingleModel = () => { + setOpen(false) + onDebugWithMultipleModelChange(modelAndParameter) + } + + const handleRemove = () => { + setOpen(false) + onMultipleModelConfigsChange( + true, + multipleModelConfigs.filter(item => item.id !== modelAndParameter.id), + ) + } + + const showDuplicate = multipleModelConfigs.length <= 3 + const showDebugAsSingleModel = !!(modelAndParameter.provider && modelAndParameter.model) + const showRemove = multipleModelConfigs.length > 2 + return (
= ({ - 2 - ? [ - { - value: 'remove', - text: t('operation.remove', { ns: 'common' }) as string, - }, - ] - : undefined - } - /> + + }> + + + + + + {showDuplicate && ( + + {t('duplicateModel', { ns: 'appDebug' })} + + )} + {showDebugAsSingleModel && ( + + {t('debugAsSingleModel', { ns: 'appDebug' })} + + )} + {showRemove && ( + <> + {(showDuplicate || showDebugAsSingleModel) && } + + {t('operation.remove', { ns: 'common' })} + + + )} + +
{ diff --git a/web/app/components/base/action-button/index.tsx b/web/app/components/base/action-button/index.tsx index ec84fa06d8..86c8933a8a 100644 --- a/web/app/components/base/action-button/index.tsx +++ b/web/app/components/base/action-button/index.tsx @@ -30,7 +30,7 @@ const actionButtonVariants = cva( }, ) -export type ActionButtonProps = { +type ActionButtonProps = { size?: 'xs' | 's' | 'm' | 'l' | 'xl' state?: ActionButtonState styleCss?: CSSProperties @@ -73,4 +73,4 @@ const ActionButton = ({ className, size, state = ActionButtonState.Default, styl ActionButton.displayName = 'ActionButton' export default ActionButton -export { ActionButton, ActionButtonState, actionButtonVariants } +export { ActionButton, ActionButtonState } diff --git a/web/app/components/base/chat/chat-with-history/__tests__/header-in-mobile.spec.tsx b/web/app/components/base/chat/chat-with-history/__tests__/header-in-mobile.spec.tsx index a183a7670b..af58a29fcc 100644 --- a/web/app/components/base/chat/chat-with-history/__tests__/header-in-mobile.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/__tests__/header-in-mobile.spec.tsx @@ -211,8 +211,9 @@ describe('HeaderInMobile', () => { fireEvent.click(screen.getByText(/share\.chat\.viewChatSettings/i)) // Check if chat settings overlay is open - // Check if chat settings overlay is open - expect(screen.getByTestId('mobile-chat-settings-overlay'))!.toBeInTheDocument() + await waitFor(() => { + expect(screen.getByTestId('mobile-chat-settings-overlay')).toBeInTheDocument() + }) // Close chat settings via overlay click fireEvent.click(screen.getByTestId('mobile-chat-settings-overlay')) @@ -236,7 +237,9 @@ describe('HeaderInMobile', () => { }) fireEvent.click(screen.getByText(/share\.chat\.viewChatSettings/i)) - expect(screen.getByTestId('mobile-chat-settings-overlay'))!.toBeInTheDocument() + await waitFor(() => { + expect(screen.getByTestId('mobile-chat-settings-overlay')).toBeInTheDocument() + }) // Click inside the settings panel (find the title) const settingsTitle = screen.getByText(/share\.chat\.chatSettingsTitle/i) @@ -282,7 +285,9 @@ describe('HeaderInMobile', () => { }) fireEvent.click(screen.getByText(/share\.chat\.resetChat/i)) - expect(handleNewConversation).toHaveBeenCalled() + await waitFor(() => { + expect(handleNewConversation).toHaveBeenCalled() + }) }) it('should handle pin conversation', async () => { @@ -348,8 +353,7 @@ describe('HeaderInMobile', () => { fireEvent.click(screen.getByText(/explore\.sidebar\.action\.rename/i)) // RenameModal should be visible - // RenameModal should be visible - expect(screen.getByRole('dialog'))!.toBeInTheDocument() + expect(await screen.findByRole('dialog')).toBeInTheDocument() const input = screen.getByDisplayValue('Conv 1') fireEvent.change(input, { target: { value: 'New Name' } }) @@ -377,8 +381,7 @@ describe('HeaderInMobile', () => { fireEvent.click(screen.getByText(/explore\.sidebar\.action\.rename/i)) // RenameModal should be visible - // RenameModal should be visible - expect(screen.getByRole('dialog'))!.toBeInTheDocument() + expect(await screen.findByRole('dialog')).toBeInTheDocument() // Click cancel button const cancelButton = screen.getByRole('button', { name: /common\.operation\.cancel/i }) @@ -410,8 +413,7 @@ describe('HeaderInMobile', () => { fireEvent.click(screen.getByText(/explore\.sidebar\.action\.rename/i)) // RenameModal should be visible with loading state - // RenameModal should be visible with loading state - expect(screen.getByRole('dialog'))!.toBeInTheDocument() + expect(await screen.findByRole('dialog')).toBeInTheDocument() }) it('should handle delete conversation', async () => { diff --git a/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx b/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx index 295bebecac..525f9b89a5 100644 --- a/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -53,11 +53,16 @@ describe('MobileOperationDropdown Component', () => { // Reset Chat await user.click(screen.getByText('share.chat.resetChat')) - expect(defaultProps.handleResetChat).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.handleResetChat).toHaveBeenCalledTimes(1) + }) + await user.click(screen.getByRole('button')) // View Chat Settings await user.click(screen.getByText('share.chat.viewChatSettings')) - expect(defaultProps.handleViewChatSettings).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.handleViewChatSettings).toHaveBeenCalledTimes(1) + }) }) it('applies hover state to ActionButton when open', async () => { @@ -72,4 +77,16 @@ describe('MobileOperationDropdown Component', () => { await user.click(trigger) expect(trigger).toHaveClass('action-btn-hover') }) + + it('closes the menu after clicking an action', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('share.chat.resetChat')) + + await waitFor(() => { + expect(screen.queryByText('share.chat.resetChat')).not.toBeInTheDocument() + }) + }) }) diff --git a/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx b/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx index 454f20066e..294d5eebc5 100644 --- a/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -74,12 +74,18 @@ describe('Operation Component', () => { expect(defaultProps.togglePin).toHaveBeenCalledTimes(1) // Rename + await user.click(screen.getByText('Chat Title')) await user.click(screen.getByText('explore.sidebar.action.rename')) - expect(defaultProps.onRenameConversation).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.onRenameConversation).toHaveBeenCalledTimes(1) + }) // Delete + await user.click(screen.getByText('Chat Title')) await user.click(screen.getByText('explore.sidebar.action.delete')) - expect(defaultProps.onDelete).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.onDelete).toHaveBeenCalledTimes(1) + }) }) it('applies hover background when open', async () => { diff --git a/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx b/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx index 43cf25908c..5d80db7ac3 100644 --- a/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx +++ b/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx @@ -1,7 +1,12 @@ -import { useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton, { ActionButtonState } from '@/app/components/base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { handleResetChat: () => void @@ -16,40 +21,45 @@ const MobileOperationDropdown = ({ }: Props) => { const { t } = useTranslation() const [open, setOpen] = useState(false) + const handleMenuAction = useCallback((callback: () => void) => { + setOpen(false) + queueMicrotask(callback) + }, []) return ( - - setOpen(v => !v)} + } data-testid="mobile-more-btn" >
- - -
+ + handleMenuAction(handleResetChat)} > -
- {t('chat.resetChat', { ns: 'share' })} -
- {!hideViewChatSettings && ( -
- {t('chat.viewChatSettings', { ns: 'share' })} -
- )} -
- - + {t('chat.resetChat', { ns: 'share' })} + + {!hideViewChatSettings && ( + handleMenuAction(handleViewChatSettings)} + > + {t('chat.viewChatSettings', { ns: 'share' })} + + )} + + ) } diff --git a/web/app/components/base/chat/chat-with-history/header/operation.tsx b/web/app/components/base/chat/chat-with-history/header/operation.tsx index d61af1f6a1..d439a43c1f 100644 --- a/web/app/components/base/chat/chat-with-history/header/operation.tsx +++ b/web/app/components/base/chat/chat-with-history/header/operation.tsx @@ -1,14 +1,16 @@ 'use client' -import type { Placement } from '@floating-ui/react' import type { FC } from 'react' +import type { Placement } from '@/app/components/base/ui/placement' import { cn } from '@langgenius/dify-ui/cn' -import { - RiArrowDownSLine, -} from '@remixicon/react' import * as React from 'react' -import { useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { title: string @@ -33,42 +35,51 @@ const Operation: FC = ({ }) => { const { t } = useTranslation() const [open, setOpen] = useState(false) + const handleDeferredAction = useCallback((action: () => void) => { + setOpen(false) + queueMicrotask(action) + }, []) return ( - - setOpen(v => !v)} + } >
{title}
- +
- - -
-
- {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} -
- {isShowRenameConversation && ( -
- {t('sidebar.action.rename', { ns: 'explore' })} -
- )} - {isShowDelete && ( -
- {t('sidebar.action.delete', { ns: 'explore' })} -
- )} -
- - + + + + {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} + + {isShowRenameConversation && ( + onRenameConversation && handleDeferredAction(onRenameConversation)} + > + {t('sidebar.action.rename', { ns: 'explore' })} + + )} + {isShowDelete && ( + handleDeferredAction(onDelete)} + > + {t('sidebar.action.delete', { ns: 'explore' })} + + )} + + ) } export default React.memo(Operation) diff --git a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx index e46b54872e..5aa8da7965 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx @@ -1,16 +1,9 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import Operation from '../operation' -// Mock PortalToFollowElem components to render children in place -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) =>
{children}
, - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) =>
{children}
, - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) =>
{children}
, -})) - describe('Operation', () => { const defaultProps = { isActive: false, @@ -72,7 +65,9 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) await user.click(screen.getByText('explore.sidebar.action.rename')) - expect(defaultProps.onRenameConversation).toHaveBeenCalled() + await waitFor(() => { + expect(defaultProps.onRenameConversation).toHaveBeenCalled() + }) }) it('should call onDelete when delete is clicked', async () => { @@ -82,7 +77,9 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) await user.click(screen.getByText('explore.sidebar.action.delete')) - expect(defaultProps.onDelete).toHaveBeenCalled() + await waitFor(() => { + expect(defaultProps.onDelete).toHaveBeenCalled() + }) }) it('should respect visibility props', async () => { @@ -108,8 +105,7 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) - const portalContent = screen.getByTestId('portal-content') - expect(portalContent).toBeInTheDocument() + expect(screen.getByText('explore.sidebar.action.pin')).toBeInTheDocument() }) it('should close dropdown when item hovering stops', async () => { @@ -120,5 +116,60 @@ describe('Operation', () => { expect(screen.getByText('explore.sidebar.action.pin')).toBeInTheDocument() rerender() + + await waitFor(() => { + expect(screen.queryByText('explore.sidebar.action.pin')).not.toBeInTheDocument() + }) + }) + + it('should keep the trigger mounted while visually hidden', () => { + render() + + const trigger = screen.getByRole('button') + expect(trigger).toHaveClass('pointer-events-none') + expect(trigger).toHaveClass('opacity-0') + }) + + it('should safely ignore rename clicks when callback is missing', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('explore.sidebar.action.rename')) + + await waitFor(() => { + expect(screen.queryByText('explore.sidebar.action.rename')).not.toBeInTheDocument() + }) + }) + + it('should not bubble trigger clicks to the parent container', async () => { + const user = userEvent.setup() + const parentClick = vi.fn() + + render( +
+ +
, + ) + + await user.click(screen.getByRole('button')) + + expect(parentClick).not.toHaveBeenCalled() + }) + + it('should not bubble popup clicks to the parent container', async () => { + const user = userEvent.setup() + const parentClick = vi.fn() + + render( +
+ +
, + ) + + await user.click(screen.getByRole('button')) + await user.click(screen.getByRole('menu')) + + expect(parentClick).not.toHaveBeenCalled() }) }) diff --git a/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx b/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx index 18ff3e4d62..adda03fb55 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx @@ -1,19 +1,17 @@ 'use client' import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { - RiDeleteBinLine, - RiEditLine, - RiMoreFill, - RiPushpinLine, - RiUnpinLine, -} from '@remixicon/react' import { useBoolean } from 'ahooks' import * as React from 'react' -import { useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton, { ActionButtonState } from '@/app/components/base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { isActive?: boolean @@ -38,24 +36,29 @@ const Operation: FC = ({ }) => { const { t } = useTranslation() const [open, setOpen] = useState(false) - const ref = useRef(null) const [isHovering, { setTrue: setIsHovering, setFalse: setNotHovering }] = useBoolean(false) useEffect(() => { if (!isItemHovering && !isHovering) setOpen(false) }, [isItemHovering, isHovering]) + const handleDeferredAction = useCallback((action?: () => void) => { + if (!action) + return + setOpen(false) + queueMicrotask(action) + }, []) return ( - - setOpen(v => !v)} + } + onClick={e => e.stopPropagation()} > = ({ : ActionButtonState.Default } > - + - - -
+ e.stopPropagation(), + }} + > + { e.stopPropagation() + togglePin() }} > -
- {isPinned && } - {!isPinned && } - {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} -
- {isShowRenameConversation && ( -
- - {t('sidebar.action.rename', { ns: 'explore' })} -
- )} - {isShowDelete && ( -
- - {t('sidebar.action.delete', { ns: 'explore' })} -
- )} -
- - + {isPinned && } + {!isPinned && } + {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} + + {isShowRenameConversation && ( + { + e.stopPropagation() + handleDeferredAction(onRenameConversation) + }} + > + + {t('sidebar.action.rename', { ns: 'explore' })} + + )} + {isShowDelete && ( + { + e.stopPropagation() + handleDeferredAction(onDelete) + }} + > + + {t('sidebar.action.delete', { ns: 'explore' })} + + )} + + ) } export default React.memo(Operation) diff --git a/web/app/components/base/dropdown/__tests__/index.spec.tsx b/web/app/components/base/dropdown/__tests__/index.spec.tsx deleted file mode 100644 index 9820554e3d..0000000000 --- a/web/app/components/base/dropdown/__tests__/index.spec.tsx +++ /dev/null @@ -1,225 +0,0 @@ -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' -import Dropdown from '../index' - -describe('Dropdown Component', () => { - const mockItems = [ - { value: 'option1', text: 'Option 1' }, - { value: 'option2', text: 'Option 2' }, - ] - const mockSecondItems = [ - { value: 'option3', text: 'Option 3' }, - ] - const onSelect = vi.fn() - - afterEach(() => { - cleanup() - vi.clearAllMocks() - }) - - it('renders default trigger properly', () => { - const { container } = render( - , - ) - const trigger = container.querySelector('button') - expect(trigger).toBeInTheDocument() - }) - - it('renders custom trigger when provided', () => { - render( - } - />, - ) - const trigger = screen.getByTestId('custom-trigger') - expect(trigger).toBeInTheDocument() - expect(trigger).toHaveTextContent('Closed') - }) - - it('opens dropdown menu on trigger click and shows items', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - // Dropdown items are rendered in a portal (document.body) - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 2')).toBeInTheDocument() - }) - - it('calls onSelect and closes dropdown when an item is clicked', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - const option1 = screen.getByText('Option 1') - await act(async () => { - fireEvent.click(option1) - }) - - expect(onSelect).toHaveBeenCalledWith(mockItems[0]) - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - }) - - it('calls onSelect and closes dropdown when a second item is clicked', async () => { - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const option3 = screen.getByText('Option 3') - await act(async () => { - fireEvent.click(option3) - }) - expect(onSelect).toHaveBeenCalledWith(mockSecondItems[0]) - expect(screen.queryByText('Option 3')).not.toBeInTheDocument() - }) - - it('renders second items and divider when provided', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - - // Check for divider (h-px bg-divider-regular) - const divider = document.body.querySelector('.bg-divider-regular.h-px') - expect(divider).toBeInTheDocument() - }) - - it('applies custom classNames', async () => { - const popupClass = 'custom-popup' - const itemClass = 'custom-item' - const secondItemClass = 'custom-second-item' - - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const popup = document.body.querySelector(`.${popupClass}`) - expect(popup).toBeInTheDocument() - - const items = screen.getAllByText('Option 1') - expect(items[0]).toHaveClass(itemClass) - - const secondItems = screen.getAllByText('Option 3') - expect(secondItems[0]).toHaveClass(secondItemClass) - }) - - it('applies open class to trigger when menu is open', async () => { - render() - const trigger = screen.getByRole('button') - await act(async () => { - fireEvent.click(trigger) - }) - expect(trigger).toHaveClass('bg-divider-regular') - }) - - it('handles JSX elements as item text', async () => { - const itemsWithJSX = [ - { value: 'jsx', text: JSX Content }, - ] - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - expect(screen.getByTestId('jsx-item')).toBeInTheDocument() - expect(screen.getByText('JSX Content')).toBeInTheDocument() - }) - - it('does not render items section if items list is empty', async () => { - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const p1Divs = document.body.querySelectorAll('.p-1') - expect(p1Divs.length).toBe(1) - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - }) - - it('does not render divider if only one section is provided', async () => { - const { rerender } = render( - , - ) - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - expect(document.body.querySelector('.bg-divider-regular.h-px')).not.toBeInTheDocument() - - await act(async () => { - rerender( - , - ) - }) - expect(document.body.querySelector('.bg-divider-regular.h-px')).not.toBeInTheDocument() - }) - - it('renders nothing if both item lists are empty', async () => { - render() - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - const popup = document.body.querySelector('.bg-components-panel-bg') - expect(popup?.children.length).toBe(0) - }) - - it('passes triggerProps to ActionButton and applies custom className', () => { - render( - , - ) - const trigger = screen.getByLabelText('dropdown-trigger') - expect(trigger).toBeDisabled() - expect(trigger).toHaveClass('custom-trigger-class') - }) -}) diff --git a/web/app/components/base/dropdown/index.stories.tsx b/web/app/components/base/dropdown/index.stories.tsx deleted file mode 100644 index 7cb7f820f6..0000000000 --- a/web/app/components/base/dropdown/index.stories.tsx +++ /dev/null @@ -1,88 +0,0 @@ -import type { Meta, StoryObj } from '@storybook/nextjs-vite' -import type { Item } from '.' -import { useState } from 'react' -import { fn } from 'storybook/test' -import Dropdown from '.' - -const PRIMARY_ITEMS: Item[] = [ - { value: 'rename', text: 'Rename' }, - { value: 'duplicate', text: 'Duplicate' }, -] - -const SECONDARY_ITEMS: Item[] = [ - { value: 'archive', text: Archive }, - { value: 'delete', text: Delete }, -] - -const meta = { - title: 'Base/Navigation/Dropdown', - component: Dropdown, - parameters: { - docs: { - description: { - component: 'Small contextual menu with optional destructive section. Uses portal positioning utilities for precise placement.', - }, - }, - }, - tags: ['autodocs'], - args: { - items: PRIMARY_ITEMS, - secondItems: SECONDARY_ITEMS, - }, -} satisfies Meta - -export default meta -type Story = StoryObj - -const DropdownDemo = (props: React.ComponentProps) => { - const [lastAction, setLastAction] = useState('None') - - return ( -
- { - setLastAction(String(item.value)) - props.onSelect?.(item) - }} - /> -
- Last action: - {' '} - {lastAction} -
-
- ) -} - -export const Playground: Story = { - render: args => , - args: { - items: PRIMARY_ITEMS, - secondItems: SECONDARY_ITEMS, - onSelect: fn(), - }, -} - -export const CustomTrigger: Story = { - render: args => ( - ( - - )} - /> - ), - args: { - items: PRIMARY_ITEMS, - onSelect: fn(), - }, -} diff --git a/web/app/components/base/dropdown/index.tsx b/web/app/components/base/dropdown/index.tsx deleted file mode 100644 index f9a19f34ea..0000000000 --- a/web/app/components/base/dropdown/index.tsx +++ /dev/null @@ -1,122 +0,0 @@ -import type { FC } from 'react' -import type { ActionButtonProps } from '@/app/components/base/action-button' -import { cn } from '@langgenius/dify-ui/cn' -import { - RiMoreFill, -} from '@remixicon/react' -import { useState } from 'react' -import ActionButton from '@/app/components/base/action-button' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' - -export type Item = { - value: string | number - text: string | React.JSX.Element -} -type DropdownProps = { - items: Item[] - secondItems?: Item[] - onSelect: (item: Item) => void - renderTrigger?: (open: boolean) => React.ReactNode - triggerProps?: ActionButtonProps - popupClassName?: string - itemClassName?: string - secondItemClassName?: string -} -const Dropdown: FC = ({ - items, - onSelect, - secondItems, - renderTrigger, - triggerProps, - popupClassName, - itemClassName, - secondItemClassName, -}) => { - const [open, setOpen] = useState(false) - - const handleSelect = (item: Item) => { - setOpen(false) - onSelect(item) - } - - return ( - - setOpen(v => !v)}> - { - renderTrigger - ? renderTrigger(open) - : ( - - - - ) - } - - -
- { - !!items.length && ( -
- { - items.map(item => ( -
handleSelect(item)} - > - {item.text} -
- )) - } -
- ) - } - { - (!!items.length && !!secondItems?.length) && ( -
- ) - } - { - !!secondItems?.length && ( -
- { - secondItems.map(item => ( -
handleSelect(item)} - > - {item.text} -
- )) - } -
- ) - } -
- - - ) -} - -export default Dropdown diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/__tests__/index.spec.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/__tests__/index.spec.tsx index 24a241ff93..906a9e01e0 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/__tests__/index.spec.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/__tests__/index.spec.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { fireEvent, render, screen } from '@testing-library/react' import * as React from 'react' import Breadcrumbs from '../index' @@ -44,6 +44,16 @@ const resetMockStoreState = () => { mockStoreState.setBucket = vi.fn() } +const getDropdownTrigger = () => { + return document.querySelector('[aria-haspopup="menu"]') as HTMLElement | null +} + +const openCollapsedBreadcrumbDropdown = () => { + const dropdownTrigger = getDropdownTrigger() + expect(dropdownTrigger).toBeInTheDocument() + fireEvent.click(dropdownTrigger as HTMLElement) +} + describe('Breadcrumbs', () => { beforeEach(() => { vi.clearAllMocks() @@ -437,15 +447,11 @@ describe('Breadcrumbs', () => { render() // Act - Click on dropdown trigger (the ... button) - const dropdownTrigger = screen.getAllByRole('button').find(btn => btn.querySelector('svg')) - if (dropdownTrigger) - fireEvent.click(dropdownTrigger) + openCollapsedBreadcrumbDropdown() // Assert - Collapsed breadcrumbs should be visible - await waitFor(() => { - expect(screen.getByText('folder3'))!.toBeInTheDocument() - expect(screen.getByText('folder4'))!.toBeInTheDocument() - }) + expect(await screen.findByText('folder3')).toBeInTheDocument() + expect(await screen.findByText('folder4')).toBeInTheDocument() }) }) }) @@ -615,9 +621,7 @@ describe('Breadcrumbs', () => { // Assert - Should collapse because 3 > 2 // Dropdown should be present - const buttons = screen.getAllByRole('button') - const hasDropdownTrigger = buttons.some(btn => btn.querySelector('svg')) - expect(hasDropdownTrigger).toBe(true) + expect(getDropdownTrigger()).toBeInTheDocument() }) it('should use displayBreadcrumbNum=3 when isInPipeline is false', () => { @@ -647,9 +651,7 @@ describe('Breadcrumbs', () => { render() // Assert - Should collapse because 3 > 2 - const buttons = screen.getAllByRole('button') - const hasDropdownTrigger = buttons.some(btn => btn.querySelector('svg')) - expect(hasDropdownTrigger).toBe(true) + expect(getDropdownTrigger()).toBeInTheDocument() }) }) }) @@ -722,23 +724,16 @@ describe('Breadcrumbs', () => { render() // Act - Click dropdown to see collapsed items - const dropdownTrigger = screen.getAllByRole('button').find(btn => btn.querySelector('svg')) - if (dropdownTrigger) - fireEvent.click(dropdownTrigger) + openCollapsedBreadcrumbDropdown() // prefixBreadcrumbs = ['f1', 'f2'] // collapsedBreadcrumbs = ['f3', 'f4'] // lastBreadcrumb = 'f5' - // prefixBreadcrumbs = ['f1', 'f2'] - // collapsedBreadcrumbs = ['f3', 'f4'] - // lastBreadcrumb = 'f5' - expect(screen.getByText('f1'))!.toBeInTheDocument() - expect(screen.getByText('f2'))!.toBeInTheDocument() - expect(screen.getByText('f5'))!.toBeInTheDocument() - await waitFor(() => { - expect(screen.getByText('f3'))!.toBeInTheDocument() - expect(screen.getByText('f4'))!.toBeInTheDocument() - }) + expect(screen.getByText('f1')).toBeInTheDocument() + expect(screen.getByText('f2')).toBeInTheDocument() + expect(screen.getByText('f5')).toBeInTheDocument() + expect(await screen.findByText('f3')).toBeInTheDocument() + expect(await screen.findByText('f4')).toBeInTheDocument() }) it('should not collapse when breadcrumbs.length <= displayBreadcrumbNum', () => { @@ -883,15 +878,8 @@ describe('Breadcrumbs', () => { render() // Act - Open dropdown and click on collapsed breadcrumb (f3, index=2) - const dropdownTrigger = screen.getAllByRole('button').find(btn => btn.querySelector('svg')) - if (dropdownTrigger) - fireEvent.click(dropdownTrigger) - - await waitFor(() => { - expect(screen.getByText('f3'))!.toBeInTheDocument() - }) - - fireEvent.click(screen.getByText('f3')) + openCollapsedBreadcrumbDropdown() + fireEvent.click(await screen.findByText('f3')) // Assert - Should slice to index 2 + 1 = 3 expect(mockStoreState.setBreadcrumbs).toHaveBeenCalledWith(['f1', 'f2', 'f3']) @@ -954,18 +942,13 @@ describe('Breadcrumbs', () => { render() // Act - Open dropdown - const dropdownTrigger = screen.getAllByRole('button').find(btn => btn.querySelector('svg')) - if (dropdownTrigger) - fireEvent.click(dropdownTrigger) + openCollapsedBreadcrumbDropdown() // Assert - First, last, and collapsed should be accessible - // Assert - First, last, and collapsed should be accessible - expect(screen.getByText('folder-0'))!.toBeInTheDocument() - expect(screen.getByText('folder-1'))!.toBeInTheDocument() - expect(screen.getByText('folder-19'))!.toBeInTheDocument() - await waitFor(() => { - expect(screen.getByText('folder-2'))!.toBeInTheDocument() - }) + expect(screen.getByText('folder-0')).toBeInTheDocument() + expect(screen.getByText('folder-1')).toBeInTheDocument() + expect(screen.getByText('folder-19')).toBeInTheDocument() + expect(await screen.findByText('folder-2')).toBeInTheDocument() }) it('should handle empty bucket string', () => { @@ -1026,9 +1009,7 @@ describe('Breadcrumbs', () => { render() // Assert - Should collapse because breadcrumbs.length > expectedNum - const buttons = screen.getAllByRole('button') - const hasDropdownTrigger = buttons.some(btn => btn.querySelector('svg')) - expect(hasDropdownTrigger).toBe(true) + expect(getDropdownTrigger()).toBeInTheDocument() }) }) diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx index f7112c243b..d57e8340e9 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx @@ -32,10 +32,10 @@ describe('Dropdown', () => { const { container } = render() - // Assert - Button should have RiMoreFill icon (rendered as svg) + // Assert - Button should have the more icon const button = screen.getByRole('button') - expect(button)!.toBeInTheDocument() - expect(container.querySelector('svg'))!.toBeInTheDocument() + expect(button).toBeInTheDocument() + expect(container.querySelector('.i-ri-more-fill')).toBeInTheDocument() }) it('should render separator after dropdown', () => { diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx index 7178b45b34..c6a90824ab 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx @@ -1,12 +1,11 @@ import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import * as React from 'react' import { useCallback, useState } from 'react' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import Menu from './menu' type DropdownProps = { @@ -22,26 +21,17 @@ const Dropdown = ({ }: DropdownProps) => { const [open, setOpen] = useState(false) - const handleTrigger = useCallback(() => { - setOpen(prev => !prev) - }, []) - const handleBreadCrumbClick = useCallback((index: number) => { onBreadcrumbClick(index) setOpen(false) }, [onBreadcrumbClick]) return ( - - + }> - - + + - + / - + ) } diff --git a/web/app/components/explore/item-operation/__tests__/index.spec.tsx b/web/app/components/explore/item-operation/__tests__/index.spec.tsx index f7f9b44a84..d54c644ab0 100644 --- a/web/app/components/explore/item-operation/__tests__/index.spec.tsx +++ b/web/app/components/explore/item-operation/__tests__/index.spec.tsx @@ -1,6 +1,81 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import * as React from 'react' import ItemOperation from '../index' +vi.mock('@/app/components/base/ui/dropdown-menu', () => { + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + onClick, + ...props + }: React.ButtonHTMLAttributes) => { + const { isOpen, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ + children, + popupProps, + }: { + children: React.ReactNode + popupProps?: React.HTMLAttributes + }) => { + const { isOpen } = useDropdownMenuContext() + if (!isOpen) + return null + + return
{children}
+ }, + DropdownMenuItem: ({ + children, + onClick, + className, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + className?: string + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) + describe('ItemOperation', () => { beforeEach(() => { vi.clearAllMocks() @@ -67,14 +142,27 @@ describe('ItemOperation', () => { expect(props.onDelete).toHaveBeenCalledTimes(1) }) + + it('should call onRenameConversation when clicking rename action', async () => { + const onRenameConversation = vi.fn() + renderComponent({ + isShowRenameConversation: true, + onRenameConversation, + }) + + fireEvent.click(screen.getByTestId('item-operation-trigger')) + fireEvent.click(await screen.findByText('explore.sidebar.action.rename')) + + expect(onRenameConversation).toHaveBeenCalledTimes(1) + }) }) describe('Edge Cases', () => { it('should close the menu when mouse leaves the panel and item is not hovering', async () => { renderComponent() fireEvent.click(screen.getByTestId('item-operation-trigger')) - const pinText = await screen.findByText('explore.sidebar.action.pin') - const menu = pinText.closest('div')?.parentElement as HTMLElement + await screen.findByText('explore.sidebar.action.pin') + const menu = screen.getByTestId('dropdown-content') fireEvent.mouseEnter(menu) fireEvent.mouseLeave(menu) @@ -83,5 +171,25 @@ describe('ItemOperation', () => { expect(screen.queryByText('explore.sidebar.action.pin')).not.toBeInTheDocument() }) }) + + it('should stop propagation when clicking inside the dropdown content', async () => { + const onParentClick = vi.fn() + + render( +
+ +
, + ) + + fireEvent.click(screen.getByTestId('item-operation-trigger')) + fireEvent.click(await screen.findByTestId('dropdown-content')) + + expect(onParentClick).not.toHaveBeenCalled() + }) }) }) diff --git a/web/app/components/explore/item-operation/index.tsx b/web/app/components/explore/item-operation/index.tsx index 94eed731fa..72bd00fa6e 100644 --- a/web/app/components/explore/item-operation/index.tsx +++ b/web/app/components/explore/item-operation/index.tsx @@ -7,10 +7,14 @@ import { } from '@remixicon/react' import { useBoolean } from 'ahooks' import * as React from 'react' -import { useEffect, useRef, useState } from 'react' +import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' - -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { Pin02 } from '../../base/icons/src/vender/line/general' import s from './style.module.css' @@ -35,61 +39,74 @@ const ItemOperation: FC = ({ isShowDelete, onDelete, }) => { - const { t } = useTranslation() + const { t } = useTranslation('explore') + const { t: tCommon } = useTranslation('common') const [open, setOpen] = useState(false) - const ref = useRef(null) const [isHovering, { setTrue: setIsHovering, setFalse: setNotHovering }] = useBoolean(false) useEffect(() => { if (!isItemHovering && !isHovering) setOpen(false) }, [isItemHovering, isHovering]) return ( - - setOpen(v => !v)} + { + e.stopPropagation() + }} > -
-
- - {tCommon('operation.more')} + + e.stopPropagation(), + }} > -
{ e.stopPropagation() + togglePin() }} > -
- - {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} -
- {isShowRenameConversation && ( -
- - {t('sidebar.action.rename', { ns: 'explore' })} -
- )} - {isShowDelete && ( -
- - {t('sidebar.action.delete', { ns: 'explore' })} -
- )} -
- - + + {isPinned ? t('sidebar.action.unpin') : t('sidebar.action.pin')} + + {isShowRenameConversation && ( + { + e.stopPropagation() + onRenameConversation?.() + }} + > + + {t('sidebar.action.rename')} + + )} + {isShowDelete && ( + { + e.stopPropagation() + onDelete() + }} + > + + {t('sidebar.action.delete')} + + )} + + ) } export default React.memo(ItemOperation) diff --git a/web/app/components/header/account-setting/data-source-page-new/__tests__/card.spec.tsx b/web/app/components/header/account-setting/data-source-page-new/__tests__/card.spec.tsx index fedb7d8a4e..bd349a3ad6 100644 --- a/web/app/components/header/account-setting/data-source-page-new/__tests__/card.spec.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/__tests__/card.spec.tsx @@ -188,10 +188,12 @@ describe('Card Component', () => { fireEvent.click(screen.getByText(/operation.edit/)) // Assert - expect(mockPluginAuthActionReturn.handleEdit).toHaveBeenCalledWith('c1', { - apiKey: 'key1', - __name__: 'Credential 1', - __credential_id__: 'c1', + await waitFor(() => { + expect(mockPluginAuthActionReturn.handleEdit).toHaveBeenCalledWith('c1', { + apiKey: 'key1', + __name__: 'Credential 1', + __credential_id__: 'c1', + }) }) }) @@ -202,7 +204,9 @@ describe('Card Component', () => { fireEvent.click(screen.getByText(/operation.remove/)) // Assert - expect(mockPluginAuthActionReturn.openConfirm).toHaveBeenCalledWith('c1') + await waitFor(() => { + expect(mockPluginAuthActionReturn.openConfirm).toHaveBeenCalledWith('c1') + }) }) it('should handle "setDefault" action from Item component', async () => { @@ -212,7 +216,9 @@ describe('Card Component', () => { fireEvent.click(screen.getByText(/auth.setDefault/)) // Assert - expect(mockPluginAuthActionReturn.handleSetDefault).toHaveBeenCalledWith('c1') + await waitFor(() => { + expect(mockPluginAuthActionReturn.handleSetDefault).toHaveBeenCalledWith('c1') + }) }) it('should handle "rename" action from Item component', async () => { @@ -231,14 +237,16 @@ describe('Card Component', () => { fireEvent.click(screen.getByText(/operation.rename/)) // Now it should show an input - const input = screen.getByPlaceholderText(/placeholder.input/) + const input = await screen.findByPlaceholderText(/placeholder.input/) fireEvent.change(input, { target: { value: 'New Name' } }) fireEvent.click(screen.getByText(/operation.save/)) // Assert - expect(mockPluginAuthActionReturn.handleRename).toHaveBeenCalledWith({ - credential_id: 'c1', - name: 'New Name', + await waitFor(() => { + expect(mockPluginAuthActionReturn.handleRename).toHaveBeenCalledWith({ + credential_id: 'c1', + name: 'New Name', + }) }) }) diff --git a/web/app/components/header/account-setting/data-source-page-new/__tests__/item.spec.tsx b/web/app/components/header/account-setting/data-source-page-new/__tests__/item.spec.tsx index b88b3a8bb7..e1c5c3f4c7 100644 --- a/web/app/components/header/account-setting/data-source-page-new/__tests__/item.spec.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/__tests__/item.spec.tsx @@ -1,5 +1,5 @@ import type { DataSourceCredential } from '../types' -import { fireEvent, render, screen } from '@testing-library/react' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' import Item from '../item' @@ -14,6 +14,9 @@ const triggerRename = async () => { fireEvent.click(dropdownTrigger) const renameOption = await screen.findByText('common.operation.rename') fireEvent.click(renameOption) + await waitFor(() => { + expect(screen.getByPlaceholderText('common.placeholder.input')).toBeInTheDocument() + }) } describe('Item Component', () => { diff --git a/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx b/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx index 0140626dd7..12042acfa1 100644 --- a/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx @@ -1,5 +1,6 @@ import type { DataSourceCredential } from '../types' -import { fireEvent, render, screen } from '@testing-library/react' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' import Operator from '../operator' @@ -9,10 +10,6 @@ import Operator from '../operator' */ // Helper to open dropdown -const openDropdown = () => { - fireEvent.click(screen.getByRole('button')) -} - describe('Operator Component', () => { const mockOnAction = vi.fn() const mockOnRename = vi.fn() @@ -37,7 +34,7 @@ describe('Operator Component', () => { // Act render() - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) // Assert expect(await screen.findByText('plugin.auth.setDefault')).toBeInTheDocument() @@ -53,7 +50,7 @@ describe('Operator Component', () => { // Act render() - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) // Assert expect(await screen.findByText('plugin.auth.setDefault')).toBeInTheDocument() @@ -71,11 +68,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.rename')) // Assert - expect(mockOnRename).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(mockOnRename).toHaveBeenCalledTimes(1) + }) expect(mockOnAction).not.toHaveBeenCalled() }) @@ -85,7 +84,7 @@ describe('Operator Component', () => { render() // Act & Assert - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) const renameBtn = await screen.findByText('common.operation.rename') expect(() => fireEvent.click(renameBtn)).not.toThrow() }) @@ -96,11 +95,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('plugin.auth.setDefault')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('setDefault', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('setDefault', credential) + }) }) it('should call onAction for "edit" action', async () => { @@ -109,11 +110,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.edit')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('edit', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('edit', credential) + }) }) it('should call onAction for "change" action', async () => { @@ -122,11 +125,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.dataSource.notion.changeAuthorizedPages')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('change', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('change', credential) + }) }) it('should call onAction for "delete" action', async () => { @@ -135,11 +140,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.remove')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('delete', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('delete', credential) + }) }) }) }) diff --git a/web/app/components/header/account-setting/data-source-page-new/operator.tsx b/web/app/components/header/account-setting/data-source-page-new/operator.tsx index c94f1bb192..437fa80139 100644 --- a/web/app/components/header/account-setting/data-source-page-new/operator.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/operator.tsx @@ -1,21 +1,20 @@ import type { DataSourceCredential, } from './types' -import type { Item } from '@/app/components/base/dropdown' -import { - RiDeleteBinLine, - RiEditLine, - RiEqualizer2Line, - RiHome9Line, - RiStickyNoteAddLine, -} from '@remixicon/react' import { memo, useCallback, - useMemo, + useState, } from 'react' import { useTranslation } from 'react-i18next' -import Dropdown from '@/app/components/base/dropdown' +import ActionButton from '@/app/components/base/action-button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' type OperatorProps = { @@ -29,106 +28,60 @@ const Operator = ({ onRename, }: OperatorProps) => { const { t } = useTranslation() + const [open, setOpen] = useState(false) const { type, } = credentialItem - const items = useMemo(() => { - const commonItems = [ - { - value: 'setDefault', - text: ( -
- -
{t('auth.setDefault', { ns: 'plugin' })}
-
- ), - }, - ...( - type === CredentialTypeEnum.OAUTH2 - ? [ - { - value: 'rename', - text: ( -
- -
{t('operation.rename', { ns: 'common' })}
-
- ), - }, - ] - : [] - ), - ...( - type === CredentialTypeEnum.API_KEY - ? [ - { - value: 'edit', - text: ( -
- -
{t('operation.edit', { ns: 'common' })}
-
- ), - }, - ] - : [] - ), - ] - if (type === CredentialTypeEnum.OAUTH2) { - const oAuthItems = [ - { - value: 'change', - text: ( -
- -
{t('dataSource.notion.changeAuthorizedPages', { ns: 'common' })}
-
- ), - }, - ] - commonItems.push(...oAuthItems) - } - return commonItems - }, [t, type]) - - const secondItems = useMemo(() => { - return [ - { - value: 'delete', - text: ( -
- -
- {t('operation.remove', { ns: 'common' })} -
-
- ), - }, - ] - }, []) - const handleSelect = useCallback((item: Item) => { - if (item.value === 'rename') { - onRename?.() - return - } - onAction( - item.value as string, - credentialItem, - ) - }, [onAction, credentialItem, onRename]) + const handleAction = useCallback((action: string) => { + setOpen(false) + queueMicrotask(() => { + if (action === 'rename') { + onRename?.() + return + } + onAction(action, credentialItem) + }) + }, [credentialItem, onAction, onRename]) return ( - + + }> + + + + + + handleAction('setDefault')}> + +
{t('auth.setDefault', { ns: 'plugin' })}
+ + {type === CredentialTypeEnum.OAUTH2 && ( + handleAction('rename')}> + +
{t('operation.rename', { ns: 'common' })}
+ + )} + {type === CredentialTypeEnum.API_KEY && ( + handleAction('edit')}> + +
{t('operation.edit', { ns: 'common' })}
+ + )} + {type === CredentialTypeEnum.OAUTH2 && ( + handleAction('change')}> + +
{t('dataSource.notion.changeAuthorizedPages', { ns: 'common' })}
+ + )} + + handleAction('delete')}> + +
+ {t('operation.remove', { ns: 'common' })} +
+ + + ) } diff --git a/web/app/components/header/account-setting/members-page/operation/index.tsx b/web/app/components/header/account-setting/members-page/operation/index.tsx index 5fb3be0195..dab0c84e5b 100644 --- a/web/app/components/header/account-setting/members-page/operation/index.tsx +++ b/web/app/components/header/account-setting/members-page/operation/index.tsx @@ -1,10 +1,15 @@ 'use client' import type { Member } from '@/models/common' -import { CheckIcon, ChevronDownIcon } from '@heroicons/react/24/outline' import { cn } from '@langgenius/dify-ui/cn' import { memo, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { toast } from '@/app/components/base/ui/toast' import { useProviderContext } from '@/context/provider-context' import { deleteMemberOrCancelInvitation, updateMemberRole } from '@/service/common' @@ -74,40 +79,50 @@ const Operation = ({ member, operatorRole, onOperate }: IOperationProps) => { } } return ( - - setOpen(prev => !prev)}> -
- {RoleMap[member.role] || RoleMap.normal} - -
- - -
-
- {roleList.map(role => ( -
handleUpdateMemberRole(role)}> - {role === member.role - ? - :
} -
-
{t(roleI18nKeyMap[role].label, { ns: 'common' })}
-
{t(roleI18nKeyMap[role].tip, { ns: 'common' })}
-
-
- ))} -
-
-
-
+ + } + > + {RoleMap[member.role] || RoleMap.normal} + + + +
+ {roleList.map(role => ( + handleUpdateMemberRole(role)} + > + {role === member.role + ? + : }
-
{t('members.removeFromTeam', { ns: 'common' })}
-
{t('members.removeFromTeamTip', { ns: 'common' })}
+
{t(roleI18nKeyMap[role].label, { ns: 'common' })}
+
{t(roleI18nKeyMap[role].tip, { ns: 'common' })}
-
-
+ + ))}
- - + +
+ + +
+
{t('members.removeFromTeam', { ns: 'common' })}
+
{t('members.removeFromTeamTip', { ns: 'common' })}
+
+ +
+ + ) } export default memo(Operation) diff --git a/web/app/components/plugins/marketplace/sort-dropdown/__tests__/index.spec.tsx b/web/app/components/plugins/marketplace/sort-dropdown/__tests__/index.spec.tsx index 4d93726c4c..990bb321de 100644 --- a/web/app/components/plugins/marketplace/sort-dropdown/__tests__/index.spec.tsx +++ b/web/app/components/plugins/marketplace/sort-dropdown/__tests__/index.spec.tsx @@ -1,15 +1,12 @@ -import { fireEvent, render, screen, within } from '@testing-library/react' +import type { + MouseEventHandler, + ReactNode, +} from 'react' +import { render, screen, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { beforeEach, describe, expect, it, vi } from 'vitest' import SortDropdown from '../index' -// ================================ -// Mock external dependencies only -// ================================ - -// Mock i18n translation hook const mockTranslation = vi.fn((key: string, options?: { ns?: string }) => { - // Build full key with namespace prefix if provided const fullKey = options?.ns ? `${options.ns}.${key}` : key const translations: Record = { 'plugin.marketplace.sortBy': 'Sort by', @@ -27,7 +24,6 @@ vi.mock('#i18n', () => ({ }), })) -// Mock marketplace atoms with controllable values let mockSort: { sortBy: string, sortOrder: string } = { sortBy: 'install_count', sortOrder: 'DESC' } const mockHandleSortChange = vi.fn() @@ -35,664 +31,123 @@ vi.mock('../../atoms', () => ({ useMarketplaceSort: () => [mockSort, mockHandleSortChange], })) -// Mock portal component with controllable open state -let mockPortalOpenState = false +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open, onOpenChange: _onOpenChange }: { - children: React.ReactNode - open: boolean - onOpenChange: (open: boolean) => void - }) => { - mockPortalOpenState = open - return ( -
- {children} -
- ) - }, - PortalToFollowElemTrigger: ({ children, onClick }: { - children: React.ReactNode - onClick: () => void - }) => ( -
- {children} -
- ), - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) => { - // Match actual behavior: only render when portal is open - if (!mockPortalOpenState) - return null - return
{children}
- }, -})) + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } -// ================================ -// Test Factory Functions -// ================================ + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
+ {children} +
+ + ), + DropdownMenuTrigger: ({ children, className }: { children: ReactNode, className?: string }) => { + const { open, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuItem: ({ + children, + onClick, + className, + }: { + children: ReactNode + onClick?: MouseEventHandler + className?: string + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) -type SortOption = { - value: string - order: string - text: string -} - -const createSortOptions = (): SortOption[] => [ - { value: 'install_count', order: 'DESC', text: 'Most Popular' }, - { value: 'version_updated_at', order: 'DESC', text: 'Recently Updated' }, - { value: 'created_at', order: 'DESC', text: 'Newly Released' }, - { value: 'created_at', order: 'ASC', text: 'First Released' }, -] - -// ================================ -// SortDropdown Component Tests -// ================================ describe('SortDropdown', () => { beforeEach(() => { vi.clearAllMocks() mockSort = { sortBy: 'install_count', sortOrder: 'DESC' } - mockPortalOpenState = false }) - // ================================ - // Rendering Tests - // ================================ - describe('Rendering', () => { - it('should render without crashing', () => { - render() + it('renders the selected sort option in the trigger', () => { + render() - expect(screen.getByTestId('portal-wrapper')).toBeInTheDocument() - }) - - it('should render sort by label', () => { - render() - - expect(screen.getByText('Sort by')).toBeInTheDocument() - }) - - it('should render selected option text', () => { - render() - - expect(screen.getByText('Most Popular')).toBeInTheDocument() - }) - - it('should render arrow down icon', () => { - const { container } = render() - - const arrowIcon = container.querySelector('.h-4.w-4.text-text-tertiary') - expect(arrowIcon).toBeInTheDocument() - }) - - it('should render trigger element with correct styles', () => { - const { container } = render() - - const trigger = container.querySelector('.cursor-pointer') - expect(trigger).toBeInTheDocument() - expect(trigger).toHaveClass('h-8', 'rounded-lg', 'bg-state-base-hover-alt') - }) - - it('should not render dropdown content when closed', () => { - render() - - expect(screen.queryByTestId('portal-content')).not.toBeInTheDocument() - }) + const trigger = screen.getByTestId('dropdown-trigger') + expect(within(trigger).getByText('Sort by')).toBeInTheDocument() + expect(within(trigger).getByText('Most Popular')).toBeInTheDocument() }) - // ================================ - // State Management Tests - // ================================ - describe('State Management', () => { - it('should initialize with closed state', () => { - render() + it('falls back to the default option when the current sort is invalid', () => { + mockSort = { sortBy: 'unknown', sortOrder: 'ASC' } - const wrapper = screen.getByTestId('portal-wrapper') - expect(wrapper).toHaveAttribute('data-open', 'false') - }) + render() - it('should display correct selected option for install_count DESC', () => { - mockSort = { sortBy: 'install_count', sortOrder: 'DESC' } - render() - - expect(screen.getByText('Most Popular')).toBeInTheDocument() - }) - - it('should display correct selected option for version_updated_at DESC', () => { - mockSort = { sortBy: 'version_updated_at', sortOrder: 'DESC' } - render() - - expect(screen.getByText('Recently Updated')).toBeInTheDocument() - }) - - it('should display correct selected option for created_at DESC', () => { - mockSort = { sortBy: 'created_at', sortOrder: 'DESC' } - render() - - expect(screen.getByText('Newly Released')).toBeInTheDocument() - }) - - it('should display correct selected option for created_at ASC', () => { - mockSort = { sortBy: 'created_at', sortOrder: 'ASC' } - render() - - expect(screen.getByText('First Released')).toBeInTheDocument() - }) - - it('should toggle open state when trigger clicked', () => { - render() - - const trigger = screen.getByTestId('portal-trigger') - fireEvent.click(trigger) - - // After click, portal content should be visible - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - }) - - it('should close dropdown when trigger clicked again', () => { - render() - - const trigger = screen.getByTestId('portal-trigger') - - // Open - fireEvent.click(trigger) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - - // Close - fireEvent.click(trigger) - expect(screen.queryByTestId('portal-content')).not.toBeInTheDocument() - }) + expect(screen.getByText('Most Popular')).toBeInTheDocument() }) - // ================================ - // User Interactions Tests - // ================================ - describe('User Interactions', () => { - it('should open dropdown on trigger click', () => { - render() + it('opens the menu and renders all sort options', async () => { + const user = userEvent.setup() + render() - const trigger = screen.getByTestId('portal-trigger') - fireEvent.click(trigger) + await user.click(screen.getByTestId('dropdown-trigger')) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - }) - - it('should render all sort options when open', () => { - render() - - // Open dropdown - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - expect(within(content).getByText('Most Popular')).toBeInTheDocument() - expect(within(content).getByText('Recently Updated')).toBeInTheDocument() - expect(within(content).getByText('Newly Released')).toBeInTheDocument() - expect(within(content).getByText('First Released')).toBeInTheDocument() - }) - - it('should call handleSortChange when option clicked', () => { - render() - - // Open dropdown - fireEvent.click(screen.getByTestId('portal-trigger')) - - // Click on "Recently Updated" - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('Recently Updated')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'version_updated_at', - sortOrder: 'DESC', - }) - }) - - it('should call handleSortChange with correct params for Most Popular', () => { - mockSort = { sortBy: 'created_at', sortOrder: 'DESC' } - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('Most Popular')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'install_count', - sortOrder: 'DESC', - }) - }) - - it('should call handleSortChange with correct params for Newly Released', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('Newly Released')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'created_at', - sortOrder: 'DESC', - }) - }) - - it('should call handleSortChange with correct params for First Released', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('First Released')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'created_at', - sortOrder: 'ASC', - }) - }) - - it('should allow selecting currently selected option', () => { - mockSort = { sortBy: 'install_count', sortOrder: 'DESC' } - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('Most Popular')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'install_count', - sortOrder: 'DESC', - }) - }) - - it('should support userEvent for trigger click', async () => { - const user = userEvent.setup() - render() - - const trigger = screen.getByTestId('portal-trigger') - await user.click(trigger) - - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - }) + const content = screen.getByTestId('dropdown-content') + expect(within(content).getByText('Most Popular')).toBeInTheDocument() + expect(within(content).getByText('Recently Updated')).toBeInTheDocument() + expect(within(content).getByText('Newly Released')).toBeInTheDocument() + expect(within(content).getByText('First Released')).toBeInTheDocument() }) - // ================================ - // Check Icon Tests - // ================================ - describe('Check Icon', () => { - it('should show check icon for selected option', () => { - mockSort = { sortBy: 'install_count', sortOrder: 'DESC' } - const { container } = render() + it('shows a check icon for the currently selected option', async () => { + const user = userEvent.setup() + const { container } = render() - // Open dropdown - fireEvent.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByTestId('dropdown-trigger')) - // Check icon should be present in the dropdown - const checkIcon = container.querySelector('.text-text-accent') - expect(checkIcon).toBeInTheDocument() - }) - - it('should show check icon only for matching sortBy AND sortOrder', () => { - mockSort = { sortBy: 'created_at', sortOrder: 'DESC' } - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const options = content.querySelectorAll('.cursor-pointer') - - // "Newly Released" (created_at DESC) should have check icon - // "First Released" (created_at ASC) should NOT have check icon - expect(options.length).toBe(4) - }) - - it('should not show check icon for different sortOrder with same sortBy', () => { - mockSort = { sortBy: 'created_at', sortOrder: 'DESC' } - const { container } = render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - // Only one check icon should be visible (for Newly Released, not First Released) - const checkIcons = container.querySelectorAll('.text-text-accent') - expect(checkIcons.length).toBe(1) - }) + expect(container.querySelector('.i-ri-check-line')).toBeInTheDocument() }) - // ================================ - // Dropdown Options Structure Tests - // ================================ - describe('Dropdown Options Structure', () => { - const sortOptions = createSortOptions() + it('updates the sort and closes the menu when an option is selected', async () => { + const user = userEvent.setup() + render() - it('should render 4 sort options', () => { - render() + await user.click(screen.getByTestId('dropdown-trigger')) + await user.click(screen.getByText('Recently Updated')) - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const options = content.querySelectorAll('.cursor-pointer') - expect(options.length).toBe(4) + expect(mockHandleSortChange).toHaveBeenCalledWith({ + sortBy: 'version_updated_at', + sortOrder: 'DESC', }) - - it.each(sortOptions)('should render option: $text', ({ text }) => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - expect(within(content).getByText(text)).toBeInTheDocument() - }) - - it('should render options with unique keys', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const options = content.querySelectorAll('.cursor-pointer') - - // All options should be rendered (no key conflicts) - expect(options.length).toBe(4) - }) - - it('should render dropdown container with correct styles', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const container = content.firstChild as HTMLElement - expect(container).toHaveClass('rounded-xl', 'shadow-lg') - }) - - it('should render option items with hover styles', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const option = content.querySelector('.cursor-pointer') - expect(option).toHaveClass('hover:bg-components-panel-on-panel-item-bg-hover') - }) - }) - - // ================================ - // Edge Cases Tests - // ================================ - describe('Edge Cases', () => { - // The component falls back to the first option (Most Popular) when sort values are invalid - - it('should fallback to default option when sortBy is unknown', () => { - mockSort = { sortBy: 'unknown_field', sortOrder: 'DESC' } - - render() - - // Should fallback to first option "Most Popular" - expect(screen.getByText('Most Popular')).toBeInTheDocument() - }) - - it('should fallback to default option when sortBy is empty', () => { - mockSort = { sortBy: '', sortOrder: 'DESC' } - - render() - - expect(screen.getByText('Most Popular')).toBeInTheDocument() - }) - - it('should fallback to default option when sortOrder is unknown', () => { - mockSort = { sortBy: 'install_count', sortOrder: 'UNKNOWN' } - - render() - - expect(screen.getByText('Most Popular')).toBeInTheDocument() - }) - - it('should render correctly when handleSortChange is a no-op', () => { - mockHandleSortChange.mockImplementation(() => {}) - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('Recently Updated')) - - expect(mockHandleSortChange).toHaveBeenCalled() - }) - - it('should handle rapid toggle clicks', () => { - render() - - const trigger = screen.getByTestId('portal-trigger') - - // Rapid clicks - fireEvent.click(trigger) - fireEvent.click(trigger) - fireEvent.click(trigger) - - // Final state should be open (odd number of clicks) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - }) - - it('should handle multiple option selections', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - - // Click multiple options - fireEvent.click(within(content).getByText('Recently Updated')) - fireEvent.click(within(content).getByText('Newly Released')) - fireEvent.click(within(content).getByText('First Released')) - - expect(mockHandleSortChange).toHaveBeenCalledTimes(3) - }) - }) - - // ================================ - // Context Integration Tests - // ================================ - describe('Context Integration', () => { - it('should read sort value from context', () => { - mockSort = { sortBy: 'version_updated_at', sortOrder: 'DESC' } - render() - - expect(screen.getByText('Recently Updated')).toBeInTheDocument() - }) - - it('should call context handleSortChange on selection', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText('First Released')) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ - sortBy: 'created_at', - sortOrder: 'ASC', - }) - }) - - it('should update display when context sort changes', () => { - const { rerender } = render() - - expect(screen.getByText('Most Popular')).toBeInTheDocument() - - // Simulate context change - mockSort = { sortBy: 'created_at', sortOrder: 'ASC' } - rerender() - - expect(screen.getByText('First Released')).toBeInTheDocument() - }) - - it('should use selector pattern correctly', () => { - render() - - // Component should have called useMarketplaceContext with selector functions - expect(screen.getByTestId('portal-wrapper')).toBeInTheDocument() - }) - }) - - // ================================ - // Accessibility Tests - // ================================ - describe('Accessibility', () => { - it('should have cursor pointer on trigger', () => { - const { container } = render() - - const trigger = container.querySelector('.cursor-pointer') - expect(trigger).toBeInTheDocument() - }) - - it('should have cursor pointer on options', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const options = content.querySelectorAll('.cursor-pointer') - expect(options.length).toBeGreaterThan(0) - }) - - it('should have visible focus indicators via hover styles', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const option = content.querySelector('.hover\\:bg-components-panel-on-panel-item-bg-hover') - expect(option).toBeInTheDocument() - }) - }) - - // ================================ - // Translation Tests - // ================================ - describe('Translations', () => { - it('should call translation for sortBy label', () => { - render() - - expect(mockTranslation).toHaveBeenCalledWith('marketplace.sortBy', { ns: 'plugin' }) - }) - - it('should call translation for all sort options', () => { - render() - - expect(mockTranslation).toHaveBeenCalledWith('marketplace.sortOption.mostPopular', { ns: 'plugin' }) - expect(mockTranslation).toHaveBeenCalledWith('marketplace.sortOption.recentlyUpdated', { ns: 'plugin' }) - expect(mockTranslation).toHaveBeenCalledWith('marketplace.sortOption.newlyReleased', { ns: 'plugin' }) - expect(mockTranslation).toHaveBeenCalledWith('marketplace.sortOption.firstReleased', { ns: 'plugin' }) - }) - }) - - // ================================ - // Portal Component Integration Tests - // ================================ - describe('Portal Component Integration', () => { - it('should pass open state to PortalToFollowElem', () => { - render() - - const wrapper = screen.getByTestId('portal-wrapper') - expect(wrapper).toHaveAttribute('data-open', 'false') - - fireEvent.click(screen.getByTestId('portal-trigger')) - - expect(wrapper).toHaveAttribute('data-open', 'true') - }) - - it('should render trigger content inside PortalToFollowElemTrigger', () => { - render() - - const trigger = screen.getByTestId('portal-trigger') - expect(within(trigger).getByText('Sort by')).toBeInTheDocument() - expect(within(trigger).getByText('Most Popular')).toBeInTheDocument() - }) - - it('should render options inside PortalToFollowElemContent', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - expect(within(content).getByText('Most Popular')).toBeInTheDocument() - }) - }) - - // ================================ - // Visual Style Tests - // ================================ - describe('Visual Styles', () => { - it('should apply correct trigger container styles', () => { - const { container } = render() - - const triggerDiv = container.querySelector('.flex.h-8.cursor-pointer.items-center.rounded-lg') - expect(triggerDiv).toBeInTheDocument() - }) - - it('should apply secondary text color to sort by label', () => { - const { container } = render() - - const label = container.querySelector('.text-text-secondary') - expect(label).toBeInTheDocument() - expect(label?.textContent).toBe('Sort by') - }) - - it('should apply primary text color to selected option', () => { - const { container } = render() - - const selected = container.querySelector('.text-text-primary.system-sm-medium') - expect(selected).toBeInTheDocument() - }) - - it('should apply tertiary text color to arrow icon', () => { - const { container } = render() - - const arrow = container.querySelector('.text-text-tertiary') - expect(arrow).toBeInTheDocument() - }) - - it('should apply accent text color to check icon when option selected', () => { - mockSort = { sortBy: 'install_count', sortOrder: 'DESC' } - const { container } = render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const checkIcon = container.querySelector('.text-text-accent') - expect(checkIcon).toBeInTheDocument() - }) - - it('should apply blur-sm backdrop to dropdown container', () => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - const container = content.querySelector('.backdrop-blur-xs') - expect(container).toBeInTheDocument() - }) - }) - - // ================================ - // All Sort Options Click Tests - // ================================ - describe('All Sort Options Click Handlers', () => { - const testCases = [ - { text: 'Most Popular', sortBy: 'install_count', sortOrder: 'DESC' }, - { text: 'Recently Updated', sortBy: 'version_updated_at', sortOrder: 'DESC' }, - { text: 'Newly Released', sortBy: 'created_at', sortOrder: 'DESC' }, - { text: 'First Released', sortBy: 'created_at', sortOrder: 'ASC' }, - ] - - it.each(testCases)( - 'should call handleSortChange with { sortBy: "$sortBy", sortOrder: "$sortOrder" } when clicking "$text"', - ({ text, sortBy, sortOrder }) => { - render() - - fireEvent.click(screen.getByTestId('portal-trigger')) - - const content = screen.getByTestId('portal-content') - fireEvent.click(within(content).getByText(text)) - - expect(mockHandleSortChange).toHaveBeenCalledWith({ sortBy, sortOrder }) - }, - ) + expect(screen.queryByTestId('dropdown-content')).not.toBeInTheDocument() }) }) diff --git a/web/app/components/plugins/marketplace/sort-dropdown/index.tsx b/web/app/components/plugins/marketplace/sort-dropdown/index.tsx index 99f5650a71..a47143de02 100644 --- a/web/app/components/plugins/marketplace/sort-dropdown/index.tsx +++ b/web/app/components/plugins/marketplace/sort-dropdown/index.tsx @@ -1,15 +1,12 @@ 'use client' import { useTranslation } from '#i18n' -import { - RiArrowDownSLine, - RiCheckLine, -} from '@remixicon/react' import { useState } from 'react' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useMarketplaceSort } from '../atoms' const SortDropdown = () => { @@ -38,50 +35,44 @@ const SortDropdown = () => { ] const [sort, handleSortChange] = useMarketplaceSort() const [open, setOpen] = useState(false) - const selectedOption = options.find(option => option.value === sort.sortBy && option.order === sort.sortOrder) ?? options[0] + const selectedOption = options.find(option => option.value === sort.sortBy && option.order === sort.sortOrder) ?? options[0]! return ( - - setOpen(v => !v)}> -
- - {t('marketplace.sortBy', { ns: 'plugin' })} - - - {selectedOption!.text} - - -
- - -
- { - options.map(option => ( -
handleSortChange({ sortBy: option.value, sortOrder: option.order })} - > - {option.text} - { - sort.sortBy === option.value && sort.sortOrder === option.order && ( - - ) - } -
- )) - } -
- - + + + {t('marketplace.sortBy', { ns: 'plugin' })} + + + {selectedOption.text} + + + + + {options.map(option => ( + { + handleSortChange({ sortBy: option.value, sortOrder: option.order }) + setOpen(false) + }} + > + {option.text} + {sort.sortBy === option.value && sort.sortOrder === option.order && ( + + )} + + ))} + + ) } diff --git a/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx b/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx index 28aec206f1..1f249b16c6 100644 --- a/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx +++ b/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx @@ -36,34 +36,85 @@ vi.mock('@/app/components/base/icons/src/vender/solid/mediaAndDevices', () => ({ })) vi.mock('@/app/components/base/ui/button', () => ({ - Button: ({ children }: { children: React.ReactNode }) => {children}, + Button: ({ children, onClick, className, ...props }: React.ButtonHTMLAttributes) => ( + + ), })) -vi.mock('@/app/components/base/portal-to-follow-elem', async () => { +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { const React = await import('react') + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + return { - PortalToFollowElem: ({ + DropdownMenu: ({ open, + onOpenChange, children, }: { open: boolean + onOpenChange?: (open: boolean) => void children: React.ReactNode }) => { portalOpen = open - return
{children}
+ return ( + +
{children}
+ + ) }, - PortalToFollowElemTrigger: ({ + DropdownMenuTrigger: ({ children, onClick, + render, }: { children: React.ReactNode - onClick: () => void - }) => , - PortalToFollowElemContent: ({ + onClick?: React.MouseEventHandler + render?: React.ReactElement + }) => { + const { isOpen, setOpen } = useDropdownMenuContext() + const handleClick = (e: React.MouseEvent) => { + onClick?.(e) + setOpen(!isOpen) + } + + if (render) + return React.cloneElement(render, { 'data-testid': 'dropdown-trigger', 'onClick': handleClick } as Record, children) + + return + }, + DropdownMenuContent: ({ children, }: { children: React.ReactNode }) => portalOpen ?
{children}
: null, + DropdownMenuItem: ({ + children, + onClick, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, } }) @@ -131,13 +182,13 @@ describe('InstallPluginDropdown', () => { expect(onSwitchToMarketplaceTab).toHaveBeenCalledTimes(1) }) - it('opens the github installer when github is selected', () => { + it('opens the github installer when github is selected', async () => { render() fireEvent.click(screen.getByTestId('dropdown-trigger')) fireEvent.click(screen.getByText('plugin.source.github')) - expect(screen.getByTestId('github-modal')).toBeInTheDocument() + expect(await screen.findByTestId('github-modal')).toBeInTheDocument() }) it('opens the local package installer when a file is selected', () => { @@ -153,4 +204,40 @@ describe('InstallPluginDropdown', () => { expect(screen.getByTestId('local-modal')).toBeInTheDocument() expect(screen.getByText('plugin.difypkg')).toBeInTheDocument() }) + + it('triggers the hidden file input when local is selected from the menu', () => { + const clickSpy = vi.spyOn(HTMLInputElement.prototype, 'click') + + render() + + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.click(screen.getByText('plugin.source.local')) + + expect(clickSpy).toHaveBeenCalledTimes(1) + clickSpy.mockRestore() + }) + + it('closes the github installer when the modal requests close', async () => { + render() + + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.click(screen.getByText('plugin.source.github')) + fireEvent.click(await screen.findByTestId('close-github-modal')) + + expect(screen.queryByTestId('github-modal')).not.toBeInTheDocument() + }) + + it('closes the local package installer when the modal requests close', () => { + const { container } = render() + + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.change(container.querySelector('input[type="file"]')!, { + target: { + files: [new File(['content'], 'plugin.difypkg')], + }, + }) + fireEvent.click(screen.getByTestId('close-local-modal')) + + expect(screen.queryByTestId('local-modal')).not.toBeInTheDocument() + }) }) diff --git a/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx b/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx index 4bfe495b93..9b98d16410 100644 --- a/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx +++ b/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx @@ -8,12 +8,13 @@ import { useTranslation } from 'react-i18next' import { FileZip } from '@/app/components/base/icons/src/vender/solid/files' import { Github } from '@/app/components/base/icons/src/vender/solid/general' import { MagicBox } from '@/app/components/base/icons/src/vender/solid/mediaAndDevices' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { Button } from '@/app/components/base/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import InstallFromGitHub from '@/app/components/plugins/install-plugin/install-from-github' import InstallFromLocalPackage from '@/app/components/plugins/install-plugin/install-from-local-package' import { SUPPORT_INSTALL_LOCAL_FILE_EXTENSIONS } from '@/config' @@ -77,61 +78,66 @@ const InstallPluginDropdown = ({ } }, [plugin_installation_permission, enable_marketplace, t]) + const handleInstallMethodSelect = (action: string) => { + if (action === 'local') { + fileInputRef.current?.click() + return + } + + if (action === 'marketplace') { + onSwitchToMarketplaceTab() + return + } + + queueMicrotask(() => { + setSelectedAction(action) + }) + } + return ( - +
- setIsMenuOpen(v => !v)}> - - - -
- - {t('installFrom', { ns: 'plugin' })} - - -
- {installMethods.map(({ icon: Icon, text, action }) => ( -
{ - if (action === 'local') { - fileInputRef.current?.click() - } - else if (action === 'marketplace') { - onSwitchToMarketplaceTab() - setIsMenuOpen(false) - } - else { - setSelectedAction(action) - setIsMenuOpen(false) - } - }} - > - - {text} -
- ))} -
-
- + + + + + {t('installFrom', { ns: 'plugin' })} + + + {installMethods.map(({ icon: Icon, text, action }) => ( + handleInstallMethodSelect(action)} + > +
+ + {text} +
+ + ))} +
{selectedAction === 'github' && ( (
handleUninstall(item.id)}>{item.name} 卸载
))} */} - + ) } diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx index 15b4429de8..c87673b750 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx @@ -685,6 +685,26 @@ describe('PluginTasks Component', () => { }) }) + it('should close the menu after clearing the last non-running plugins', async () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.success, plugin_unique_identifier: 'success-1' }), + ]) + + render() + + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + await waitFor(() => { + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByRole('button', { name: /task\.clearAll/i })) + + await waitFor(() => { + expect(document.querySelector('.w-\\[360px\\]')).not.toBeInTheDocument() + }) + }) + it('should clear only error plugins when onClearErrors is called', async () => { const { mockMutateAsync } = setupMocks([ createMockPlugin({ status: TaskStatus.failed, plugin_unique_identifier: 'error-1' }), @@ -797,6 +817,30 @@ describe('PluginTasks Component', () => { expect(document.querySelector('.w-\\[360px\\]'))!.toBeInTheDocument() }) + + it('should open for installing-with-success state', () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.running, plugin_unique_identifier: 'running-1' }), + createMockPlugin({ status: TaskStatus.success, plugin_unique_identifier: 'success-1' }), + ]) + + render() + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) + + it('should open for installing-with-error state', () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.running, plugin_unique_identifier: 'running-1' }), + createMockPlugin({ status: TaskStatus.failed, plugin_unique_identifier: 'failed-1' }), + ]) + + render() + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) }) }) diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/components/__tests__/plugin-item.spec.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/components/__tests__/plugin-item.spec.tsx index 772e246e22..ea8257af2a 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/components/__tests__/plugin-item.spec.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/components/__tests__/plugin-item.spec.tsx @@ -202,6 +202,7 @@ describe('PluginItem', () => { fireEvent.click(clearButton) expect(handleClear).toHaveBeenCalledTimes(1) + expect(clearButton).toHaveClass('invisible', 'flex', 'group-hover/item:visible') }) it('should not render clear button when onClear is not provided', () => { diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/components/error-plugin-item.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/components/error-plugin-item.tsx index ef857cb6c6..0129c65ee2 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/components/error-plugin-item.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/components/error-plugin-item.tsx @@ -108,7 +108,7 @@ const ErrorPluginItem: FC = ({ plugin, getIconUrl, languag )} statusText={( - + {plugin.message || errorMsg} )} diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-item.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-item.tsx index f627047b24..2fdb5696ea 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-item.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-item.tsx @@ -39,7 +39,7 @@ const PluginItem: FC = ({
{plugin.labels[language]}
-
+
{statusText}
{action} @@ -47,7 +47,7 @@ const PluginItem: FC = ({ {onClear && (
-
+
{plugins.map(plugin => ( = ({ {t('task.clearAll', { ns: 'plugin' })}
-
+
{errorPlugins.map(plugin => ( { handleClearErrorPlugin, } = usePluginTaskStatus() const { getIconUrl } = useGetIcon() + const canOpenMenu = isFailed || isInstalling || isInstallingWithSuccess || isInstallingWithError || isSuccess // Generate tooltip text based on status const tip = useMemo(() => { @@ -85,27 +86,20 @@ const PluginTasks = () => { [clearPluginsAndClose], ) - const handleTriggerClick = useCallback(() => { - if (isFailed || isInstalling || isInstallingWithSuccess || isInstallingWithError || isSuccess) - setOpen(v => !v) - }, [isFailed, isInstalling, isInstallingWithSuccess, isInstallingWithError, isSuccess]) - // Hide when no plugin tasks if (totalPluginsLength === 0) return null return (
- - + } + disabled={!canOpenMenu} + > { totalPluginsLength={totalPluginsLength} onClick={() => {}} /> - - + + { onClearErrors={handleClearErrors} onClearSingle={handleClearSingle} /> - - + +
) } diff --git a/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx b/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx index 7ccd788cb0..edad871855 100644 --- a/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx +++ b/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx @@ -3,6 +3,27 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-libra import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import MenuDropdown from '../menu-dropdown' +vi.mock('../info-modal', () => ({ + default: ({ + isShow, + onClose, + data, + }: { + isShow: boolean + onClose: () => void + data?: SiteInfo + }) => { + if (!isShow) + return null + return ( +
+ {data?.title} + +
+ ) + }, +})) + const mockReplace = vi.fn() const mockPathname = '/test-path' vi.mock('@/next/navigation', () => ({ @@ -191,6 +212,25 @@ describe('MenuDropdown', () => { expect(screen.getByText('Test App')).toBeInTheDocument() }) }) + + it('should close InfoModal when the close handler runs', async () => { + render() + + fireEvent.click(screen.getByRole('button')) + await waitFor(() => { + expect(screen.getByText('common.userProfile.about')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByText('common.userProfile.about')) + await waitFor(() => { + expect(screen.getByTestId('info-modal')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByText('Close Info')) + await waitFor(() => { + expect(screen.queryByTestId('info-modal')).not.toBeInTheDocument() + }) + }) }) describe('forceClose prop', () => { diff --git a/web/app/components/share/text-generation/menu-dropdown.tsx b/web/app/components/share/text-generation/menu-dropdown.tsx index bc8323676c..d50a0d77de 100644 --- a/web/app/components/share/text-generation/menu-dropdown.tsx +++ b/web/app/components/share/text-generation/menu-dropdown.tsx @@ -1,26 +1,25 @@ 'use client' -import type { Placement } from '@floating-ui/react' import type { FC } from 'react' +import type { Placement } from '@/app/components/base/ui/placement' import type { SiteInfo } from '@/models/share' import { cn } from '@langgenius/dify-ui/cn' -import { - RiEqualizer2Line, -} from '@remixicon/react' import * as React from 'react' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import ThemeSwitcher from '@/app/components/base/theme-switcher' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLinkItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useWebAppStore } from '@/context/web-app-context' import { AccessMode } from '@/models/access-control' import { usePathname, useRouter } from '@/next/navigation' import { webAppLogout } from '@/service/webapp-auth' -import Divider from '../../base/divider' import InfoModal from './info-modal' type Props = { @@ -40,24 +39,22 @@ const MenuDropdown: FC = ({ const router = useRouter() const pathname = usePathname() const { t } = useTranslation() - const [open, doSetOpen] = useState(false) - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - doSetOpen(v) - openRef.current = v - }, [doSetOpen]) - - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) + const [open, setOpen] = useState(false) const shareCode = useWebAppStore(s => s.shareCode) const handleLogout = useCallback(async () => { + setOpen(false) await webAppLogout(shareCode!) router.replace(`/webapp-signin?redirect_url=${pathname}`) - }, [router, pathname, webAppLogout, shareCode]) + }, [pathname, router, setOpen, shareCode]) const [show, setShow] = useState(false) + const handleOpenInfoModal = useCallback(() => { + setOpen(false) + queueMicrotask(() => { + setShow(true) + }) + }, []) useEffect(() => { if (forceClose) @@ -66,60 +63,56 @@ const MenuDropdown: FC = ({ return ( <> - - -
- - - -
- - -
-
-
-
{t('theme.theme', { ns: 'common' })}
- -
+ } + aria-label={t('operation.more', { ns: 'common' })} + > + + + + + +
+
+
{t('theme.theme', { ns: 'common' })}
+
- -
- {data?.privacy_policy && ( - - {t('chat.privacyPolicyMiddle', { ns: 'share' })} - - )} -
{ - handleTrigger() - setShow(true) - }} - className="cursor-pointer rounded-lg px-3 py-1.5 system-md-regular text-text-secondary hover:bg-state-base-hover" - > - {t('userProfile.about', { ns: 'common' })} -
-
- {!(hideLogout || webAppAccessMode === AccessMode.EXTERNAL_MEMBERS || webAppAccessMode === AccessMode.PUBLIC) && ( -
-
- {t('userProfile.logout', { ns: 'common' })} -
-
- )}
- - + + {data?.privacy_policy && ( + + {t('chat.privacyPolicyMiddle', { ns: 'share' })} + + )} + + {t('userProfile.about', { ns: 'common' })} + + {!(hideLogout || webAppAccessMode === AccessMode.EXTERNAL_MEMBERS || webAppAccessMode === AccessMode.PUBLIC) && ( + + {t('userProfile.logout', { ns: 'common' })} + + )} + + {show && ( { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ isOpen: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + render, + onClick, + }: { + children: React.ReactNode + render?: React.ReactElement + onClick?: React.MouseEventHandler + }) => { + const { isOpen, setOpen } = useDropdownMenuContext() + const handleClick = (e: React.MouseEvent) => { + onClick?.(e) + setOpen(!isOpen) + } + + if (render) + return React.cloneElement(render, { 'data-testid': 'dropdown-trigger', 'onClick': handleClick } as Record, children) + + return + }, + DropdownMenuContent: ({ + children, + className, + popupClassName, + }: { + children: React.ReactNode + className?: string + popupClassName?: string + }) => { + const { isOpen } = useDropdownMenuContext() + return isOpen ?
{children}
: null + }, + DropdownMenuItem: ({ + children, + onClick, + className, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + className?: string + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) + describe('OperationDropdown', () => { const defaultProps = { onEdit: vi.fn(), @@ -16,7 +92,7 @@ describe('OperationDropdown', () => { it('should render trigger button with more icon', () => { render() - const button = document.querySelector('button') + const button = screen.getByTestId('dropdown-trigger') expect(button).toBeInTheDocument() const svg = button?.querySelector('svg') expect(svg).toBeInTheDocument() @@ -39,37 +115,27 @@ describe('OperationDropdown', () => { it('should open dropdown when trigger is clicked', async () => { render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) + fireEvent.click(screen.getByTestId('dropdown-trigger')) - // Dropdown content should be rendered - expect(screen.getByText('tools.mcp.operation.edit')).toBeInTheDocument() - expect(screen.getByText('tools.mcp.operation.remove')).toBeInTheDocument() - } + expect(screen.getByText('tools.mcp.operation.edit')).toBeInTheDocument() + expect(screen.getByText('tools.mcp.operation.remove')).toBeInTheDocument() }) it('should call onOpenChange when opened', () => { const onOpenChange = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - expect(onOpenChange).toHaveBeenCalledWith(true) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + expect(onOpenChange).toHaveBeenCalledWith(true) }) it('should close dropdown when trigger is clicked again', async () => { const onOpenChange = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - fireEvent.click(trigger) - expect(onOpenChange).toHaveBeenLastCalledWith(false) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.click(screen.getByTestId('dropdown-trigger')) + expect(onOpenChange).toHaveBeenLastCalledWith(false) }) }) @@ -78,62 +144,38 @@ describe('OperationDropdown', () => { const onEdit = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - - const editOption = screen.getByText('tools.mcp.operation.edit') - fireEvent.click(editOption) - - expect(onEdit).toHaveBeenCalledTimes(1) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.click(screen.getByText('tools.mcp.operation.edit')) + expect(onEdit).toHaveBeenCalledTimes(1) }) it('should call onRemove when remove option is clicked', () => { const onRemove = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - - const removeOption = screen.getByText('tools.mcp.operation.remove') - fireEvent.click(removeOption) - - expect(onRemove).toHaveBeenCalledTimes(1) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + fireEvent.click(screen.getByText('tools.mcp.operation.remove')) + expect(onRemove).toHaveBeenCalledTimes(1) }) it('should close dropdown after edit is clicked', () => { const onOpenChange = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - onOpenChange.mockClear() - - const editOption = screen.getByText('tools.mcp.operation.edit') - fireEvent.click(editOption) - - expect(onOpenChange).toHaveBeenCalledWith(false) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + onOpenChange.mockClear() + fireEvent.click(screen.getByText('tools.mcp.operation.edit')) + expect(onOpenChange).toHaveBeenCalledWith(false) }) it('should close dropdown after remove is clicked', () => { const onOpenChange = vi.fn() render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - onOpenChange.mockClear() - - const removeOption = screen.getByText('tools.mcp.operation.remove') - fireEvent.click(removeOption) - - expect(onOpenChange).toHaveBeenCalledWith(false) - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + onOpenChange.mockClear() + fireEvent.click(screen.getByText('tools.mcp.operation.remove')) + expect(onOpenChange).toHaveBeenCalledWith(false) }) }) @@ -141,39 +183,25 @@ describe('OperationDropdown', () => { it('should have correct dropdown width', () => { render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - - const dropdown = document.querySelector('.w-\\[160px\\]') - expect(dropdown).toBeInTheDocument() - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + const dropdown = document.querySelector('.w-\\[160px\\]') + expect(dropdown).toBeInTheDocument() }) - it('should have rounded-xl on dropdown', () => { + it('should render dropdown content through the shared popup shell', () => { render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - - const dropdown = document.querySelector('[class*="rounded-xl"][class*="border"]') - expect(dropdown).toBeInTheDocument() - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + expect(screen.getByTestId('dropdown-content')).toBeInTheDocument() }) - it('should show destructive hover style on remove option', () => { + it('should apply destructive highlighted styles on remove option', () => { render() - const trigger = document.querySelector('button') - if (trigger) { - fireEvent.click(trigger) - - // The text is in a div, and the hover style is on the parent div with group class - const removeOptionText = screen.getByText('tools.mcp.operation.remove') - const removeOptionContainer = removeOptionText.closest('.group') - expect(removeOptionContainer).toHaveClass('hover:bg-state-destructive-hover') - } + fireEvent.click(screen.getByTestId('dropdown-trigger')) + const removeOptionText = screen.getByText('tools.mcp.operation.remove') + const removeOptionContainer = removeOptionText.closest('button') + expect(removeOptionContainer).toHaveClass('data-highlighted:bg-state-destructive-hover') }) }) diff --git a/web/app/components/tools/mcp/detail/operation-dropdown.tsx b/web/app/components/tools/mcp/detail/operation-dropdown.tsx index 4f5468aebc..9a7ee67051 100644 --- a/web/app/components/tools/mcp/detail/operation-dropdown.tsx +++ b/web/app/components/tools/mcp/detail/operation-dropdown.tsx @@ -7,14 +7,15 @@ import { RiMoreFill, } from '@remixicon/react' import * as React from 'react' -import { useCallback, useRef, useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { inCard?: boolean @@ -30,60 +31,37 @@ const OperationDropdown: FC = ({ onRemove, }) => { const { t } = useTranslation() - const [open, doSetOpen] = useState(false) - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - doSetOpen(v) - openRef.current = v - onOpenChange?.(v) - }, [doSetOpen]) - - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) + const [open, setOpen] = useState(false) + const handleOpenChange = useCallback((nextOpen: boolean) => { + setOpen(nextOpen) + onOpenChange?.(nextOpen) + }, [onOpenChange]) return ( - - -
- - - -
- - -
-
{ - onEdit() - handleTrigger() - }} - > - -
{t('mcp.operation.edit', { ns: 'tools' })}
-
-
{ - onRemove() - handleTrigger() - }} - > - -
{t('mcp.operation.remove', { ns: 'tools' })}
-
-
- - + + } + > + + + + + +
{t('mcp.operation.edit', { ns: 'tools' })}
+ + + +
{t('mcp.operation.remove', { ns: 'tools' })}
+ + + ) } export default React.memo(OperationDropdown) diff --git a/web/app/components/workflow/block-selector/market-place-plugin/__tests__/action.spec.tsx b/web/app/components/workflow/block-selector/market-place-plugin/__tests__/action.spec.tsx new file mode 100644 index 0000000000..1d845dd5fc --- /dev/null +++ b/web/app/components/workflow/block-selector/market-place-plugin/__tests__/action.spec.tsx @@ -0,0 +1,124 @@ +import type { ComponentProps } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { useDownloadPlugin } from '@/service/use-plugins' +import OperationDropdown from '../action' + +const mockDownloadBlob = vi.fn() +const mockRemoveQueries = vi.fn() + +vi.mock('next-themes', () => ({ + useTheme: () => ({ + theme: 'light', + }), +})) + +vi.mock('@/service/use-plugins', () => ({ + useDownloadPlugin: vi.fn(), +})) + +vi.mock('@/utils/download', () => ({ + downloadBlob: (...args: unknown[]) => mockDownloadBlob(...args), +})) + +vi.mock('@/utils/var', () => ({ + getMarketplaceUrl: (path: string) => `https://marketplace.example${path}`, +})) + +const createQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}) + +const renderComponent = (props?: Partial>) => { + const queryClient = createQueryClient() + vi.spyOn(queryClient, 'removeQueries').mockImplementation(((...args) => { + return mockRemoveQueries(...args) + }) as typeof queryClient.removeQueries) + + return render( + + + , + ) +} + +describe('OperationDropdown', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(useDownloadPlugin).mockImplementation((_, enabled) => ({ + data: enabled ? null : null, + isLoading: false, + }) as unknown as ReturnType) + }) + + it('should render download and view details actions when opened', async () => { + renderComponent({ open: true }) + + expect(screen.getByText('common.operation.download')).toBeInTheDocument() + expect(screen.getByText('common.operation.viewDetails')).toBeInTheDocument() + }) + + it('should request a download when download is clicked', async () => { + const onOpenChange = vi.fn() + renderComponent({ open: true, onOpenChange }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + expect(onOpenChange).toHaveBeenCalledWith(false) + expect(mockRemoveQueries).toHaveBeenCalled() + }) + + it('should skip download when already loading', async () => { + vi.mocked(useDownloadPlugin).mockReturnValue({ + data: null, + isLoading: true, + } as unknown as ReturnType) + + renderComponent({ open: true }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + expect(mockRemoveQueries).not.toHaveBeenCalled() + }) + + it('should download the blob when the hook returns data', async () => { + vi.mocked(useDownloadPlugin).mockImplementation((_, enabled) => ({ + data: enabled ? new Blob(['plugin zip'], { type: 'application/zip' }) : null, + isLoading: false, + }) as unknown as ReturnType) + + renderComponent({ open: true }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + await waitFor(() => { + expect(mockDownloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), + fileName: 'langgenius-test-plugin_1.0.0.zip', + }) + }) + expect(mockRemoveQueries).toHaveBeenCalled() + }) + + it('should link to the marketplace detail page', () => { + renderComponent({ open: true }) + + expect(screen.getByRole('menuitem', { name: 'common.operation.viewDetails' })).toHaveAttribute( + 'href', + 'https://marketplace.example/plugins/langgenius/test-plugin', + ) + }) +}) diff --git a/web/app/components/workflow/block-selector/market-place-plugin/action.tsx b/web/app/components/workflow/block-selector/market-place-plugin/action.tsx index 4ae623ffc1..a058e8c051 100644 --- a/web/app/components/workflow/block-selector/market-place-plugin/action.tsx +++ b/web/app/components/workflow/block-selector/market-place-plugin/action.tsx @@ -1,19 +1,19 @@ 'use client' import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import { useQueryClient } from '@tanstack/react-query' import { useTheme } from 'next-themes' import * as React from 'react' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' -// import { Button } from '@/app/components/base/ui/button' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLinkItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useDownloadPlugin } from '@/service/use-plugins' import { downloadBlob } from '@/utils/download' import { getMarketplaceUrl } from '@/utils/var' @@ -36,16 +36,10 @@ const OperationDropdown: FC = ({ const { t } = useTranslation() const { theme } = useTheme() const queryClient = useQueryClient() - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - onOpenChange(v) - openRef.current = v + const setOpen = useCallback((value: boolean) => { + onOpenChange(value) }, [onOpenChange]) - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) - const [needDownload, setNeedDownload] = useState(false) const downloadInfo = useMemo(() => ({ organization: author, @@ -56,12 +50,13 @@ const OperationDropdown: FC = ({ const handleDownload = useCallback(() => { if (isLoading) return + setOpen(false) queryClient.removeQueries({ queryKey: ['plugins', 'downloadPlugin', downloadInfo], exact: true, }) setNeedDownload(true) - }, [downloadInfo, isLoading, queryClient]) + }, [downloadInfo, isLoading, queryClient, setOpen]) useEffect(() => { if (!needDownload || !blob) @@ -75,27 +70,33 @@ const OperationDropdown: FC = ({ }) }, [author, blob, downloadInfo, name, needDownload, queryClient, version]) return ( - - + }> - + - - -
-
{t('operation.download', { ns: 'common' })}
- {t('operation.viewDetails', { ns: 'common' })} -
- - + + + + {t('operation.download', { ns: 'common' })} + + + {t('operation.viewDetails', { ns: 'common' })} + + + ) } export default React.memo(OperationDropdown) diff --git a/web/app/components/workflow/header/__tests__/test-run-menu-helpers.spec.tsx b/web/app/components/workflow/header/__tests__/test-run-menu-helpers.spec.tsx index cce4c070a1..7df9cd091f 100644 --- a/web/app/components/workflow/header/__tests__/test-run-menu-helpers.spec.tsx +++ b/web/app/components/workflow/header/__tests__/test-run-menu-helpers.spec.tsx @@ -1,7 +1,7 @@ +import type * as React from 'react' import type { TriggerOption } from '../test-run-menu' import { fireEvent, render, renderHook, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import * as React from 'react' import { TriggerType } from '../test-run-menu' import { getNormalizedShortcutKey, @@ -10,6 +10,33 @@ import { useShortcutMenu, } from '../test-run-menu-helpers' +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuItem: ({ children, onClick, className }: { children: React.ReactNode, onClick?: React.MouseEventHandler, className?: string }) => ( + + ), + } +}) + vi.mock('../shortcuts-name', () => ({ default: ({ keys }: { keys: string[] }) => {keys.join('+')}, })) diff --git a/web/app/components/workflow/header/__tests__/test-run-menu.spec.tsx b/web/app/components/workflow/header/__tests__/test-run-menu.spec.tsx index 2e3384b61e..1d462bfc9c 100644 --- a/web/app/components/workflow/header/__tests__/test-run-menu.spec.tsx +++ b/web/app/components/workflow/header/__tests__/test-run-menu.spec.tsx @@ -5,25 +5,62 @@ import { act } from 'react' import * as React from 'react' import TestRunMenu, { TriggerType } from '../test-run-menu' -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ - children, - }: { - children: React.ReactNode - }) =>
{children}
, - PortalToFollowElemTrigger: ({ - children, - onClick, - }: { - children: React.ReactNode - onClick?: () => void - }) =>
{children}
, - PortalToFollowElemContent: ({ - children, - }: { - children: React.ReactNode - }) =>
{children}
, -})) +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + render, + }: { + children: React.ReactNode + render?: React.ReactElement + }) => { + const { open, setOpen } = useDropdownMenuContext() + + if (render) + return React.cloneElement(render, { onClick: () => setOpen(!open) } as Record, children) + + return + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuGroup: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuLabel: ({ children, className }: { children: React.ReactNode, className?: string }) =>
{children}
, + DropdownMenuGroupLabel: ({ children, className }: { children: React.ReactNode, className?: string }) =>
{children}
, + DropdownMenuSeparator: ({ className }: { className?: string }) =>
, + DropdownMenuItem: ({ children, onClick, className }: { children: React.ReactNode, onClick?: React.MouseEventHandler, className?: string }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) vi.mock('../shortcuts-name', () => ({ default: ({ keys }: { keys: string[] }) => {keys.join('+')}, @@ -95,10 +132,11 @@ describe('TestRunMenu', () => { act(() => { fireEvent.click(screen.getByRole('button', { name: 'Toggle via ref' })) }) + expect(screen.getByText('~')).toBeInTheDocument() + fireEvent.keyDown(window, { key: '0' }) expect(onSelect).toHaveBeenCalledWith(expect.objectContaining({ id: 'run-all' })) - expect(screen.getByText('~')).toBeInTheDocument() }) it('should ignore disabled options in the rendered menu', async () => { diff --git a/web/app/components/workflow/header/test-run-menu-helpers.tsx b/web/app/components/workflow/header/test-run-menu-helpers.tsx index 1c34761df6..4a25cd87a6 100644 --- a/web/app/components/workflow/header/test-run-menu-helpers.tsx +++ b/web/app/components/workflow/header/test-run-menu-helpers.tsx @@ -6,6 +6,7 @@ import { isValidElement, useEffect, } from 'react' +import { DropdownMenuItem } from '@/app/components/base/ui/dropdown-menu' import ShortcutsName from '../shortcuts-name' export type ShortcutMapping = { @@ -27,9 +28,8 @@ export const OptionRow = ({ onSelect: (option: TriggerOption) => void }) => { return ( -
onSelect(option)} >
@@ -41,7 +41,7 @@ export const OptionRow = ({ {shortcutKey && ( )} -
+ ) } diff --git a/web/app/components/workflow/header/test-run-menu.tsx b/web/app/components/workflow/header/test-run-menu.tsx index 1d496e4332..6540875e6b 100644 --- a/web/app/components/workflow/header/test-run-menu.tsx +++ b/web/app/components/workflow/header/test-run-menu.tsx @@ -1,7 +1,7 @@ import type { ShortcutMapping } from './test-run-menu-helpers' import { forwardRef, useCallback, useImperativeHandle, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { DropdownMenu, DropdownMenuContent, DropdownMenuGroup, DropdownMenuLabel, DropdownMenuSeparator, DropdownMenuTrigger } from '@/app/components/base/ui/dropdown-menu' import { OptionRow, SingleOptionTrigger, useShortcutMenu } from './test-run-menu-helpers' export enum TriggerType { @@ -127,7 +127,7 @@ const TestRunMenu = forwardRef(({ }), [hasSingleEnabledOption, runSoleOption]) const renderOption = (option: TriggerOption) => { - return + return } const { hasUserInput, hasTriggers, hasRunAll } = useMemo(() => getMenuVisibility(options), [options]) @@ -141,27 +141,28 @@ const TestRunMenu = forwardRef(({ } return ( - - setOpen(!open)}> -
- {children} -
- - -
-
+ }> + {children} + + + + {t('common.chooseStartNodeToRun', { ns: 'workflow' })} -
+
{hasUserInput && renderOption(options.userInput!)} {(hasTriggers || hasRunAll) && hasUserInput && ( -
+ )} {hasRunAll && renderOption(options.runAll!)} @@ -170,9 +171,9 @@ const TestRunMenu = forwardRef(({ .filter(trigger => trigger.enabled !== false) .map(trigger => renderOption(trigger))}
-
- - + + + ) }) diff --git a/web/app/components/workflow/nodes/_base/components/next-step/__tests__/operator.spec.tsx b/web/app/components/workflow/nodes/_base/components/next-step/__tests__/operator.spec.tsx new file mode 100644 index 0000000000..9afa29642d --- /dev/null +++ b/web/app/components/workflow/nodes/_base/components/next-step/__tests__/operator.spec.tsx @@ -0,0 +1,152 @@ +import type { + ReactNode, +} from 'react' +import type { CommonNodeType } from '@/app/components/workflow/types' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { useState } from 'react' +import { + useAvailableBlocks, + useNodesInteractions, +} from '@/app/components/workflow/hooks' +import { BlockEnum } from '@/app/components/workflow/types' +import Operator from '../operator' + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, options?: { ns?: string }) => options?.ns ? `${options.ns}.${key}` : key, + }), +})) + +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ children, render }: { children: ReactNode, render?: ReactNode }) => { + const { open, setOpen } = useDropdownMenuContext() + if (render) + return
setOpen(!open)}>{children}
+ + return + }, + DropdownMenuContent: ({ children }: { children: ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + } +}) + +vi.mock('@/app/components/base/ui/button', () => ({ + Button: ({ children, className }: { children: ReactNode, className?: string }) => ( + + ), +})) + +vi.mock('@/app/components/workflow/block-selector', () => ({ + default: ({ trigger, onSelect }: { trigger: ((open: boolean) => ReactNode) | ReactNode, onSelect: (type: BlockEnum) => void }) => ( +
+ {typeof trigger === 'function' ? trigger(false) : trigger} + +
+ ), +})) + +vi.mock('@/app/components/workflow/hooks', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useAvailableBlocks: vi.fn(), + useNodesInteractions: vi.fn(), + } +}) + +const mockUseAvailableBlocks = vi.mocked(useAvailableBlocks) +const mockUseNodesInteractions = vi.mocked(useNodesInteractions) + +const mockHandleNodeChange = vi.fn() +const mockHandleNodeDelete = vi.fn() +const mockHandleNodeDisconnect = vi.fn() + +const defaultNodeData = { + type: BlockEnum.Code, + title: 'Code Node', +} as CommonNodeType + +const TestHarness = () => { + const [open, setOpen] = useState(false) + return ( + + ) +} + +describe('NextStep operator', () => { + beforeEach(() => { + vi.clearAllMocks() + mockUseAvailableBlocks.mockReturnValue({ + availablePrevBlocks: [BlockEnum.HttpRequest], + availableNextBlocks: [BlockEnum.HttpRequest], + getAvailableBlocks: vi.fn(), + } as ReturnType) + mockUseNodesInteractions.mockReturnValue({ + handleNodeChange: mockHandleNodeChange, + handleNodeDelete: mockHandleNodeDelete, + handleNodeDisconnect: mockHandleNodeDisconnect, + } as unknown as ReturnType) + }) + + it('opens the menu and keeps the change action available', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getAllByRole('button')[0]!) + + expect(screen.getByText('workflow.panel.change')).toBeInTheDocument() + expect(screen.getByText('workflow.common.disconnect')).toBeInTheDocument() + expect(screen.getByText('common.operation.delete')).toBeInTheDocument() + }) + + it('changes the next-step block through the nested selector trigger', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getAllByRole('button')[0]!) + await user.click(screen.getByText('select-http')) + + expect(mockHandleNodeChange).toHaveBeenCalledWith('node-1', BlockEnum.HttpRequest, 'source', undefined) + }) + + it('disconnects and deletes the next step from the menu', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getAllByRole('button')[0]!) + await user.click(screen.getByText('workflow.common.disconnect')) + expect(mockHandleNodeDisconnect).toHaveBeenCalledWith('node-1') + expect(screen.queryByText('workflow.common.disconnect')).not.toBeInTheDocument() + + await user.click(screen.getAllByRole('button')[0]!) + await user.click(screen.getByText('common.operation.delete')) + expect(mockHandleNodeDelete).toHaveBeenCalledWith('node-1') + }) +}) diff --git a/web/app/components/workflow/nodes/_base/components/next-step/operator.tsx b/web/app/components/workflow/nodes/_base/components/next-step/operator.tsx index bc979eed60..c0a4f0b537 100644 --- a/web/app/components/workflow/nodes/_base/components/next-step/operator.tsx +++ b/web/app/components/workflow/nodes/_base/components/next-step/operator.tsx @@ -2,18 +2,17 @@ import type { CommonNodeType, OnSelectBlock, } from '@/app/components/workflow/types' -import { RiMoreFill } from '@remixicon/react' import { intersection } from 'es-toolkit/array' import { useCallback, } from 'react' import { useTranslation } from 'react-i18next' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { Button } from '@/app/components/base/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import BlockSelector from '@/app/components/workflow/block-selector' import { useAvailableBlocks, @@ -86,18 +85,21 @@ const Operator = ({ } = useNodesInteractions() return ( - - onOpenChange(!open)}> + }> - - + +
handleNodeDisconnect(nodeId)} + onClick={() => { + onOpenChange(false) + handleNodeDisconnect(nodeId) + }} > {t('common.disconnect', { ns: 'workflow' })}
@@ -115,14 +120,17 @@ const Operator = ({
handleNodeDelete(nodeId)} + onClick={() => { + onOpenChange(false) + handleNodeDelete(nodeId) + }} > {t('operation.delete', { ns: 'common' })}
- - + + ) } diff --git a/web/app/components/workflow/nodes/_base/components/panel-operator/__tests__/index.spec.tsx b/web/app/components/workflow/nodes/_base/components/panel-operator/__tests__/index.spec.tsx index 183e28c5f0..6dab0f33a5 100644 --- a/web/app/components/workflow/nodes/_base/components/panel-operator/__tests__/index.spec.tsx +++ b/web/app/components/workflow/nodes/_base/components/panel-operator/__tests__/index.spec.tsx @@ -70,7 +70,11 @@ const createQueryResult = (data: T): UseQueryResult => ({ promise: Promise.resolve(data), } as UseQueryResult) -const renderComponent = (showHelpLink: boolean = true, onOpenChange?: (open: boolean) => void) => +const renderComponent = ( + showHelpLink: boolean = true, + onOpenChange?: (open: boolean) => void, + offset?: { mainAxis: number, crossAxis: number } | number, +) => renderWorkflowFlowComponent( , @@ -158,5 +163,15 @@ describe('PanelOperator', () => { expect(screen.queryByText('workflow.panel.helpLink')).not.toBeInTheDocument() expect(screen.getByText('Node description')).toBeInTheDocument() }) + + it('should still open the popup when using a numeric offset and no open-change callback', async () => { + const user = userEvent.setup() + const { container } = renderComponent(true, undefined, 0) + + await user.click(container.querySelector('.panel-operator-trigger') as HTMLElement) + + expect(screen.getByText('workflow.panel.runThisStep')).toBeInTheDocument() + expect(screen.getByText('Node description')).toBeInTheDocument() + }) }) }) diff --git a/web/app/components/workflow/nodes/_base/components/panel-operator/index.tsx b/web/app/components/workflow/nodes/_base/components/panel-operator/index.tsx index 173a084dcf..2109365d75 100644 --- a/web/app/components/workflow/nodes/_base/components/panel-operator/index.tsx +++ b/web/app/components/workflow/nodes/_base/components/panel-operator/index.tsx @@ -1,23 +1,22 @@ import type { OffsetOptions } from '@floating-ui/react' import type { Node } from '@/app/components/workflow/types' -import { RiMoreFill } from '@remixicon/react' import { memo, useCallback, useState, } from 'react' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import PanelOperatorPopup from './panel-operator-popup' type PanelOperatorProps = { id: string data: Node['data'] triggerClassName?: string - offset?: OffsetOptions + offset?: OffsetOptions | number onOpenChange?: (open: boolean) => void inNode?: boolean showHelpLink?: boolean @@ -34,6 +33,14 @@ const PanelOperator = ({ showHelpLink = true, }: PanelOperatorProps) => { const [open, setOpen] = useState(false) + const sideOffset = typeof offset === 'number' + ? offset + : typeof offset === 'object' && offset && 'mainAxis' in offset && typeof offset.mainAxis === 'number' + ? offset.mainAxis + : 4 + const alignOffset = typeof offset === 'object' && offset && 'crossAxis' in offset && typeof offset.crossAxis === 'number' + ? offset.crossAxis + : 0 const handleOpenChange = useCallback((newOpen: boolean) => { setOpen(newOpen) @@ -43,13 +50,11 @@ const PanelOperator = ({ }, [onOpenChange]) return ( - - handleOpenChange(!open)}> + }>
- +
- - + + setOpen(false)} + onClosePopup={() => handleOpenChange(false)} showHelpLink={showHelpLink} /> - - + + ) } diff --git a/web/app/components/workflow/nodes/assigner/components/__tests__/operation-selector.spec.tsx b/web/app/components/workflow/nodes/assigner/components/__tests__/operation-selector.spec.tsx index 63813c8a46..e29b1e42a3 100644 --- a/web/app/components/workflow/nodes/assigner/components/__tests__/operation-selector.spec.tsx +++ b/web/app/components/workflow/nodes/assigner/components/__tests__/operation-selector.spec.tsx @@ -4,6 +4,75 @@ import { VarType } from '@/app/components/workflow/types' import { WriteMode } from '../../types' import OperationSelector from '../operation-selector' +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + className, + disabled, + }: { + children: React.ReactNode + className?: string + disabled?: boolean + }) => { + const { open, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuGroup: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuLabel: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuGroupLabel: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuSeparator: () =>
, + DropdownMenuItem: ({ + children, + onClick, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) + describe('assigner/operation-selector', () => { it('shows numeric write modes and emits the selected operation', async () => { const user = userEvent.setup() diff --git a/web/app/components/workflow/nodes/assigner/components/operation-selector.tsx b/web/app/components/workflow/nodes/assigner/components/operation-selector.tsx index 8bce904b74..a22b8f5f3f 100644 --- a/web/app/components/workflow/nodes/assigner/components/operation-selector.tsx +++ b/web/app/components/workflow/nodes/assigner/components/operation-selector.tsx @@ -3,18 +3,17 @@ import type { WriteMode } from '../types' import type { Item } from '../utils' import type { VarType } from '@/app/components/workflow/types' import { cn } from '@langgenius/dify-ui/cn' -import { - RiArrowDownSLine, - RiCheckLine, -} from '@remixicon/react' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import Divider from '@/app/components/base/divider' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuGroup, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { getOperationItems, isOperationItem } from '../utils' type OperationSelectorProps = { @@ -49,65 +48,57 @@ const OperationSelector: FC = ({ const selectedItem = items.find(item => item.value === value) return ( - - !disabled && setOpen(v => !v)} + -
-
- + - {selectedItem && isOperationItem(selectedItem) ? t(`nodes.assigner.operations.${selectedItem.name}`, { ns: 'workflow' }) : t('nodes.assigner.operations.title', { ns: 'workflow' })} - -
- + > + {selectedItem && isOperationItem(selectedItem) ? t(`nodes.assigner.operations.${selectedItem.name}`, { ns: 'workflow' }) : t('nodes.assigner.operations.title', { ns: 'workflow' })} +
- + + - -
-
-
-
{t('nodes.assigner.operations.title', { ns: 'workflow' })}
-
- {items.map(item => ( - !isOperationItem(item) - ? ( - - ) - : ( -
{ - onSelect(item) - setOpen(false) - }} - > -
- {t(`nodes.assigner.operations.${item.name}`, { ns: 'workflow' })} -
- {item.value === value && ( -
- -
- )} + + + {t('nodes.assigner.operations.title', { ns: 'workflow' })} + {items.map(item => ( + !isOperationItem(item) + ? ( + + ) + : ( + onSelect(item)} + > +
+ {t(`nodes.assigner.operations.${item.name}`, { ns: 'workflow' })}
- ) - ))} -
-
- - + {item.value === value && ( +
+ +
+ )} + + ) + ))} + + + ) } diff --git a/web/app/components/workflow/note-node/__tests__/index.spec.tsx b/web/app/components/workflow/note-node/__tests__/index.spec.tsx index 9814bb63f4..1bfa14ffb1 100644 --- a/web/app/components/workflow/note-node/__tests__/index.spec.tsx +++ b/web/app/components/workflow/note-node/__tests__/index.spec.tsx @@ -110,6 +110,8 @@ describe('NoteNode', () => { await waitFor(() => { expect(screen.getByText('workflow.nodes.note.editor.small')).toBeInTheDocument() }) + + expect(screen.getByText('workflow.nodes.note.editor.small').closest('.nodrag.nopan.nowheel')).toBeInTheDocument() }) it('should hide the toolbar for temporary notes', () => { diff --git a/web/app/components/workflow/note-node/index.tsx b/web/app/components/workflow/note-node/index.tsx index 5050041a05..fa69f05841 100644 --- a/web/app/components/workflow/note-node/index.tsx +++ b/web/app/components/workflow/note-node/index.tsx @@ -95,7 +95,7 @@ const NoteNode = ({
{ data.selected && !data._isTempNode && ( -
+
{ expect(onThemeChange).toHaveBeenCalledWith(NoteTheme.violet) - fireEvent.click(container.querySelectorAll('[data-state="closed"]')[container.querySelectorAll('[data-state="closed"]').length - 1] as HTMLElement) + fireEvent.click(screen.getByRole('button', { name: 'common.operation.more' })) fireEvent.click(screen.getByText('workflow.common.copy')) expect(onCopy).toHaveBeenCalledTimes(1) diff --git a/web/app/components/workflow/note-node/note-editor/toolbar/__tests__/operator.spec.tsx b/web/app/components/workflow/note-node/note-editor/toolbar/__tests__/operator.spec.tsx index 1870bf913a..c60898b29e 100644 --- a/web/app/components/workflow/note-node/note-editor/toolbar/__tests__/operator.spec.tsx +++ b/web/app/components/workflow/note-node/note-editor/toolbar/__tests__/operator.spec.tsx @@ -1,13 +1,132 @@ -import { fireEvent, render, screen } from '@testing-library/react' +import type { + MouseEvent, + MouseEventHandler, + ReactElement, + ReactNode, +} from 'react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import Operator from '../operator' +type DropdownTriggerRenderProps = { + 'className'?: string + 'role'?: string + 'aria-label'?: string + 'onMouseDown'?: MouseEventHandler + 'onClick'?: MouseEventHandler +} + +type DropdownTriggerProps = { + 'children': ReactNode + 'className'?: string + 'render'?: ReactElement + 'onMouseDown'?: MouseEventHandler + 'onClick'?: MouseEventHandler + 'aria-label'?: string +} + +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ + children, + className, + render, + onMouseDown, + onClick, + 'aria-label': ariaLabel, + }: DropdownTriggerProps) => { + const { open, setOpen } = useDropdownMenuContext() + if (render) { + const handleMouseDown = (event: MouseEvent) => { + const baseUiEvent = event as MouseEvent & { preventBaseUIHandler?: () => void } + baseUiEvent.preventBaseUIHandler = vi.fn() + onMouseDown?.(baseUiEvent) + render.props.onMouseDown?.(event) + } + + const handleClick = (event: MouseEvent) => { + onClick?.(event) + render.props.onClick?.(event) + if (!onMouseDown) + setOpen(!open) + } + + return ( +
+ {children} +
+ ) + } + + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuItem: ({ + children, + onClick, + className, + }: { + children: ReactNode + onClick?: MouseEventHandler + className?: string + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuSeparator: ({ className }: { className?: string }) =>
, + } +}) + const renderOperator = (showAuthor = false) => { const onCopy = vi.fn() const onDuplicate = vi.fn() const onDelete = vi.fn() const onShowAuthorChange = vi.fn() - const renderResult = render( + render( { ) return { - ...renderResult, onCopy, onDelete, onDuplicate, @@ -27,41 +145,35 @@ const renderOperator = (showAuthor = false) => { } describe('NoteEditor Toolbar Operator', () => { - it('should trigger copy, duplicate, and delete from the opened menu', () => { + it('triggers copy, duplicate, and delete from the opened menu', async () => { + const user = userEvent.setup() const { - container, onCopy, onDelete, onDuplicate, } = renderOperator() - const trigger = container.querySelector('[data-state="closed"]') as HTMLElement - - fireEvent.click(trigger) - fireEvent.click(screen.getByText('workflow.common.copy')) - + await user.click(screen.getByRole('button', { name: 'common.operation.more' })) + await user.click(screen.getByText('workflow.common.copy')) expect(onCopy).toHaveBeenCalledTimes(1) - fireEvent.click(container.querySelector('[data-state="closed"]') as HTMLElement) - fireEvent.click(screen.getByText('workflow.common.duplicate')) - + await user.click(screen.getByRole('button', { name: 'common.operation.more' })) + await user.click(screen.getByText('workflow.common.duplicate')) expect(onDuplicate).toHaveBeenCalledTimes(1) - fireEvent.click(container.querySelector('[data-state="closed"]') as HTMLElement) - fireEvent.click(screen.getByText('common.operation.delete')) - + await user.click(screen.getByRole('button', { name: 'common.operation.more' })) + await user.click(screen.getByText('common.operation.delete')) expect(onDelete).toHaveBeenCalledTimes(1) }) - it('should forward the switch state through onShowAuthorChange', () => { - const { - container, - onShowAuthorChange, - } = renderOperator(true) + it('keeps the menu open when toggling show author', async () => { + const user = userEvent.setup() + const { onShowAuthorChange } = renderOperator(true) - fireEvent.click(container.querySelector('[data-state="closed"]') as HTMLElement) - fireEvent.click(screen.getByRole('switch')) + await user.click(screen.getByRole('button', { name: 'common.operation.more' })) + await user.click(screen.getByRole('switch')) expect(onShowAuthorChange).toHaveBeenCalledWith(false) + expect(screen.getByText('workflow.nodes.note.editor.showAuthor')).toBeInTheDocument() }) }) diff --git a/web/app/components/workflow/note-node/note-editor/toolbar/index.tsx b/web/app/components/workflow/note-node/note-editor/toolbar/index.tsx index c7b8fa9787..f00d3464ac 100644 --- a/web/app/components/workflow/note-node/note-editor/toolbar/index.tsx +++ b/web/app/components/workflow/note-node/note-editor/toolbar/index.tsx @@ -18,7 +18,11 @@ const Toolbar = ({ onShowAuthorChange, }: ToolbarProps) => { return ( -
+
event.stopPropagation()} + onClick={event => event.stopPropagation()} + > - setOpen(!open)}> -
- -
- - + } + aria-label={t('operation.more', { ns: 'common' })} + className={cn( + 'flex h-8 w-8 cursor-pointer items-center justify-center rounded-lg text-text-tertiary hover:bg-state-base-hover hover:text-text-secondary', + open && 'bg-state-base-hover text-text-secondary', + )} + onMouseDown={(event) => { + event.preventDefault() + event.stopPropagation() + ;(event as typeof event & { preventBaseUIHandler?: () => void }).preventBaseUIHandler?.() + setOpen(prev => !prev) + }} + onClick={event => event.stopPropagation()} + > + + +
-
{ - onCopy() setOpen(false) + onCopy() }} > {t('common.copy', { ns: 'workflow' })} -
-
+ { - onDuplicate() setOpen(false) + onDuplicate() }} > {t('common.duplicate', { ns: 'workflow' })} -
+
-
+
-
+
-
{ - onDelete() setOpen(false) + onDelete() }} > {t('operation.delete', { ns: 'common' })} -
+
- - + + ) } diff --git a/web/app/components/workflow/operator/__tests__/more-actions.spec.tsx b/web/app/components/workflow/operator/__tests__/more-actions.spec.tsx new file mode 100644 index 0000000000..b984ca18d6 --- /dev/null +++ b/web/app/components/workflow/operator/__tests__/more-actions.spec.tsx @@ -0,0 +1,309 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { act } from 'react' +import MoreActions from '../more-actions' + +const mockToPng = vi.fn() +const mockToJpeg = vi.fn() +const mockToSvg = vi.fn() +const mockDownloadUrl = vi.fn() +const mockSetViewport = vi.fn() +const mockGetNodesReadOnly = vi.fn() +const { + mockAppStoreState, + mockWorkflowState, +} = vi.hoisted(() => ({ + mockAppStoreState: { + appSidebarExpand: 'collapse', + }, + mockWorkflowState: { + knowledgeName: '', + appName: 'Demo App', + maximizeCanvas: false, + }, +})) + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, options?: { ns?: string }) => options?.ns ? `${options.ns}.${key}` : key, + }), +})) + +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ children, className }: { children: React.ReactNode, className?: string }) => { + const { open, setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuItem: ({ + children, + onClick, + className, + }: { + children: React.ReactNode + onClick?: React.MouseEventHandler + className?: string + }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + DropdownMenuSeparator: ({ className }: { className?: string }) =>
, + } +}) + +vi.mock('html-to-image', () => ({ + toPng: (...args: unknown[]) => mockToPng(...args), + toJpeg: (...args: unknown[]) => mockToJpeg(...args), + toSvg: (...args: unknown[]) => mockToSvg(...args), +})) + +vi.mock('reactflow', () => ({ + getNodesBounds: () => ({ x: 0, y: 0, width: 240, height: 120 }), + useReactFlow: () => ({ + getNodes: () => [{ id: 'node-1' }], + getViewport: () => ({ x: 0, y: 0, zoom: 1 }), + setViewport: mockSetViewport, + }), +})) + +vi.mock('@/app/components/app/store', () => ({ + useStore: (selector: (state: typeof mockAppStoreState) => unknown) => selector(mockAppStoreState), +})) + +vi.mock('@/app/components/workflow/store', () => ({ + useStore: (selector: (state: typeof mockWorkflowState) => unknown) => selector(mockWorkflowState), +})) + +vi.mock('@/app/components/workflow/hooks', () => ({ + useNodesReadOnly: () => ({ + getNodesReadOnly: mockGetNodesReadOnly, + }), +})) + +vi.mock('@/utils/download', () => ({ + downloadUrl: (...args: unknown[]) => mockDownloadUrl(...args), +})) + +vi.mock('../tip-popup', () => ({ + default: ({ children }: { children: React.ReactNode }) => <>{children}, +})) + +vi.mock('@/app/components/base/image-uploader/image-preview', () => ({ + default: ({ title, onCancel }: { title: string, onCancel: () => void }) => ( +
+ {title} + +
+ ), +})) + +describe('MoreActions', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.useRealTimers() + mockGetNodesReadOnly.mockReturnValue(false) + mockToPng.mockResolvedValue('data:image/png;base64,current') + mockToJpeg.mockResolvedValue('data:image/jpeg;base64,current') + mockToSvg.mockResolvedValue('data:image/svg+xml;base64,current') + mockAppStoreState.appSidebarExpand = 'collapse' + mockWorkflowState.knowledgeName = '' + mockWorkflowState.appName = 'Demo App' + mockWorkflowState.maximizeCanvas = false + + document.body.innerHTML = '' + const viewport = document.createElement('div') + viewport.className = 'react-flow__viewport' + document.body.appendChild(viewport) + }) + + it('opens the menu and exports the current view as png', async () => { + const user = userEvent.setup() + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getAllByText('workflow.common.exportPNG')[0]!) + + await waitFor(() => { + expect(mockToPng).toHaveBeenCalledTimes(1) + }) + expect(mockDownloadUrl).toHaveBeenCalledWith({ + url: 'data:image/png;base64,current', + fileName: 'Demo App.png', + }) + }) + + it('does not open the menu when the workflow is read only', async () => { + const user = userEvent.setup() + mockGetNodesReadOnly.mockReturnValue(true) + + render() + + await user.click(screen.getByRole('button')) + + expect(screen.queryByText('workflow.common.exportImage')).not.toBeInTheDocument() + }) + + it('shows a preview when exporting the whole workflow', async () => { + vi.useFakeTimers() + + render() + + fireEvent.click(screen.getByRole('button')) + fireEvent.click(screen.getAllByText('workflow.common.exportPNG')[1]!) + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + + expect(screen.getByTestId('image-preview')).toHaveTextContent('Demo App-whole-workflow.png') + await act(async () => { + await vi.runAllTimersAsync() + }) + expect(mockSetViewport).toHaveBeenCalledTimes(2) + expect(mockDownloadUrl).toHaveBeenCalledWith({ + url: 'data:image/png;base64,current', + fileName: 'Demo App-whole-workflow.png', + }) + }) + + it.each([ + ['workflow.common.exportJPEG', mockToJpeg, 'Demo App.jpeg'], + ['workflow.common.exportSVG', mockToSvg, 'Demo App.svg'], + ])('exports the current view with %s', async (label, exporter, fileName) => { + const user = userEvent.setup() + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getAllByText(label)[0]!) + + await waitFor(() => { + expect(exporter).toHaveBeenCalledTimes(1) + }) + expect(mockDownloadUrl).toHaveBeenCalledWith({ + url: expect.any(String), + fileName, + }) + }) + + it('exports the whole workflow as svg when the canvas is maximized', async () => { + vi.useFakeTimers() + mockWorkflowState.maximizeCanvas = true + + render() + + fireEvent.click(screen.getByRole('button')) + fireEvent.click(screen.getAllByText('workflow.common.exportSVG')[1]!) + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + + expect(mockToSvg).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.runAllTimersAsync() + }) + expect(mockSetViewport).toHaveBeenCalledTimes(2) + expect(screen.getByTestId('image-preview')).toHaveTextContent('Demo App-whole-workflow.svg') + }) + + it('returns early when there is no app or knowledge name', async () => { + const user = userEvent.setup() + mockWorkflowState.appName = '' + mockWorkflowState.knowledgeName = '' + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getAllByText('workflow.common.exportPNG')[0]!) + + expect(mockToPng).not.toHaveBeenCalled() + expect(mockDownloadUrl).not.toHaveBeenCalled() + }) + + it('returns early when the viewport element is missing', async () => { + const user = userEvent.setup() + document.querySelector('.react-flow__viewport')?.remove() + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getAllByText('workflow.common.exportPNG')[0]!) + + expect(mockToPng).not.toHaveBeenCalled() + expect(mockDownloadUrl).not.toHaveBeenCalled() + }) + + it('returns early when the workflow becomes read only before exporting', async () => { + const user = userEvent.setup() + + render() + + await user.click(screen.getByRole('button')) + mockGetNodesReadOnly.mockReturnValue(true) + await user.click(screen.getAllByText('workflow.common.exportJPEG')[0]!) + + expect(mockToJpeg).not.toHaveBeenCalled() + expect(mockDownloadUrl).not.toHaveBeenCalled() + }) + + it('logs export failures and lets the preview close', async () => { + const user = userEvent.setup() + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + mockToJpeg.mockRejectedValueOnce(new Error('boom')) + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getAllByText('workflow.common.exportJPEG')[0]!) + + await waitFor(() => { + expect(consoleErrorSpy).toHaveBeenCalledWith('Export image failed:', expect.any(Error)) + }) + expect(screen.queryByTestId('image-preview')).not.toBeInTheDocument() + + mockToPng.mockResolvedValueOnce('data:image/png;base64,current') + fireEvent.click(screen.getByRole('button')) + fireEvent.click(screen.getAllByText('workflow.common.exportPNG')[1]!) + await waitFor(() => { + expect(screen.getByTestId('image-preview')).toBeInTheDocument() + }) + await user.click(screen.getByText('close-preview')) + expect(screen.queryByTestId('image-preview')).not.toBeInTheDocument() + + consoleErrorSpy.mockRestore() + }) +}) diff --git a/web/app/components/workflow/operator/__tests__/zoom-in-out.spec.tsx b/web/app/components/workflow/operator/__tests__/zoom-in-out.spec.tsx new file mode 100644 index 0000000000..8b0f12c7d6 --- /dev/null +++ b/web/app/components/workflow/operator/__tests__/zoom-in-out.spec.tsx @@ -0,0 +1,197 @@ +import { fireEvent, render, screen, within } from '@testing-library/react' +import ZoomInOut from '../zoom-in-out' + +const { + mockZoomIn, + mockZoomOut, + mockZoomTo, + mockFitView, + mockViewport, + mockHandleSyncWorkflowDraft, + mockToggleMiniMap, + mockToggleUserComments, + mockToggleUserCursors, +} = vi.hoisted(() => ({ + mockZoomIn: vi.fn(), + mockZoomOut: vi.fn(), + mockZoomTo: vi.fn(), + mockFitView: vi.fn(), + mockViewport: { zoom: 1 }, + mockHandleSyncWorkflowDraft: vi.fn(), + mockToggleMiniMap: vi.fn(), + mockToggleUserComments: vi.fn(), + mockToggleUserCursors: vi.fn(), +})) + +let workflowReadOnly = false +let collaborationEnabled = true + +vi.mock('reactflow', () => ({ + useReactFlow: () => ({ + zoomIn: mockZoomIn, + zoomOut: mockZoomOut, + zoomTo: mockZoomTo, + fitView: mockFitView, + }), + useViewport: () => mockViewport, +})) + +vi.mock('@/app/components/workflow/hooks', () => ({ + useNodesSyncDraft: () => ({ + handleSyncWorkflowDraft: mockHandleSyncWorkflowDraft, + }), + useWorkflowReadOnly: () => ({ + workflowReadOnly, + getWorkflowReadOnly: () => workflowReadOnly, + }), +})) + +vi.mock('@/context/global-public-context', () => ({ + useGlobalPublicStore: (selector: (state: { systemFeatures: { enable_collaboration_mode: boolean } }) => unknown) => selector({ + systemFeatures: { + enable_collaboration_mode: collaborationEnabled, + }, + }), +})) + +vi.mock('../tip-popup', () => ({ + default: ({ children }: { children: React.ReactNode }) => <>{children}, +})) + +const getZoomControls = () => { + const label = Array.from(document.querySelectorAll('button')).find((element) => { + return /^\d+%$/.test(element.textContent ?? '') && element.className.includes('w-[34px]') + }) + const zoomOutIcon = document.querySelector('.i-ri-zoom-out-line') + const zoomInIcon = document.querySelector('.i-ri-zoom-in-line') + + if (!label || !zoomOutIcon || !zoomInIcon) + throw new Error('Missing zoom controls') + + return { + zoomOutTrigger: zoomOutIcon.parentElement as HTMLElement, + label, + zoomInTrigger: zoomInIcon.parentElement as HTMLElement, + } +} + +const openZoomMenu = () => { + fireEvent.click(getZoomControls().label) + return within(screen.getByRole('menu')) +} + +describe('workflow zoom controls', () => { + beforeEach(() => { + vi.clearAllMocks() + mockViewport.zoom = 1 + workflowReadOnly = false + collaborationEnabled = true + }) + + it('zooms out and zooms in when the viewport is within the supported range', () => { + render() + + const { zoomOutTrigger, zoomInTrigger } = getZoomControls() + + fireEvent.click(zoomOutTrigger) + fireEvent.click(zoomInTrigger) + + expect(mockZoomOut).toHaveBeenCalledTimes(1) + expect(mockZoomIn).toHaveBeenCalledTimes(1) + }) + + it('zooms to a preset value and syncs the draft', () => { + render() + + const menu = openZoomMenu() + fireEvent.click(menu.getByText('50%')) + + expect(mockZoomTo).toHaveBeenCalledWith(0.5) + expect(mockHandleSyncWorkflowDraft).toHaveBeenCalledTimes(1) + }) + + it.each([ + ['100%', 1], + ['200%', 2], + ])('zooms to %s and syncs the draft', (label, zoom) => { + render() + + const menu = openZoomMenu() + fireEvent.click(menu.getByText(label)) + + expect(mockZoomTo).toHaveBeenCalledWith(zoom) + expect(mockHandleSyncWorkflowDraft).toHaveBeenCalledTimes(1) + }) + + it('toggles collaboration options without syncing the draft', () => { + render( + , + ) + + let menu = openZoomMenu() + fireEvent.click(menu.getByText('workflow.operator.showMiniMap')) + expect(mockToggleMiniMap).toHaveBeenCalledTimes(1) + expect(mockHandleSyncWorkflowDraft).not.toHaveBeenCalled() + + menu = openZoomMenu() + fireEvent.click(menu.getByText('workflow.operator.showUserComments')) + expect(mockToggleUserComments).toHaveBeenCalledTimes(1) + + menu = openZoomMenu() + fireEvent.click(menu.getByText('workflow.operator.showUserCursors')) + expect(mockToggleUserCursors).toHaveBeenCalledTimes(1) + }) + + it('keeps the show-user-comments action disabled in comment mode', () => { + render( + , + ) + + const menu = openZoomMenu() + fireEvent.click(menu.getByText('workflow.operator.showUserComments')) + + expect(mockToggleUserComments).not.toHaveBeenCalled() + }) + + it('does not open the menu when the workflow is read only', () => { + workflowReadOnly = true + render() + + fireEvent.click(getZoomControls().label) + + expect(screen.queryByRole('menu')).not.toBeInTheDocument() + }) + + it('blocks inline zooming out at the minimum viewport scale', () => { + mockViewport.zoom = 0.25 + render() + + fireEvent.click(getZoomControls().zoomOutTrigger) + expect(mockZoomOut).not.toHaveBeenCalled() + }) + + it('blocks inline zooming in at the maximum viewport scale', () => { + mockViewport.zoom = 2 + render() + + fireEvent.click(getZoomControls().zoomInTrigger) + expect(mockZoomIn).not.toHaveBeenCalled() + }) + + it('renders collaboration menu entries only when collaboration is enabled', () => { + collaborationEnabled = false + render() + + const menu = openZoomMenu() + expect(menu.getByText('workflow.operator.showMiniMap')).toBeInTheDocument() + expect(menu.queryByText('workflow.operator.showUserComments')).not.toBeInTheDocument() + expect(menu.queryByText('workflow.operator.showUserCursors')).not.toBeInTheDocument() + }) +}) diff --git a/web/app/components/workflow/operator/more-actions.tsx b/web/app/components/workflow/operator/more-actions.tsx index 5e71cc658b..66dbed1a91 100644 --- a/web/app/components/workflow/operator/more-actions.tsx +++ b/web/app/components/workflow/operator/more-actions.tsx @@ -14,10 +14,12 @@ import { useShallow } from 'zustand/react/shallow' import { useStore as useAppStore } from '@/app/components/app/store' import ImagePreview from '@/app/components/base/image-uploader/image-preview' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useStore } from '@/app/components/workflow/store' import { downloadUrl } from '@/utils/download' import { useNodesReadOnly } from '../hooks' @@ -37,6 +39,7 @@ const MoreActions: FC = () => { const { appSidebarExpand } = useAppStore(useShallow(state => ({ appSidebarExpand: state.appSidebarExpand, }))) + const isReadOnly = getNodesReadOnly() const crossAxisOffset = useMemo(() => { if (maximizeCanvas) @@ -161,93 +164,67 @@ const MoreActions: FC = () => { } }, [getNodesReadOnly, appName, reactFlow, knowledgeName]) - const handleTrigger = useCallback(() => { - if (getNodesReadOnly()) - return - - setOpen(v => !v) - }, [getNodesReadOnly]) - return ( <> - { + if (isReadOnly) { + setOpen(false) + return + } + setOpen(nextOpen) }} > - + -
- -
+ - - -
-
-
- - {t('common.exportImage', { ns: 'workflow' })} -
-
- {t('common.currentView', { ns: 'workflow' })} -
-
handleExportImage('png')} - > - {t('common.exportPNG', { ns: 'workflow' })} -
-
handleExportImage('jpeg')} - > - {t('common.exportJPEG', { ns: 'workflow' })} -
-
handleExportImage('svg')} - > - {t('common.exportSVG', { ns: 'workflow' })} -
- -
- -
- {t('common.currentWorkflow', { ns: 'workflow' })} -
-
handleExportImage('png', true)} - > - {t('common.exportPNG', { ns: 'workflow' })} -
-
handleExportImage('jpeg', true)} - > - {t('common.exportJPEG', { ns: 'workflow' })} -
-
handleExportImage('svg', true)} - > - {t('common.exportSVG', { ns: 'workflow' })} -
-
+ + +
+ + {t('common.exportImage', { ns: 'workflow' })}
- - +
+ {t('common.currentView', { ns: 'workflow' })} +
+ handleExportImage('png')}> + {t('common.exportPNG', { ns: 'workflow' })} + + handleExportImage('jpeg')}> + {t('common.exportJPEG', { ns: 'workflow' })} + + handleExportImage('svg')}> + {t('common.exportSVG', { ns: 'workflow' })} + + + + +
+ {t('common.currentWorkflow', { ns: 'workflow' })} +
+ handleExportImage('png', true)}> + {t('common.exportPNG', { ns: 'workflow' })} + + handleExportImage('jpeg', true)}> + {t('common.exportJPEG', { ns: 'workflow' })} + + handleExportImage('svg', true)}> + {t('common.exportSVG', { ns: 'workflow' })} + + + {previewUrl && ( = ({ } = useWorkflowReadOnly() const isCollaborationEnabled = useGlobalPublicStore(s => s.systemFeatures.enable_collaboration_mode) - const ZOOM_IN_OUT_OPTIONS = [ + const zoomOptions = [ [ { key: ZoomType.zoomTo200, @@ -135,6 +126,8 @@ const ZoomInOut: FC = ({ if (workflowReadOnly) return + setOpen(false) + if (type === ZoomType.zoomToFit) fitView() @@ -173,154 +166,134 @@ const ZoomInOut: FC = ({ handleSyncWorkflowDraft() } - const handleTrigger = useCallback(() => { - if (getWorkflowReadOnly()) - return - - setOpen(v => !v) - }, [getWorkflowReadOnly]) - return ( - - -
+ -
{ + if (zoom <= 0.25) + return + + e.stopPropagation() + zoomOut() + }} > - -
{ - if (zoom <= 0.25) - return - - e.stopPropagation() - zoomOut() - }} - > - -
- -
- {Number.parseFloat(`${zoom * 100}`).toFixed(0)} - % -
- -
= 2 ? 'cursor-not-allowed' : 'cursor-pointer hover:bg-black/5'}`} - onClick={(e) => { - if (zoom >= 2) - return - - e.stopPropagation() - zoomIn() - }} - > - -
- +
-
- - -
- { - ZOOM_IN_OUT_OPTIONS.map((options, i) => ( - - { - i !== 0 && ( - - ) - } -
- { - options.map(option => ( -
+ + + {Number.parseFloat(`${zoom * 100}`).toFixed(0)} + % + + +
+ {zoomOptions.map((options, groupIndex) => ( + + {groupIndex !== 0 && ( + + )} +
+ {options.map(option => ( + handleZoom(option.key)} > -
+
{option.key === ZoomType.toggleUserComments && showUserComments && ( - + )} {option.key === ZoomType.toggleUserComments && !showUserComments && ( -
+ )} {option.key === ZoomType.toggleUserCursors && showUserCursors && ( - + )} {option.key === ZoomType.toggleUserCursors && !showUserCursors && ( -
+ )} {option.key === ZoomType.toggleMiniMap && showMiniMap && ( - + )} {option.key === ZoomType.toggleMiniMap && !showMiniMap && ( -
+ )} {option.key === ZoomType.zoomToFit && ( - + )} {option.key !== ZoomType.toggleUserComments && option.key !== ZoomType.toggleUserCursors && option.key !== ZoomType.toggleMiniMap && option.key !== ZoomType.zoomToFit && ( -
+ )} {option.text}
- { - option.key === ZoomType.zoomToFit && ( - - ) - } - { - option.key === ZoomType.zoomTo50 && ( - - ) - } - { - option.key === ZoomType.zoomTo100 && ( - - ) - } + {option.key === ZoomType.zoomToFit && ( + + )} + {option.key === ZoomType.zoomTo50 && ( + + )} + {option.key === ZoomType.zoomTo100 && ( + + )}
-
- )) - } -
- - )) - } -
- - + + ))} +
+ + ))} +
+ + + +
= 2 ? 'cursor-not-allowed' : 'cursor-pointer hover:bg-black/5'}`} + onClick={(e) => { + if (zoom >= 2) + return + + e.stopPropagation() + zoomIn() + }} + > + +
+ +
+
) } diff --git a/web/app/components/workflow/panel/version-history-panel/context-menu/__tests__/menu-item.spec.tsx b/web/app/components/workflow/panel/version-history-panel/context-menu/__tests__/menu-item.spec.tsx index 02b1f8ee34..844e189067 100644 --- a/web/app/components/workflow/panel/version-history-panel/context-menu/__tests__/menu-item.spec.tsx +++ b/web/app/components/workflow/panel/version-history-panel/context-menu/__tests__/menu-item.spec.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { DropdownMenu, DropdownMenuContent } from '@/app/components/base/ui/dropdown-menu' import { VersionHistoryContextMenuOptions } from '../../../../types' import MenuItem from '../menu-item' @@ -9,14 +10,18 @@ describe('MenuItem', () => { const onClick = vi.fn() render( - , + + + + + , ) await user.click(screen.getByText('Delete')) diff --git a/web/app/components/workflow/panel/version-history-panel/context-menu/index.tsx b/web/app/components/workflow/panel/version-history-panel/context-menu/index.tsx index a635e80bab..f063902753 100644 --- a/web/app/components/workflow/panel/version-history-panel/context-menu/index.tsx +++ b/web/app/components/workflow/panel/version-history-panel/context-menu/index.tsx @@ -1,14 +1,13 @@ import type { FC } from 'react' import { RiMoreFill } from '@remixicon/react' import * as React from 'react' -import { useCallback } from 'react' -import Divider from '@/app/components/base/divider' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { Button } from '@/app/components/base/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { VersionHistoryContextMenuOptions } from '../../../types' import MenuItem from './menu-item' import useContextMenu from './use-context-menu' @@ -28,58 +27,44 @@ const ContextMenu: FC = (props: ContextMenuProps) => { options, } = useContextMenu(props) - const handleClickTrigger = useCallback((e: React.MouseEvent) => { - e.stopPropagation() - setOpen(v => !v) - }, [setOpen]) - return ( - - - - - -
-
- { - options.map((option) => { - return ( - - ) - }) - } -
- { - isShowDelete && ( - <> - -
- -
- - ) - } -
- - + e.stopPropagation()} />} + > + + + + { + options.map(option => ( + + )) + } + { + isShowDelete && ( + <> + + + + ) + } + + ) } diff --git a/web/app/components/workflow/panel/version-history-panel/context-menu/menu-item.tsx b/web/app/components/workflow/panel/version-history-panel/context-menu/menu-item.tsx index 5a3f21272f..b1d9ef6ec5 100644 --- a/web/app/components/workflow/panel/version-history-panel/context-menu/menu-item.tsx +++ b/web/app/components/workflow/panel/version-history-panel/context-menu/menu-item.tsx @@ -2,6 +2,7 @@ import type { FC } from 'react' import type { VersionHistoryContextMenuOptions } from '../../../types' import { cn } from '@langgenius/dify-ui/cn' import * as React from 'react' +import { DropdownMenuItem } from '@/app/components/base/ui/dropdown-menu' type MenuItemProps = { item: { @@ -18,23 +19,25 @@ const MenuItem: FC = ({ isDestructive = false, }) => { return ( -
{ + onClick={(event) => { + event.stopPropagation() onClick(item.key) }} >
{item.name}
-
+ ) } diff --git a/web/app/components/workflow/run/agent-log/__tests__/agent-log-nav-more.spec.tsx b/web/app/components/workflow/run/agent-log/__tests__/agent-log-nav-more.spec.tsx index d109635af4..35b32a5ce6 100644 --- a/web/app/components/workflow/run/agent-log/__tests__/agent-log-nav-more.spec.tsx +++ b/web/app/components/workflow/run/agent-log/__tests__/agent-log-nav-more.spec.tsx @@ -3,6 +3,52 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import AgentLogNavMore from '../agent-log-nav-more' +vi.mock('@/app/components/base/ui/dropdown-menu', async () => { + const React = await import('react') + const DropdownMenuContext = React.createContext<{ open: boolean, setOpen: (open: boolean) => void } | null>(null) + + const useDropdownMenuContext = () => { + const context = React.use(DropdownMenuContext) + if (!context) + throw new Error('DropdownMenu components must be wrapped in DropdownMenu') + return context + } + + return { + DropdownMenu: ({ children, open, onOpenChange }: { children: React.ReactNode, open: boolean, onOpenChange?: (open: boolean) => void }) => ( + +
{children}
+ + ), + DropdownMenuTrigger: ({ children, render }: { children: React.ReactNode, render?: React.ReactElement }) => { + const { open, setOpen } = useDropdownMenuContext() + + if (render) + return React.cloneElement(render, { onClick: () => setOpen(!open) } as Record, children) + + return + }, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => { + const { open } = useDropdownMenuContext() + return open ?
{children}
: null + }, + DropdownMenuItem: ({ children, onClick }: { children: React.ReactNode, onClick?: React.MouseEventHandler }) => { + const { setOpen } = useDropdownMenuContext() + return ( + + ) + }, + } +}) + const createLogItem = (overrides: Partial = {}): AgentLogItemWithChildren => ({ message_id: 'message-1', label: 'Planner', diff --git a/web/app/components/workflow/run/agent-log/agent-log-nav-more.tsx b/web/app/components/workflow/run/agent-log/agent-log-nav-more.tsx index 8bdb6ad227..77f3c778a6 100644 --- a/web/app/components/workflow/run/agent-log/agent-log-nav-more.tsx +++ b/web/app/components/workflow/run/agent-log/agent-log-nav-more.tsx @@ -1,12 +1,13 @@ import type { AgentLogItemWithChildren } from '@/types/workflow' import { RiMoreLine } from '@remixicon/react' import { useState } from 'react' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import { Button } from '@/app/components/base/ui/button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type AgentLogNavMoreProps = { options: AgentLogItemWithChildren[] @@ -19,42 +20,39 @@ const AgentLogNavMore = ({ const [open, setOpen] = useState(false) return ( - - setOpen(v => !v)}> - - - -
- { - options.map(option => ( -
{ - onShowAgentOrToolLog(option) - setOpen(false) - }} - > - {option.label} -
- )) - } -
- - + + )} + > + + + + { + options.map(option => ( + onShowAgentOrToolLog(option)} + > + {option.label} + + )) + } + + ) } diff --git a/web/app/components/workflow/workflow-preview/components/__tests__/zoom-in-out.spec.tsx b/web/app/components/workflow/workflow-preview/components/__tests__/zoom-in-out.spec.tsx index 4991ae7b04..6e8ad90933 100644 --- a/web/app/components/workflow/workflow-preview/components/__tests__/zoom-in-out.spec.tsx +++ b/web/app/components/workflow/workflow-preview/components/__tests__/zoom-in-out.spec.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, within } from '@testing-library/react' +import { fireEvent, render, screen, within } from '@testing-library/react' import ZoomInOut from '../zoom-in-out' const { @@ -26,29 +26,25 @@ vi.mock('reactflow', () => ({ })) const getZoomControls = () => { - const label = Array.from(document.querySelectorAll('div')).find((element) => { + const label = Array.from(document.querySelectorAll('button')).find((element) => { return /^\d+%$/.test(element.textContent ?? '') && element.className.includes('w-[34px]') }) - const icons = Array.from(document.querySelectorAll('svg')) + const zoomOutIcon = document.querySelector('.i-ri-zoom-out-line') + const zoomInIcon = document.querySelector('.i-ri-zoom-in-line') - if (!label || icons.length < 2) + if (!label || !zoomOutIcon || !zoomInIcon) throw new Error('Missing zoom controls') return { - zoomOutTrigger: icons[0]!.parentElement as HTMLElement, + zoomOutTrigger: zoomOutIcon.parentElement as HTMLElement, label, - zoomInTrigger: icons[1]!.parentElement as HTMLElement, + zoomInTrigger: zoomInIcon.parentElement as HTMLElement, } } const openZoomMenu = () => { fireEvent.click(getZoomControls().label) - - const portal = document.querySelector('[data-floating-ui-portal]') - if (!portal) - throw new Error('Missing zoom menu portal') - - return within(portal as HTMLElement) + return within(screen.getByRole('menu')) } describe('workflow preview zoom controls', () => { diff --git a/web/app/components/workflow/workflow-preview/components/zoom-in-out.tsx b/web/app/components/workflow/workflow-preview/components/zoom-in-out.tsx index 7a81bccc50..3f714c5ca9 100644 --- a/web/app/components/workflow/workflow-preview/components/zoom-in-out.tsx +++ b/web/app/components/workflow/workflow-preview/components/zoom-in-out.tsx @@ -1,13 +1,8 @@ import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { - RiZoomInLine, - RiZoomOutLine, -} from '@remixicon/react' import { Fragment, memo, - useCallback, useState, } from 'react' import { useTranslation } from 'react-i18next' @@ -15,18 +10,17 @@ import { useReactFlow, useViewport, } from 'reactflow' -import Divider from '@/app/components/base/divider' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import TipPopup from '@/app/components/workflow/operator/tip-popup' import ShortcutsName from '@/app/components/workflow/shortcuts-name' enum ZoomType { - zoomIn = 'zoomIn', - zoomOut = 'zoomOut', zoomToFit = 'zoomToFit', zoomTo25 = 'zoomTo25', zoomTo50 = 'zoomTo50', @@ -46,7 +40,7 @@ const ZoomInOut: FC = () => { const { zoom } = useViewport() const [open, setOpen] = useState(false) - const ZOOM_IN_OUT_OPTIONS = [ + const zoomOptions = [ [ { key: ZoomType.zoomTo200, @@ -78,6 +72,8 @@ const ZoomInOut: FC = () => { ] const handleZoom = (type: string) => { + setOpen(false) + if (type === ZoomType.zoomToFit) fitView() @@ -97,119 +93,98 @@ const ZoomInOut: FC = () => { zoomTo(2) } - const handleTrigger = useCallback(() => { - setOpen(v => !v) - }, []) - return ( - - -
+ -
{ + if (zoom <= 0.25) + return + + e.stopPropagation() + zoomOut() + }} > - -
{ - if (zoom <= 0.25) - return - - e.stopPropagation() - zoomOut() - }} - > - -
- -
- {Number.parseFloat(`${zoom * 100}`).toFixed(0)} - % -
- -
= 2 ? 'cursor-not-allowed' : 'cursor-pointer hover:bg-black/5'}`} - onClick={(e) => { - if (zoom >= 2) - return - - e.stopPropagation() - zoomIn() - }} - > - -
- +
-
- - -
- { - ZOOM_IN_OUT_OPTIONS.map((options, i) => ( - - { - i !== 0 && ( - - ) - } -
- { - options.map(option => ( -
+ + + {Number.parseFloat(`${zoom * 100}`).toFixed(0)} + % + + +
+ {zoomOptions.map((options, groupIndex) => ( + + {groupIndex !== 0 && ( + + )} +
+ {options.map(option => ( + handleZoom(option.key)} > {option.text}
- { - option.key === ZoomType.zoomToFit && ( - - ) - } - { - option.key === ZoomType.zoomTo50 && ( - - ) - } - { - option.key === ZoomType.zoomTo100 && ( - - ) - } + {option.key === ZoomType.zoomToFit && ( + + )} + {option.key === ZoomType.zoomTo50 && ( + + )} + {option.key === ZoomType.zoomTo100 && ( + + )}
-
- )) - } -
- - )) - } -
- - + + ))} +
+ + ))} +
+ + + +
= 2 ? 'cursor-not-allowed' : 'cursor-pointer hover:bg-black/5'}`} + onClick={(e) => { + if (zoom >= 2) + return + + e.stopPropagation() + zoomIn() + }} + > + +
+ +
+
) } diff --git a/web/docs/overlay-migration.md b/web/docs/overlay-migration.md index 6762c2bfb1..29ef814fcd 100644 --- a/web/docs/overlay-migration.md +++ b/web/docs/overlay-migration.md @@ -9,9 +9,7 @@ This document tracks the migration away from legacy overlay APIs. - `@/app/components/base/tooltip` - `@/app/components/base/modal` - `@/app/components/base/select` (including `custom` / `pure`) - - `@/app/components/base/dropdown` - `@/app/components/base/dialog` - - `@/app/components/base/toast` (including `context`) - Replacement primitives: - `@/app/components/base/ui/tooltip` - `@/app/components/base/ui/dropdown-menu` @@ -42,12 +40,6 @@ This document tracks the migration away from legacy overlay APIs. - Remove remaining allowlist entries. - Remove legacy overlay implementations when import count reaches zero. -## Toast migration strategy - -- `@/app/components/base/toast` has been replaced by `@/app/components/base/ui/toast`. -- All new toast usage must go through `@/app/components/base/ui/toast`. -- When a file with legacy toast usage is touched, prefer migrating that call site in the same change; full-repo toast cleanup is not required in one PR. - ## Allowlist maintenance - After each migration batch, run: diff --git a/web/eslint.constants.mjs b/web/eslint.constants.mjs index fb912d6524..57a14fc22d 100644 --- a/web/eslint.constants.mjs +++ b/web/eslint.constants.mjs @@ -52,13 +52,6 @@ export const OVERLAY_RESTRICTED_IMPORT_PATTERNS = [ ], message: 'Deprecated: use @/app/components/base/ui/select instead. See issue #32767.', }, - { - group: [ - '**/base/dropdown', - '**/base/dropdown/index', - ], - message: 'Deprecated: use @/app/components/base/ui/dropdown-menu instead. See issue #32767.', - }, { group: [ '**/base/dialog', @@ -80,7 +73,6 @@ export const OVERLAY_MIGRATION_LEGACY_BASE_FILES = [ 'app/components/base/chip/index.tsx', 'app/components/base/date-and-time-picker/date-picker/index.tsx', 'app/components/base/date-and-time-picker/time-picker/index.tsx', - 'app/components/base/dropdown/index.tsx', 'app/components/base/features/new-feature-panel/file-upload/setting-modal.tsx', 'app/components/base/features/new-feature-panel/text-to-speech/voice-settings.tsx', 'app/components/base/file-uploader/file-from-link-or-local/index.tsx',