From c407f40e0d61a44d3b9bc204997c2de073862ee7 Mon Sep 17 00:00:00 2001
From: yyh <92089059+lyzno1@users.noreply.github.com>
Date: Mon, 18 May 2026 14:45:12 +0800
Subject: [PATCH] refactor(web): simplify github install focus (#36314)
---
eslint-suppressions.json | 2 +-
.../__tests__/index.spec.tsx | 195 ++++--------------
.../install-from-github/index.tsx | 49 ++++-
.../steps/__tests__/setURL.spec.tsx | 189 -----------------
.../install-from-github/steps/setURL.tsx | 69 -------
.../install-plugin-dropdown.spec.tsx | 5 +-
.../plugin-page/install-plugin-dropdown.tsx | 2 +-
7 files changed, 81 insertions(+), 430 deletions(-)
delete mode 100644 web/app/components/plugins/install-plugin/install-from-github/steps/__tests__/setURL.spec.tsx
delete mode 100644 web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx
diff --git a/eslint-suppressions.json b/eslint-suppressions.json
index c1835f888f..07504d2754 100644
--- a/eslint-suppressions.json
+++ b/eslint-suppressions.json
@@ -2417,7 +2417,7 @@
},
"web/app/components/plugins/install-plugin/install-from-github/index.tsx": {
"ts/no-explicit-any": {
- "count": 3
+ "count": 2
}
},
"web/app/components/plugins/install-plugin/install-from-github/steps/selectPackage.tsx": {
diff --git a/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx b/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx
index e923de5e38..105e1212b0 100644
--- a/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx
+++ b/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx
@@ -101,25 +101,6 @@ vi.mock('../../hooks/use-hide-logic', () => ({
}))
// Mock child components
-vi.mock('../steps/setURL', () => ({
- default: ({ repoUrl, onChange, onNext, onCancel }: {
- repoUrl: string
- onChange: (value: string) => void
- onNext: () => void
- onCancel: () => void
- }) => (
-
- onChange(e.target.value)}
- />
-
-
-
- ),
-}))
-
vi.mock('../steps/selectPackage', () => ({
default: ({
repoUrl,
@@ -236,6 +217,10 @@ vi.mock('../../base/installed', () => ({
),
}))
+const getRepoUrlInput = () => screen.getByLabelText('plugin.installFromGitHub.gitHubRepo')
+const getNextButton = () => screen.getByRole('button', { name: 'plugin.installModal.next' })
+const getCancelButton = () => screen.getByRole('button', { name: 'plugin.installModal.cancel' })
+
describe('InstallFromGitHub', () => {
const defaultProps = {
onClose: vi.fn(),
@@ -261,8 +246,8 @@ describe('InstallFromGitHub', () => {
it('should render modal with correct initial state for new installation', () => {
render()
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
- expect(screen.getByTestId('repo-url-input')).toHaveValue('')
+ expect(getRepoUrlInput()).toBeInTheDocument()
+ expect(getRepoUrlInput()).toHaveValue('')
})
it('should render modal with selectPackage step when updatePayload is provided', () => {
@@ -312,7 +297,7 @@ describe('InstallFromGitHub', () => {
it('should update repoUrl when user types in input', () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/test/repo' } })
expect(input).toHaveValue('https://github.com/test/repo')
@@ -321,10 +306,10 @@ describe('InstallFromGitHub', () => {
it('should transition from setUrl to selectPackage on successful URL submit', async () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- const nextBtn = screen.getByTestId('next-btn')
+ const nextBtn = getNextButton()
fireEvent.click(nextBtn)
await waitFor(() => {
@@ -443,10 +428,10 @@ describe('InstallFromGitHub', () => {
it('should show error toast for invalid GitHub URL', async () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'invalid-url' } })
- const nextBtn = screen.getByTestId('next-btn')
+ const nextBtn = getNextButton()
fireEvent.click(nextBtn)
await waitFor(() => {
@@ -462,10 +447,10 @@ describe('InstallFromGitHub', () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- const nextBtn = screen.getByTestId('next-btn')
+ const nextBtn = getNextButton()
fireEvent.click(nextBtn)
await waitFor(() => {
@@ -481,10 +466,10 @@ describe('InstallFromGitHub', () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- const nextBtn = screen.getByTestId('next-btn')
+ const nextBtn = getNextButton()
fireEvent.click(nextBtn)
await waitFor(() => {
@@ -504,9 +489,9 @@ describe('InstallFromGitHub', () => {
render()
// Navigate to selectPackage
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('select-package-step')).toBeInTheDocument()
@@ -516,7 +501,7 @@ describe('InstallFromGitHub', () => {
fireEvent.click(screen.getByTestId('back-btn'))
await waitFor(() => {
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
+ expect(getRepoUrlInput()).toBeInTheDocument()
})
})
@@ -546,7 +531,7 @@ describe('InstallFromGitHub', () => {
it('should call onClose when cancel button is clicked', () => {
render()
- fireEvent.click(screen.getByTestId('cancel-btn'))
+ fireEvent.click(getCancelButton())
expect(defaultProps.onClose).toHaveBeenCalledTimes(1)
})
@@ -783,10 +768,10 @@ describe('InstallFromGitHub', () => {
it('should handle URL without trailing slash', async () => {
render()
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(mockFetchReleases).toHaveBeenCalledWith('owner', 'repo')
@@ -797,11 +782,11 @@ describe('InstallFromGitHub', () => {
render()
// Set URL
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/test/myrepo' } })
// Navigate to selectPackage
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('select-package-step')).toBeInTheDocument()
@@ -980,12 +965,12 @@ describe('InstallFromGitHub', () => {
render()
// Start from setUrl step
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
+ expect(getRepoUrlInput()).toBeInTheDocument()
// Enter URL
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('select-package-step')).toBeInTheDocument()
@@ -1086,7 +1071,7 @@ describe('InstallFromGitHub', () => {
render()
// Verify we're on setUrl step
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
+ expect(getRepoUrlInput()).toBeInTheDocument()
// The setUrl step doesn't expose onBack in the real component,
// but our mock doesn't have it either - this is correct behavior
@@ -1097,9 +1082,9 @@ describe('InstallFromGitHub', () => {
render()
// Navigate to selectPackage
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('select-package-step')).toBeInTheDocument()
@@ -1123,11 +1108,11 @@ describe('InstallFromGitHub', () => {
fireEvent.click(screen.getByTestId('back-btn'))
await waitFor(() => {
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
+ expect(getRepoUrlInput()).toBeInTheDocument()
})
// Verify URL is preserved after back navigation
- expect(screen.getByTestId('repo-url-input')).toHaveValue('https://github.com/owner/repo')
+ expect(getRepoUrlInput()).toHaveValue('https://github.com/owner/repo')
})
})
})
@@ -1368,114 +1353,6 @@ describe('Install Plugin Utils', () => {
// Steps Components Tests
// ================================
-// SetURL Component Tests
-describe('SetURL Component', () => {
- // Import the real component for testing
- const SetURL = vi.fn()
-
- beforeEach(() => {
- vi.clearAllMocks()
- // Re-mock the SetURL component with a more testable version
- vi.doMock('./steps/setURL', () => ({
- default: SetURL,
- }))
- })
-
- describe('Rendering', () => {
- it('should render label with correct text', () => {
- render()
-
- // The mocked component should be rendered
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
- })
-
- it('should render input field with placeholder', () => {
- render()
-
- const input = screen.getByTestId('repo-url-input')
- expect(input).toBeInTheDocument()
- })
-
- it('should render cancel and next buttons', () => {
- render()
-
- expect(screen.getByTestId('cancel-btn')).toBeInTheDocument()
- expect(screen.getByTestId('next-btn')).toBeInTheDocument()
- })
- })
-
- describe('Props', () => {
- it('should display repoUrl value in input', () => {
- render()
-
- const input = screen.getByTestId('repo-url-input')
- fireEvent.change(input, { target: { value: 'https://github.com/test/repo' } })
-
- expect(input).toHaveValue('https://github.com/test/repo')
- })
-
- it('should call onChange when input value changes', () => {
- render()
-
- const input = screen.getByTestId('repo-url-input')
- fireEvent.change(input, { target: { value: 'new-value' } })
-
- expect(input).toHaveValue('new-value')
- })
- })
-
- describe('User Interactions', () => {
- it('should call onNext when next button is clicked', async () => {
- mockFetchReleases.mockResolvedValue(createMockReleases())
-
- render()
-
- const input = screen.getByTestId('repo-url-input')
- fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
-
- fireEvent.click(screen.getByTestId('next-btn'))
-
- await waitFor(() => {
- expect(mockFetchReleases).toHaveBeenCalled()
- })
- })
-
- it('should call onCancel when cancel button is clicked', () => {
- const onClose = vi.fn()
- render()
-
- fireEvent.click(screen.getByTestId('cancel-btn'))
-
- expect(onClose).toHaveBeenCalledTimes(1)
- })
- })
-
- describe('Edge Cases', () => {
- it('should handle empty URL input', () => {
- render()
-
- const input = screen.getByTestId('repo-url-input')
- expect(input).toHaveValue('')
- })
-
- it('should handle URL with whitespace only', () => {
- render()
-
- const input = screen.getByTestId('repo-url-input')
- fireEvent.change(input, { target: { value: ' ' } })
-
- // With whitespace only, next should still be submittable but validation will fail
- fireEvent.click(screen.getByTestId('next-btn'))
-
- // Should show error for invalid URL
- expect(mockNotify).toHaveBeenCalledWith({
- type: 'error',
- message: 'plugin.error.inValidGitHubUrl',
- })
- })
- })
-})
-
// SelectPackage Component Tests
describe('SelectPackage Component', () => {
beforeEach(() => {
@@ -1513,9 +1390,9 @@ describe('SelectPackage Component', () => {
render()
// Navigate to selectPackage step
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('back-btn')).toBeInTheDocument()
@@ -1590,9 +1467,9 @@ describe('SelectPackage Component', () => {
render()
// Navigate to selectPackage
- const input = screen.getByTestId('repo-url-input')
+ const input = getRepoUrlInput()
fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
- fireEvent.click(screen.getByTestId('next-btn'))
+ fireEvent.click(getNextButton())
await waitFor(() => {
expect(screen.getByTestId('select-package-step')).toBeInTheDocument()
@@ -1601,7 +1478,7 @@ describe('SelectPackage Component', () => {
fireEvent.click(screen.getByTestId('back-btn'))
await waitFor(() => {
- expect(screen.getByTestId('set-url-step')).toBeInTheDocument()
+ expect(getRepoUrlInput()).toBeInTheDocument()
})
})
diff --git a/web/app/components/plugins/install-plugin/install-from-github/index.tsx b/web/app/components/plugins/install-plugin/install-from-github/index.tsx
index b0667a9ee0..71b0a431d3 100644
--- a/web/app/components/plugins/install-plugin/install-from-github/index.tsx
+++ b/web/app/components/plugins/install-plugin/install-from-github/index.tsx
@@ -2,6 +2,7 @@
import type { PluginDeclaration, UpdateFromGitHubPayload } from '../../types'
import type { InstallState } from '@/app/components/plugins/types'
+import { Button } from '@langgenius/dify-ui/button'
import { cn } from '@langgenius/dify-ui/cn'
import { Dialog, DialogCloseButton, DialogContent } from '@langgenius/dify-ui/dialog'
import { toast } from '@langgenius/dify-ui/toast'
@@ -17,7 +18,6 @@ import useRefreshPluginList from '../hooks/use-refresh-plugin-list'
import { convertRepoToUrl, parseGitHubUrl } from '../utils'
import Loaded from './steps/loaded'
import SelectPackage from './steps/selectPackage'
-import SetURL from './steps/setURL'
const i18nPrefix = 'installFromGitHub'
@@ -194,12 +194,41 @@ const InstallFromGitHub: React.FC = ({ updatePayload, on
: (
{state.step === InstallStepFromGitHub.setUrl && (
-
setState(prevState => ({ ...prevState, repoUrl: value }))}
- onNext={handleUrlSubmit}
- onCancel={onClose}
- />
+ <>
+
+ setState(prevState => ({ ...prevState, repoUrl: e.target.value }))}
+ className="flex grow items-center gap-0.5 self-stretch overflow-hidden rounded-lg border border-components-input-border-active bg-components-input-bg-active p-2 system-sm-regular text-ellipsis text-components-input-text-filled outline-hidden"
+ placeholder="Please enter GitHub repo URL"
+ />
+
+
+
+
+ >
)}
{state.step === InstallStepFromGitHub.selectPackage && (
= ({ updatePayload, on
onBack={handleBack}
/>
)}
- {state.step === InstallStepFromGitHub.readyToInstall && (
+ {state.step === InstallStepFromGitHub.readyToInstall && manifest && uniqueIdentifier && (
{
- const defaultProps = {
- repoUrl: '',
- onChange: vi.fn(),
- onNext: vi.fn(),
- onCancel: vi.fn(),
- }
-
- beforeEach(() => {
- vi.clearAllMocks()
- })
-
- // ================================
- // Rendering Tests
- // ================================
- describe('Rendering', () => {
- it('should render label with GitHub repo text', () => {
- render()
-
- expect(screen.getByText('plugin.installFromGitHub.gitHubRepo')).toBeInTheDocument()
- })
-
- it('should render input field with correct attributes', () => {
- render()
-
- const input = screen.getByRole('textbox')
- expect(input).toBeInTheDocument()
- expect(input).toHaveAttribute('type', 'url')
- expect(input).toHaveAttribute('id', 'repoUrl')
- expect(input).toHaveAttribute('name', 'repoUrl')
- expect(input).toHaveAttribute('placeholder', 'Please enter GitHub repo URL')
- })
-
- it('should render cancel button', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.cancel' })).toBeInTheDocument()
- })
-
- it('should render next button', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeInTheDocument()
- })
-
- it('should associate label with input field', () => {
- render()
-
- const input = screen.getByLabelText('plugin.installFromGitHub.gitHubRepo')
- expect(input).toBeInTheDocument()
- })
-
- it('should auto-focus the input on mount', async () => {
- render()
-
- const input = screen.getByRole('textbox')
- await waitFor(() => {
- expect(input).toHaveFocus()
- })
- })
- })
-
- // ================================
- // Props Tests
- // ================================
- describe('Props', () => {
- it('should display repoUrl value in input', () => {
- render()
-
- expect(screen.getByRole('textbox')).toHaveValue('https://github.com/test/repo')
- })
-
- it('should display empty string when repoUrl is empty', () => {
- render()
-
- expect(screen.getByRole('textbox')).toHaveValue('')
- })
- })
-
- // ================================
- // User Interactions Tests
- // ================================
- describe('User Interactions', () => {
- it('should call onChange when input value changes', () => {
- const onChange = vi.fn()
- render()
-
- const input = screen.getByRole('textbox')
- fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } })
-
- expect(onChange).toHaveBeenCalledTimes(1)
- expect(onChange).toHaveBeenCalledWith('https://github.com/owner/repo')
- })
-
- it('should call onCancel when cancel button is clicked', () => {
- const onCancel = vi.fn()
- render()
-
- fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.cancel' }))
-
- expect(onCancel).toHaveBeenCalledTimes(1)
- })
-
- it('should call onNext when next button is clicked', () => {
- const onNext = vi.fn()
- render()
-
- fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.next' }))
-
- expect(onNext).toHaveBeenCalledTimes(1)
- })
- })
-
- // ================================
- // Button State Tests
- // ================================
- describe('Button State', () => {
- it('should disable next button when repoUrl is empty', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeDisabled()
- })
-
- it('should disable next button when repoUrl is only whitespace', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeDisabled()
- })
-
- it('should enable next button when repoUrl has content', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).not.toBeDisabled()
- })
-
- it('should not disable cancel button regardless of repoUrl', () => {
- render()
-
- expect(screen.getByRole('button', { name: 'plugin.installModal.cancel' })).not.toBeDisabled()
- })
- })
-
- // ================================
- // Edge Cases Tests
- // ================================
- describe('Edge Cases', () => {
- it('should handle URL with special characters', () => {
- const onChange = vi.fn()
- render()
-
- const input = screen.getByRole('textbox')
- fireEvent.change(input, { target: { value: 'https://github.com/test-org/repo_name-123' } })
-
- expect(onChange).toHaveBeenCalledWith('https://github.com/test-org/repo_name-123')
- })
-
- it('should handle very long URLs', () => {
- const longUrl = `https://github.com/${'a'.repeat(100)}/${'b'.repeat(100)}`
- render()
-
- expect(screen.getByRole('textbox')).toHaveValue(longUrl)
- })
-
- it('should handle onChange with empty string', () => {
- const onChange = vi.fn()
- render()
-
- const input = screen.getByRole('textbox')
- fireEvent.change(input, { target: { value: '' } })
-
- expect(onChange).toHaveBeenCalledWith('')
- })
-
- it('should preserve callback references on rerender', () => {
- const onNext = vi.fn()
- const { rerender } = render()
-
- rerender()
-
- fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.next' }))
-
- expect(onNext).toHaveBeenCalledTimes(1)
- })
- })
-})
diff --git a/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx b/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx
deleted file mode 100644
index 45ce5980ac..0000000000
--- a/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx
+++ /dev/null
@@ -1,69 +0,0 @@
-'use client'
-
-import { Button } from '@langgenius/dify-ui/button'
-import * as React from 'react'
-import { useTranslation } from 'react-i18next'
-
-type SetURLProps = {
- repoUrl: string
- onChange: (value: string) => void
- onNext: () => void
- onCancel: () => void
-}
-
-const SetURL: React.FC = ({ repoUrl, onChange, onNext, onCancel }) => {
- const { t } = useTranslation()
- const inputRef = React.useRef(null)
-
- // Focus the input after the dropdown's focus-return animation settles.
- // Using rAF avoids racing the DropdownMenu FloatingFocusManager that returns
- // focus to the trigger on close.
- React.useEffect(() => {
- const frame = requestAnimationFrame(() => {
- inputRef.current?.focus()
- })
- return () => cancelAnimationFrame(frame)
- }, [])
-
- return (
- <>
-
- onChange(e.target.value)}
- className="shadows-shadow-xs flex grow items-center gap-[2px] self-stretch
- overflow-hidden rounded-lg border border-components-input-border-active bg-components-input-bg-active p-2
- system-sm-regular text-ellipsis text-components-input-text-filled outline-hidden"
- placeholder="Please enter GitHub repo URL"
- />
-
-
-
-
- >
- )
-}
-
-export default SetURL
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 9f883af1b2..bc39cfdbd4 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
@@ -60,16 +60,18 @@ vi.mock('@langgenius/dify-ui/dropdown-menu', async () => {
DropdownMenu: ({
open,
onOpenChange,
+ modal,
children,
}: {
open: boolean
onOpenChange?: (open: boolean) => void
+ modal?: boolean
children: React.ReactNode
}) => {
portalOpen = open
return (
- {children}
+ {children}
)
},
@@ -158,6 +160,7 @@ describe('InstallPluginDropdown', () => {
fireEvent.click(screen.getByTestId('dropdown-trigger'))
+ expect(screen.getByTestId('dropdown-menu')).toHaveAttribute('data-modal', 'false')
expect(screen.getByText('plugin.installFrom')).toBeInTheDocument()
expect(screen.getByText('plugin.source.marketplace')).toBeInTheDocument()
expect(screen.getByText('plugin.source.github')).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 a972c9891c..8e996345af 100644
--- a/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx
+++ b/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx
@@ -111,7 +111,7 @@ const InstallPluginDropdown = ({
}
return (
-
+