youngkingdom/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-05-05 09:58:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(skill-editor): lift Ctrl+S handler to Provider and remove redundant hook

Browse Source 
Move global keyboard shortcut handling from component-level hook to
SkillSaveProvider, eliminating duplicate event listener registrations
and race conditions. Delete use-skill-file-save hook as its logic is
now consolidated in the provider with direct store access.
This commit is contained in:
yyh
2026-01-25 21:17:25 +08:00
parent 84d032c104
commit cdcd9fd1a2
4 changed files with 33 additions and 285 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
web/app/components/workflow/skill/file-content-panel.tsx
View File

@ -19,7 +19,6 @@ import MarkdownFileEditor from './editor/markdown-file-editor'
import { useFileTypeInfo } from './hooks/use-file-type-info'
import { useSkillAssetNodeMap } from './hooks/use-skill-asset-tree'
import { useSkillFileData } from './hooks/use-skill-file-data'
import { useSkillFileSave } from './hooks/use-skill-file-save'
import { useSkillSaveManager } from './hooks/use-skill-save-manager'
import StartTabContent from './start-tab'
import { getFileLanguage } from './utils/file-utils'
@ -100,25 +99,17 @@ const FileContentPanel: FC = () => {
storeApi.getState().pinTab(fileTabId)
}, [fileTabId, isEditable, originalContent, storeApi])
useSkillFileSave({
appId,
activeTabId: fileTabId,
isEditable,
originalContent,
currentMetadata,
t,
})
const { saveFile, registerFallback, unregisterFallback } = useSkillSaveManager()
const fallbackRef = useRef({ content: originalContent, metadata: currentMetadata })
fallbackRef.current = { content: originalContent, metadata: currentMetadata }
useEffect(() => {
if (!fileTabId || fileContent?.content === undefined)
return
registerFallback(fileTabId, { content: originalContent, metadata: currentMetadata })
const fallback = { content: originalContent, metadata: currentMetadata }
fallbackRef.current = fallback
registerFallback(fileTabId, fallback)
return () => {
unregisterFallback(fileTabId)
@ -129,8 +120,8 @@ const FileContentPanel: FC = () => {
if (!fileTabId || !isEditable)
return
const { content: fallbackContent, metadata: fallbackMetadata } = fallbackRef.current
return () => {
const { content: fallbackContent, metadata: fallbackMetadata } = fallbackRef.current
void saveFile(fileTabId, {
fallbackContent,
fallbackMetadata,

209
web/app/components/workflow/skill/hooks/use-skill-file-save.spec.tsx
View File

@ -1,209 +0,0 @@
import type { SaveFileOptions, SaveResult } from './use-skill-save-manager'
import { act, renderHook, waitFor } from '@testing-library/react'
import Toast from '@/app/components/base/toast'
import { useSkillFileSave } from './use-skill-file-save'
const { mockSaveFile, mockToastNotify } = vi.hoisted(() => ({
mockSaveFile: vi.fn<(fileId: string, options?: SaveFileOptions) => Promise<SaveResult>>(),
mockToastNotify: vi.fn(),
}))
vi.mock('./use-skill-save-manager', () => ({
useSkillSaveManager: () => ({
saveFile: mockSaveFile,
}),
}))
vi.mock('@/app/components/base/toast', () => ({
default: {
notify: mockToastNotify,
},
}))
const createParams = (overrides: Partial<Parameters<typeof useSkillFileSave>[0]> = {}) => ({
appId: 'app-1',
activeTabId: 'file-1' as string | null,
isEditable: true,
originalContent: 'original content',
currentMetadata: { version: 1 } as Record<string, unknown>,
t: vi.fn(() => 'saved-message') as unknown as Parameters<typeof useSkillFileSave>[0]['t'],
...overrides,
})
// Scenario: save behavior and shortcut handling for skill file saves.
describe('useSkillFileSave', () => {
beforeEach(() => {
vi.clearAllMocks()
mockSaveFile.mockResolvedValue({ saved: false })
})
// Scenario: guard clauses prevent invalid saves.
describe('Guards', () => {
it('should return early when active tab id is missing', async () => {
// Arrange
const params = createParams({ activeTabId: null })
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(mockSaveFile).not.toHaveBeenCalled()
expect(Toast.notify).not.toHaveBeenCalled()
})
it('should return early when app id is empty', async () => {
// Arrange
const params = createParams({ appId: '' })
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(mockSaveFile).not.toHaveBeenCalled()
expect(Toast.notify).not.toHaveBeenCalled()
})
it('should return early when not editable', async () => {
// Arrange
const params = createParams({ isEditable: false })
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(mockSaveFile).not.toHaveBeenCalled()
expect(Toast.notify).not.toHaveBeenCalled()
})
})
// Scenario: save results surface as toast notifications.
describe('Save Results', () => {
it('should call saveFile with fallback data when valid', async () => {
// Arrange
const params = createParams({
originalContent: 'fallback content',
currentMetadata: { tag: 'v1' },
})
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(mockSaveFile).toHaveBeenCalledWith('file-1', {
fallbackContent: 'fallback content',
fallbackMetadata: { tag: 'v1' },
})
expect(Toast.notify).not.toHaveBeenCalled()
})
it('should show error toast when save fails', async () => {
// Arrange
const params = createParams()
mockSaveFile.mockResolvedValueOnce({ saved: false, error: new Error('boom') })
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(Toast.notify).toHaveBeenCalledWith({
type: 'error',
message: 'Error: boom',
})
expect(params.t).not.toHaveBeenCalled()
})
it('should show success toast when save succeeds', async () => {
// Arrange
const params = createParams()
mockSaveFile.mockResolvedValueOnce({ saved: true })
const { result } = renderHook(() => useSkillFileSave(params))
// Act
await act(async () => {
await result.current()
})
// Assert
expect(params.t).toHaveBeenCalledWith('api.saved', { ns: 'common' })
expect(Toast.notify).toHaveBeenCalledWith({
type: 'success',
message: 'saved-message',
})
})
})
// Scenario: Ctrl/Cmd+S triggers save and suppresses default behavior.
describe('Keyboard Shortcut', () => {
it('should trigger save on Ctrl+S', async () => {
// Arrange
const params = createParams()
renderHook(() => useSkillFileSave(params))
const event = new KeyboardEvent('keydown', { key: 's', ctrlKey: true, cancelable: true })
const preventDefault = vi.fn()
Object.defineProperty(event, 'preventDefault', { value: preventDefault })
// Act
act(() => {
window.dispatchEvent(event)
})
// Assert
await waitFor(() => {
expect(mockSaveFile).toHaveBeenCalled()
})
expect(preventDefault).toHaveBeenCalled()
})
it('should trigger save on Cmd+S', async () => {
// Arrange
const params = createParams()
renderHook(() => useSkillFileSave(params))
const event = new KeyboardEvent('keydown', { key: 's', metaKey: true, cancelable: true })
const preventDefault = vi.fn()
Object.defineProperty(event, 'preventDefault', { value: preventDefault })
// Act
act(() => {
window.dispatchEvent(event)
})
// Assert
await waitFor(() => {
expect(mockSaveFile).toHaveBeenCalled()
})
expect(preventDefault).toHaveBeenCalled()
})
it('should not trigger save when key is not s', async () => {
// Arrange
const params = createParams()
renderHook(() => useSkillFileSave(params))
const event = new KeyboardEvent('keydown', { key: 'x', ctrlKey: true, cancelable: true })
// Act
act(() => {
window.dispatchEvent(event)
})
// Assert
await waitFor(() => {
expect(mockSaveFile).not.toHaveBeenCalled()
})
})
})
})

63
web/app/components/workflow/skill/hooks/use-skill-file-save.ts
View File

@ -1,63 +0,0 @@
import type { TFunction } from 'i18next'
import { useEventListener } from 'ahooks'
import { useCallback } from 'react'
import Toast from '@/app/components/base/toast'
import { useSkillSaveManager } from './use-skill-save-manager'
type UseSkillFileSaveParams = {
appId: string
activeTabId: string | null
isEditable: boolean
originalContent: string
currentMetadata: Record<string, unknown> | undefined
t: TFunction<'workflow'>
}
/**
* Hook to handle file save logic and Ctrl+S keyboard shortcut.
* Returns the save handler function.
*/
export function useSkillFileSave({
appId,
activeTabId,
isEditable,
originalContent,
currentMetadata,
t,
}: UseSkillFileSaveParams): () => Promise<void> {
const { saveFile } = useSkillSaveManager()
const handleSave = useCallback(async () => {
if (!activeTabId || !appId || !isEditable)
return
const result = await saveFile(activeTabId, {
fallbackContent: originalContent,
fallbackMetadata: currentMetadata,
})
if (result.error) {
Toast.notify({
type: 'error',
message: String(result.error),
})
return
}
if (result.saved) {
Toast.notify({
type: 'success',
message: t('api.saved', { ns: 'common' }),
})
}
}, [activeTabId, appId, currentMetadata, isEditable, originalContent, saveFile, t])
useEventListener('keydown', (e: KeyboardEvent) => {
if ((e.ctrlKey || e.metaKey) && e.key === 's') {
e.preventDefault()
handleSave()
}
}, { target: window })
return handleSave
}

29
web/app/components/workflow/skill/hooks/use-skill-save-manager.tsx
View File

@ -1,7 +1,10 @@
import { useQueryClient } from '@tanstack/react-query'
import { useEventListener } from 'ahooks'
import isDeepEqual from 'fast-deep-equal'
import * as React from 'react'
import { useCallback, useMemo, useRef } from 'react'
import { useTranslation } from 'react-i18next'
import Toast from '@/app/components/base/toast'
import { useWorkflowStore } from '@/app/components/workflow/store'
import { consoleQuery } from '@/service/client'
import { useUpdateAppAssetFileContent } from '@/service/use-app-asset'
@ -53,6 +56,7 @@ export const SkillSaveProvider = ({
appId,
children,
}: SkillSaveProviderProps) => {
const { t } = useTranslation()
const storeApi = useWorkflowStore()
const queryClient = useQueryClient()
const updateContent = useUpdateAppAssetFileContent()
@ -225,6 +229,31 @@ export const SkillSaveProvider = ({
fallbackRegistryRef.current.delete(fileId)
}, [])
useEventListener('keydown', (e: KeyboardEvent) => {
if ((e.ctrlKey || e.metaKey) && e.key === 's') {
e.preventDefault()
const { activeTabId } = storeApi.getState()
if (!activeTabId || activeTabId === START_TAB_ID)
return
const fallback = fallbackRegistryRef.current.get(activeTabId)
void saveFile(activeTabId, {
fallbackContent: fallback?.content,
fallbackMetadata: fallback?.metadata,
}).then((result) => {
if (result.error) {
const errorMessage = result.error instanceof Error
? result.error.message
: String(result.error)
Toast.notify({ type: 'error', message: errorMessage })
}
else if (result.saved) {
Toast.notify({ type: 'success', message: t('api.saved', { ns: 'common' }) })
}
})
}
}, { target: typeof window !== 'undefined' ? window : undefined })
const value = useMemo<SkillSaveContextValue>(() => ({
saveFile,
saveAllDirty,