diff --git a/openspec/changes/add-skill-avatars-and-moderation/design.md b/openspec/changes/add-skill-avatars-and-moderation/design.md index 1796a5df..9bab90e0 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/design.md +++ b/openspec/changes/add-skill-avatars-and-moderation/design.md @@ -42,3 +42,12 @@ ## 裁剪组件选型调整（2026-10-13） 头像文件选择复用已有 `react-dropzone`，裁剪交互采用 `react-image-crop`（ISC，兼容项目 React 版本）。方形选区的拖动、缩放选区、触屏和键盘行为交由开源组件处理，移除自写居中裁剪与缩放交互。仅保留选区图像导出及私有上传／提交状态衔接，不改变审核、引用消费、保留／替换／移除规则。组件适配须验证非居中选区的实际导出结果、取消与迟到响应隔离，以及新增／更新／独立维护入口。 + +## 头像入口与弹窗简化（2026-10-13，用户人工反馈） + +- 详情页仅保留标题旁头像，删除专门头像状态卡片、重复候选图、供应商开关说明及已批准审核理由。审核治理能力不因展示简化而移除。 +- 我的技能自绘列表补齐名称旁头像，删除独立“管理头像”按钮与入口；统一“更新”页同时支持仅修改头像或上传新技能包。无技能包时只提交既有头像维护接口，不新增版本；有包时保留原子发布／头像关联流程。新建及驳回重提仍必须提供技能包。 +- 使用已有 Tailwind CSS 视觉样式及 Radix Dialog 封装承载 React Image Crop，不引入新的整体 UI 框架。父表单为64像素头像和简短操作；裁剪弹窗画布有界、自适应，错误和重试在弹窗内。 +- 弹窗取消／关闭／ESC恢复打开前草稿，上传确认成功后关闭弹窗并展示小预览；成功上传仍需父页面最终提交。更新页仅展示所有者需要处理的待审／失败／驳回信息及原有重试、申诉动作，保持治理闭环。 + +组件接入方式：仓库已有 `web/components.json`，本地 `shared/ui/Button`、`Dialog` 沿用 shadcn 的源码分发／组件组合方式，底层为 Radix 与 Tailwind。当前为 Tailwind 3 和旧 `default` 风格；本轮复用现有封装与主题，不重新执行 init，也不覆盖已有 Portal／焦点／主题定制。头像选择由本地预览、Button、Dialog 和 React Image Crop 组合，shadcn Avatar 的展示职责与图片裁剪／上传业务分开。 diff --git a/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md b/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md index 8f97d8d0..62723d7a 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md +++ b/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md @@ -110,13 +110,19 @@ PRIVATE 且仅所有者可访问的头像完成文件安全处理后可直接使 前后端 SHALL 提供可理解的头像提交与审核闭环。 -新增和更新 Skill 页面提供可选自定义头像、方形裁剪、上传预览及上传状态，不提供预设图标选择；未上传时使用首字母兜底。更新页面展示当前头像并提供保留、替换、移除三种动作，默认保留。“我的技能”提供单独修改头像的入口，复用同一套上传与裁剪组件，不要求发布新的 SkillVersion。详情页展示头像、审核状态和驳回原因，不再承载完整上传表单。技能内容与头像分别显示提交和审核状态，技能内容审核通过不代表头像审核通过。审核员可以读取其管辖范围候选并处理，机器单开时可以处理例外申诉。所有展示 Skill 身份的位置使用同一头像与兜底组件。 +新增和更新 Skill 页面提供可选自定义头像、方形裁剪、上传预览及上传状态，不提供预设图标选择；未上传时使用首字母兜底。更新页面展示当前头像并提供保留、替换、移除三种动作，默认保留。“我的技能”仅保留统一的“更新”入口，列表在技能名称旁显示头像。更新页复用同一套自定义头像组件，可只提交头像修改而不要求上传技能包或发布新的 SkillVersion；上传技能包时仍按原流程发布版本。选择图片后在项目已有弹窗内裁剪，父表单仅保留小头像预览与必要操作，不展开大图。关闭、取消或按 ESC 退出裁剪时恢复打开前的头像草稿。详情页仅在标题旁显示当前头像，不再显示单独的头像管理面板、重复图片、机器开关说明或已批准理由；所有者需要处理的头像待审状态和驳回原因集中在更新页。技能内容与头像分别显示提交和审核状态，技能内容审核通过不代表头像审核通过。审核员可以读取其管辖范围候选并处理，机器单开时可以处理例外申诉。所有展示 Skill 身份的位置使用同一头像与兜底组件。 #### Scenario: 申请等待审核 - **WHEN** 所有者上传候选后需人工决定 - **THEN** 页面展示待审状态且已发布头像维持不变 + +#### Scenario: 裁剪取消 +- **WHEN** 所有者已选择头像草稿，再选择另一张图并取消裁剪弹窗 +- **THEN** 恢复弹窗打开前的草稿，包括原临时上传引用或移除选择 +- **AND** 迟到的上传结果不得替换恢复后的草稿 + ### Requirement: REQ-AV-10 头像 SHALL 先私有上传再随提交关联 新增、更新或单独修改头像时，客户端 SHALL 先裁剪并上传图片到私有存储，上传成功后取得可用于提交的临时引用。选择了新图片但仍在上传或上传失败时，客户端不得提交该次操作；服务端亦须校验引用对应的文件上传成功、未过期、属于当前提交者且不可越权关联。未选择头像的新技能可正常提交。 @@ -144,7 +150,7 @@ PRIVATE 且仅所有者可访问的头像完成文件安全处理后可直接使 - **THEN** 服务端拒绝提交，不更改技能或头像状态 #### Scenario: 单独修改头像 -- **WHEN** 所有者从“我的技能”上传并提交新头像 +- **WHEN** 所有者从“我的技能”的“更新”入口上传并提交新头像 - **THEN** 复用相同上传、关联及审核流程，不创建新的 SkillVersion ### Requirement: REQ-AV-11 更新页移除 SHALL 在提交成功后生效 diff --git a/openspec/changes/add-skill-avatars-and-moderation/tasks.md b/openspec/changes/add-skill-avatars-and-moderation/tasks.md index 682285fd..d9835511 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/tasks.md +++ b/openspec/changes/add-skill-avatars-and-moderation/tasks.md @@ -49,3 +49,9 @@ - [x] 6.3 构建当前提交的唯一预览，实际验证新增／更新／独立维护并更新 PR #947 验收记录。 开源组件替换的验收：真实组件专项14项、全量前端965项及类型／Lint／构建通过；独立Review无阻塞。Chromium实际拖动红蓝测试图到左半，上传512 PNG三个采样点均为红色；中俄键盘操作断言选区位置改变；头像生命周期和无头像发布／下载两个轨道通过。机器供应商真实调用未执行，唯一预览继续保留供人工验收。 + +## 7. 人工验收后的入口与展示优化（2026-10-13，实施中） + +- [x] 7.1 简化详情展示，补齐我的技能列表头像，合并为唯一更新入口，保留仅改头像不新增版本能力。 +- [x] 7.2 复用 Radix 弹窗承载开源裁剪组件，父表单紧凑展示，取消恢复草稿并隔离迟到结果。 +- [ ] 7.3 验证更新页头像单独提交／带技能包提交、审核恢复、列表／详情、桌面／移动弹窗和键盘焦点，独立Review后更新同一预览供人工验收。 diff --git a/web/src/features/skill/skill-avatar-cache.test.tsx b/web/src/features/skill/skill-avatar-cache.test.tsx index c7f7f5ef..aa3e8d1a 100644 --- a/web/src/features/skill/skill-avatar-cache.test.tsx +++ b/web/src/features/skill/skill-avatar-cache.test.tsx @@ -2,13 +2,9 @@ import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query' import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' -const api = vi.hoisted(() => ({ status: vi.fn(), submit: vi.fn(), queue: vi.fn(), decide: vi.fn() })) +const api = vi.hoisted(() => ({ status: vi.fn(), appeal: vi.fn(), queue: vi.fn(), decide: vi.fn() })) vi.mock('@/api/client', () => ({ skillAvatarApi: api, buildApiUrl: (url: string) => url })) vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) -vi.mock('./skill-avatar-editor', () => ({ - KEEP_AVATAR: { action: 'KEEP', ready: true }, - SkillAvatarEditor: ({ onChange }: { onChange: (value: { action: 'REPLACE'; ready: boolean; uploadId: string }) => void }) => , -})) import { SkillAvatarManager } from './skill-avatar-manager' import { SkillAvatarReviewQueue } from './skill-avatar-review' @@ -20,19 +16,18 @@ const active = { ...pending, avatarUrl: '/active', candidateStatus: 'ACTIVE', re describe('avatar cache across owner and reviewer flows', () => { afterEach(() => { cleanup(); Object.values(api).forEach(fn => fn.mockReset()) }) - it('refreshes a previously empty fresh review queue when the owner submits a staged image', async () => { + it('refreshes a previously empty fresh review queue when the owner appeals a blocked candidate', async () => { const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } }) - client.setQueryData(ownerKey, { ...active, candidateId: 20 }) + client.setQueryData(ownerKey, { ...pending, candidateId: 20, machineStatus: 'BLOCKED', reviewStatus: 'NOT_REQUIRED' }) client.setQueryData(queueKey, []) api.status.mockResolvedValue(pending) - api.submit.mockResolvedValue(pending) + api.appeal.mockResolvedValue(pending) api.queue.mockResolvedValue([pending]) render() expect(screen.getByText('skillAvatar.emptyQueue')).toBeTruthy() expect(api.queue).not.toHaveBeenCalled() - fireEvent.click(screen.getByText('Use staged image')) - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.saveChanges' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.appeal' })) expect(await screen.findByText('@team/foo')).toBeTruthy() expect(client.getQueryData(ownerKey)).toEqual(pending) diff --git a/web/src/features/skill/skill-avatar-editor.test.tsx b/web/src/features/skill/skill-avatar-editor.test.tsx index d67a0b39..6fe4832c 100644 --- a/web/src/features/skill/skill-avatar-editor.test.tsx +++ b/web/src/features/skill/skill-avatar-editor.test.tsx @@ -1,5 +1,5 @@ /** @vitest-environment jsdom */ -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const api = vi.hoisted(() => ({ stage: vi.fn() })) vi.mock('@/api/client', () => ({ skillAvatarApi: api, buildApiUrl: (url: string) => url })) @@ -8,7 +8,8 @@ import { SkillAvatarEditor } from './skill-avatar-editor' async function selectImage() { fireEvent.change(screen.getByLabelText('skillAvatar.choose'), { target: { files: [new File(['image'], 'avatar.png', { type: 'image/png' })] } }) - const image = await screen.findByAltText('skillAvatar.cropPreview') + const dialog = await screen.findByRole('dialog') + const image = within(dialog).getByAltText('skillAvatar.cropPreview') fireEvent.load(image) } function deferred() { @@ -41,7 +42,7 @@ describe('SkillAvatarEditor staged uploads', () => { const initialLeft = area.style.left fireEvent.keyDown(area, { key: 'ArrowRight', shiftKey: true }) expect(area.style.left).not.toBe(initialLeft) - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) const context = vi.mocked(HTMLCanvasElement.prototype.getContext).mock.results[0].value as unknown as { translate: ReturnType; drawImage: ReturnType } // Initial square is 576px at x=224. Library Shift+Right moves 10 display px = 40 source px. @@ -58,7 +59,7 @@ describe('SkillAvatarEditor staged uploads', () => { fireEvent.pointerDown(area, { clientX: 128, clientY: 96 }) fireEvent.pointerMove(document, { clientX: 148, clientY: 106 }) fireEvent.pointerUp(document) - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) const context = vi.mocked(HTMLCanvasElement.prototype.getContext).mock.results[0].value as unknown as { translate: ReturnType } expect(context.translate.mock.calls[0][0]).toBeCloseTo(-304) @@ -71,9 +72,9 @@ describe('SkillAvatarEditor staged uploads', () => { vi.mocked(HTMLCanvasElement.prototype.toBlob).mockImplementation(callback => { finish = callback }) render() await selectImage() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(finish).toBeTypeOf('function')) - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.discardChange' })) + fireEvent.click(screen.getByRole('button', { name: 'common.cancel' })) finish(new Blob(['cropped'], { type: 'image/png' })) await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'KEEP', ready: true })) expect(api.stage).not.toHaveBeenCalled() @@ -83,10 +84,13 @@ describe('SkillAvatarEditor staged uploads', () => { api.stage.mockReturnValue(new Promise(() => {})) render() const input = screen.getByLabelText('skillAvatar.choose') as HTMLInputElement - expect(input.tabIndex).toBe(0) + expect(input.tabIndex).toBe(-1) + const choose = screen.getByRole('button', { name: 'skillAvatar.choose' }) as HTMLButtonElement + expect(choose.disabled).toBe(false) + expect(input.className).toContain('sr-only') expect(input.disabled).toBe(false) await selectImage() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) expect(input.disabled).toBe(true) }) @@ -103,7 +107,7 @@ describe('SkillAvatarEditor staged uploads', () => { render() fireEvent.change(screen.getByLabelText('skillAvatar.choose'), { target: { files: [new File(['image'], 'avatar.png', { type: 'image/png' })] } }) const image = await screen.findByAltText('skillAvatar.cropPreview') - expect((screen.getByRole('button', { name: 'skillAvatar.cropUpload' }) as HTMLButtonElement).disabled).toBe(true) + expect((screen.getByRole('button', { name: 'skillAvatar.cropConfirm' }) as HTMLButtonElement).disabled).toBe(true) fireEvent.error(image) expect(screen.getByRole('alert').textContent).toBe('skillAvatar.invalid') expect(api.stage).not.toHaveBeenCalled() @@ -116,7 +120,7 @@ describe('SkillAvatarEditor staged uploads', () => { render() await selectImage() expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) expect(api.stage.mock.calls[0][0]).toBeInstanceOf(Blob) expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.UPLOADING') @@ -134,7 +138,8 @@ describe('SkillAvatarEditor staged uploads', () => { expect(api.stage).not.toHaveBeenCalled() fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.discardChange' })) expect(change).toHaveBeenLastCalledWith({ action: 'KEEP', ready: true }) - expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.KEEP') + expect(screen.queryByRole('status')).toBeNull() + expect(document.querySelector('img')?.getAttribute('src')).toBe('/active') }) it('keeps submission blocked after upload failure and allows a successful retry', async () => { @@ -142,26 +147,28 @@ describe('SkillAvatarEditor staged uploads', () => { api.stage.mockRejectedValueOnce(new Error('storage unavailable')).mockResolvedValueOnce(uploaded) render() await selectImage() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) expect((await screen.findByRole('alert')).textContent).toBe('storage unavailable') expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.uploadRetry' })) await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) }) - it.each(['discardChange', 'remove'] as const)('ignores an old upload completion after %s', async action => { + it.each(['cancel', 'escape'] as const)('ignores an old upload completion after %s', async action => { const change = vi.fn() const task = deferred() api.stage.mockReturnValue(task.promise) render() await selectImage() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) - fireEvent.click(screen.getByRole('button', { name: `skillAvatar.${action}` })) + if (action === 'cancel') fireEvent.click(screen.getByRole('button', { name: 'common.cancel' })) + else fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) task.resolve(uploaded) - await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: action === 'remove' ? 'REMOVE' : 'KEEP', ready: true })) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'KEEP', ready: true })) expect(change.mock.calls.some(([value]) => value.uploadId)).toBe(false) expect(screen.queryByAltText('skillAvatar.cropPreview')).toBeNull() + expect(screen.queryByRole('dialog')).toBeNull() }) it('invalidates a staged reference when a different image is selected', async () => { @@ -169,11 +176,73 @@ describe('SkillAvatarEditor staged uploads', () => { api.stage.mockResolvedValue(uploaded) render() await selectImage() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) await selectImage() expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) - expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.SELECTED') + expect(screen.getByRole('dialog')).toBeTruthy() + expect(screen.queryByRole('status')).toBeNull() + }) + + + it('uses the shared portal dialog and restores chooser focus when Escape cancels', async () => { + const { container } = render() + const chooser = screen.getByRole('button', { name: 'skillAvatar.choose' }) + chooser.focus() + await selectImage() + const dialog = screen.getByRole('dialog', { name: 'skillAvatar.cropTitle' }) + expect(container.contains(dialog)).toBe(false) + expect(within(dialog).getByText('skillAvatar.cropHelp')).toBeTruthy() + expect(container.querySelector('.ReactCrop')).toBeNull() + fireEvent.keyDown(dialog, { key: 'Escape' }) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + await waitFor(() => expect(document.activeElement).toBe(chooser)) + expect(api.stage).not.toHaveBeenCalled() + expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:chosen') + }) + + it('restores an already uploaded replacement when a second crop is canceled', async () => { + const change = vi.fn() + api.stage.mockResolvedValue(uploaded) + render() + await selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + await selectImage() + fireEvent.click(screen.getByRole('button', { name: 'common.cancel' })) + expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true }) + expect(screen.getByAltText('skillAvatar.cropPreview').getAttribute('src')).toBe(uploaded.previewUrl) + expect(api.stage).toHaveBeenCalledOnce() + }) + + it('does not overwrite a prior staged draft with a late second upload', async () => { + const change = vi.fn() + const late = deferred() + api.stage.mockResolvedValueOnce(uploaded).mockReturnValueOnce(late.promise) + render() + await selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + await selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropConfirm' })) + await waitFor(() => expect(api.stage).toHaveBeenCalledTimes(2)) + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + late.resolve({ ...uploaded, uploadId: 'stale-second-upload', previewUrl: '/private/second' }) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) + expect(change.mock.calls.some(([value]) => value.uploadId === 'stale-second-upload')).toBe(false) + expect(screen.getByAltText('skillAvatar.cropPreview').getAttribute('src')).toBe(uploaded.previewUrl) + }) + + it('restores a removal draft when the crop dialog close button is used', async () => { + const change = vi.fn() + render() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.remove' })) + await selectImage() + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(change).toHaveBeenLastCalledWith({ action: 'REMOVE', ready: true }) + expect(screen.queryByRole('dialog')).toBeNull() + expect(screen.getByText('F')).toBeTruthy() + expect(api.stage).not.toHaveBeenCalled() }) it('previews removal as a draft and cancel restores the original image', () => { diff --git a/web/src/features/skill/skill-avatar-editor.tsx b/web/src/features/skill/skill-avatar-editor.tsx index c5e7923a..dce9bd08 100644 --- a/web/src/features/skill/skill-avatar-editor.tsx +++ b/web/src/features/skill/skill-avatar-editor.tsx @@ -5,7 +5,7 @@ import 'react-image-crop/dist/ReactCrop.css' import { useTranslation } from 'react-i18next' import { buildApiUrl, skillAvatarApi } from '@/api/client' import { Button } from '@/shared/ui/button' -import { Input } from '@/shared/ui/input' +import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription, DialogFooter } from '@/shared/ui/dialog' import { SkillAvatar } from './skill-avatar' export type AvatarEdit = { action: 'KEEP' | 'REPLACE' | 'REMOVE'; uploadId?: string; ready: boolean } @@ -32,6 +32,10 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable const [previewUrl, setPreviewUrl] = useState(null) const [state, setState] = useState<'KEEP' | 'SELECTED' | 'UPLOADING' | 'READY' | 'ERROR' | 'REMOVE'>('KEEP') const [error, setError] = useState('') + const [dialogOpen, setDialogOpen] = useState(false) + const edit = useRef(KEEP_AVATAR) + const beforeSelection = useRef({ edit: KEEP_AVATAR, previewUrl: null as string | null, state: 'KEEP' as typeof state }) + const chooseButton = useRef(null) const generation = useRef(0) const image = useRef(null) useEffect(() => () => { generation.current += 1 }, []) @@ -45,7 +49,22 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable setState(remove ? 'REMOVE' : 'KEEP') setCrop(undefined) if (inputRef.current) inputRef.current.value = '' - onChange(remove ? { action: 'REMOVE', ready: true } : KEEP_AVATAR) + setDialogOpen(false) + edit.current = remove ? { action: 'REMOVE', ready: true } : KEEP_AVATAR + onChange(edit.current) + } + + function cancelSelection() { + generation.current += 1 + setDialogOpen(false) + setSourceUrl(null) + setCrop(undefined) + setError('') + setPreviewUrl(beforeSelection.current.previewUrl) + setState(beforeSelection.current.state) + edit.current = beforeSelection.current.edit + onChange(edit.current) + if (inputRef.current) inputRef.current.value = '' } async function upload() { @@ -61,7 +80,12 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable if (attempt !== generation.current) return setPreviewUrl(staged.previewUrl) setState('READY') - onChange({ action: 'REPLACE', uploadId: staged.uploadId, ready: true }) + edit.current = { action: 'REPLACE', uploadId: staged.uploadId, ready: true } + onChange(edit.current) + setDialogOpen(false) + setSourceUrl(null) + setCrop(undefined) + if (inputRef.current) inputRef.current.value = '' } catch (cause) { if (attempt !== generation.current) return setState('ERROR') @@ -69,7 +93,7 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable } } - const { getRootProps, getInputProps, inputRef } = useDropzone({ + const { getRootProps, getInputProps, inputRef, open } = useDropzone({ accept: { 'image/png': ['.png'], 'image/jpeg': ['.jpg', '.jpeg'] }, maxSize: 2 * 1024 * 1024, maxFiles: 1, @@ -78,6 +102,9 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable noClick: true, noKeyboard: true, onDrop: (accepted, rejected) => { + if (state === 'KEEP' || state === 'READY' || state === 'REMOVE') { + beforeSelection.current = { edit: edit.current, previewUrl, state } + } generation.current += 1 setPreviewUrl(null) setSourceUrl(null) @@ -88,45 +115,60 @@ export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disable setState('ERROR'); setError(t('skillAvatar.invalid')); return } setSourceUrl(URL.createObjectURL(file)) - setError(''); setState('SELECTED') + setError(''); setState('SELECTED'); setDialogOpen(true) }, }) - return
-

{t('skillAvatar.optionalTitle')}

-

{t('skillAvatar.stagedHelp')}

-
- {state === 'READY' && previewUrl ? {t('skillAvatar.cropPreview')} - : } -

{t(`skillAvatar.uploadStates.${state}`)}

+ return
+

{t('skillAvatar.optionalTitle')}

+
+ {state === 'READY' && previewUrl ? {t('skillAvatar.cropPreview')} + : } + +
+
+ + {(canRemove || state === 'READY') && state !== 'REMOVE' && } + {state !== 'KEEP' && !dialogOpen && } +
+ {state !== 'KEEP' && !dialogOpen &&

{t(state === 'READY' ? 'skillAvatar.stagedReady' : state === 'REMOVE' ? 'skillAvatar.removedDraft' : `skillAvatar.uploadStates.${state}`)}

} +
-
- -
- {sourceUrl && state !== 'READY' &&
- setCrop(percentCrop)} - ariaLabels={{ - cropArea: t('skillAvatar.cropArea'), - nwDragHandle: t('skillAvatar.cropHandles.nw'), nDragHandle: t('skillAvatar.cropHandles.n'), - neDragHandle: t('skillAvatar.cropHandles.ne'), eDragHandle: t('skillAvatar.cropHandles.e'), - seDragHandle: t('skillAvatar.cropHandles.se'), sDragHandle: t('skillAvatar.cropHandles.s'), - swDragHandle: t('skillAvatar.cropHandles.sw'), wDragHandle: t('skillAvatar.cropHandles.w'), - }}> - {t('skillAvatar.cropPreview')} { - const { naturalWidth, naturalHeight } = event.currentTarget - setCrop(centerCrop(makeAspectCrop({ unit: '%', width: Math.min(75, 75 * naturalHeight / naturalWidth) }, 1, naturalWidth, naturalHeight), naturalWidth, naturalHeight)) - }} onError={() => { setState('ERROR'); setError(t('skillAvatar.invalid')); setCrop(undefined) }} /> - -

{t('skillAvatar.cropHelp')}

- -
} -
- {state !== 'KEEP' && } - {canRemove && state !== 'REMOVE' && } -
- {error &&

{error}

} + {!dialogOpen && error &&

{error}

} + { if (!value) cancelSelection() }}> + { event.preventDefault(); chooseButton.current?.focus() }}> + + {t('skillAvatar.cropTitle')} + {t('skillAvatar.cropHelp')} + + {sourceUrl &&
+ setCrop(percentCrop)} + ariaLabels={{ + cropArea: t('skillAvatar.cropArea'), + nwDragHandle: t('skillAvatar.cropHandles.nw'), nDragHandle: t('skillAvatar.cropHandles.n'), + neDragHandle: t('skillAvatar.cropHandles.ne'), eDragHandle: t('skillAvatar.cropHandles.e'), + seDragHandle: t('skillAvatar.cropHandles.se'), sDragHandle: t('skillAvatar.cropHandles.s'), + swDragHandle: t('skillAvatar.cropHandles.sw'), wDragHandle: t('skillAvatar.cropHandles.w'), + }}> + {t('skillAvatar.cropPreview')} { + const { naturalWidth, naturalHeight } = event.currentTarget + setCrop(centerCrop(makeAspectCrop({ unit: '%', width: Math.min(75, 75 * naturalHeight / naturalWidth) }, 1, naturalWidth, naturalHeight), naturalWidth, naturalHeight)) + }} onError={() => { setState('ERROR'); setError(t('skillAvatar.invalid')); setCrop(undefined) }} /> + +
} + {state === 'UPLOADING' &&

{t('skillAvatar.uploadStates.UPLOADING')}

} + {error &&

{error}

} + + + + + +
} diff --git a/web/src/features/skill/skill-avatar-manager.test.tsx b/web/src/features/skill/skill-avatar-manager.test.tsx index 6c8aa0c1..5dd1b298 100644 --- a/web/src/features/skill/skill-avatar-manager.test.tsx +++ b/web/src/features/skill/skill-avatar-manager.test.tsx @@ -3,13 +3,13 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' -const api = vi.hoisted(() => ({ status: vi.fn(), delete: vi.fn(), submit: vi.fn() })) +const api = vi.hoisted(() => ({ status: vi.fn(), delete: vi.fn(), submit: vi.fn(), retry: vi.fn(), appeal: vi.fn() })) vi.mock('@/api/client', () => ({ skillAvatarApi: api, buildApiUrl: (url: string) => url })) vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) import { SkillAvatarManager } from './skill-avatar-manager' describe('SkillAvatarManager appeal', () => { - afterEach(() => { cleanup(); api.status.mockReset(); api.delete.mockReset(); api.submit.mockReset() }) + afterEach(() => { cleanup(); Object.values(api).forEach(fn => fn.mockReset()) }) it.each([ ['PENDING', 'BLOCKED', 'NOT_REQUIRED', true], @@ -36,26 +36,24 @@ describe('SkillAvatarManager appeal', () => { expect(screen.getByText('skillAvatar.automaticRetryStopped')).toBeTruthy() expect(screen.getByRole('button', { name: 'skillAvatar.retry' }).hasAttribute('disabled')).toBe(true) }) - it('renders detail status without upload, removal or retry controls', async () => { - api.status.mockResolvedValue({ avatarUrl: '/active', candidateId: 7, candidateStatus: 'REJECTED', machineStatus: 'BLOCKED', reviewStatus: 'REJECTED', reviewReason: 'not suitable' }) - const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() - expect(await screen.findByText('not suitable')).toBeTruthy() - expect(screen.queryByLabelText('skillAvatar.choose')).toBeNull() - expect(screen.queryByRole('button')).toBeNull() - }) - - it('does not remove an avatar until save and preserves the draft when the server rejects it', async () => { - api.status.mockResolvedValue({ avatarUrl: '/active', candidateId: 7, candidateStatus: 'ACTIVE', machineStatus: 'SKIPPED', reviewStatus: 'APPROVED' }) - api.delete.mockRejectedValue(new Error('save failed')) + it('calls the existing retry API and preserves the recoverable state on failure', async () => { + api.status.mockResolvedValue({ candidateId: 7, candidateStatus: 'PENDING', machineStatus: 'ERROR', reviewStatus: 'NOT_REQUIRED', machineNextRetryAt: null, automaticRetryExhausted: true }) + api.retry.mockRejectedValue(new Error('service unavailable')) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) render() - fireEvent.click(await screen.findByRole('button', { name: 'skillAvatar.remove' })) - expect(api.delete).not.toHaveBeenCalled() - fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.saveChanges' })) - expect((await screen.findByRole('alert')).textContent).toBe('save failed') - await waitFor(() => expect(api.delete).toHaveBeenCalledWith('team', 'foo')) - expect(screen.getByText('skillAvatar.uploadStates.REMOVE')).toBeTruthy() + fireEvent.click(await screen.findByRole('button', { name: 'skillAvatar.retry' })) + expect((await screen.findByRole('alert')).textContent).toBe('service unavailable') + expect(api.retry).toHaveBeenCalledWith('team', 'foo', 7) + expect(screen.getByRole('button', { name: 'skillAvatar.retry' }).hasAttribute('disabled')).toBe(false) }) + it('does not render approved reasons, duplicate image or editing controls', async () => { + api.status.mockResolvedValue({ avatarUrl: '/active', candidateId: 7, candidateStatus: 'ACTIVE', machineStatus: 'SKIPPED', reviewStatus: 'APPROVED', reviewReason: 'approved reason' }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + await waitFor(() => expect(api.status).toHaveBeenCalled()) + expect(screen.queryByText('approved reason')).toBeNull() + expect(screen.queryByRole('img')).toBeNull() + expect(screen.queryByRole('button')).toBeNull() + }) }) diff --git a/web/src/features/skill/skill-avatar-manager.tsx b/web/src/features/skill/skill-avatar-manager.tsx index 2cfa428f..18e62114 100644 --- a/web/src/features/skill/skill-avatar-manager.tsx +++ b/web/src/features/skill/skill-avatar-manager.tsx @@ -1,59 +1,41 @@ import { useState } from 'react' import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { useTranslation } from 'react-i18next' -import { buildApiUrl, skillAvatarApi } from '@/api/client' +import { skillAvatarApi } from '@/api/client' import { Button } from '@/shared/ui/button' -import { Card } from '@/shared/ui/card' -import { SkillAvatarEditor, KEEP_AVATAR, type AvatarEdit } from './skill-avatar-editor' -export function SkillAvatarManager({ namespace, slug, name = slug, readOnly = false }: { namespace: string; slug: string; name?: string; readOnly?: boolean }) { +export function SkillAvatarManager({ namespace, slug, disabled = false }: { namespace: string; slug: string; disabled?: boolean }) { const { t } = useTranslation() const queryClient = useQueryClient() const key = ['skill-avatar', namespace, slug] - const status = useQuery({ queryKey: key, queryFn: () => skillAvatarApi.status(namespace, slug) }) - const [edit, setEdit] = useState(KEEP_AVATAR) + const status = useQuery({ queryKey: key, queryFn: () => skillAvatarApi.status(namespace, slug), enabled: !!namespace && !!slug }) const [error, setError] = useState('') - const [editorVersion, setEditorVersion] = useState(0) const mutation = useMutation({ - mutationFn: async (action: { kind: 'upload'; uploadId: string } | { kind: 'remove' | 'appeal' | 'retry'; id?: number }) => { - if (action.kind === 'upload') return skillAvatarApi.submit(namespace, slug, action.uploadId) - if (action.kind === 'remove') return skillAvatarApi.delete(namespace, slug) - if (action.kind === 'appeal') return skillAvatarApi.appeal(namespace, slug, action.id!) - return skillAvatarApi.retry(namespace, slug, action.id!) - }, - onSuccess: async (_data, action) => { + mutationFn: async (action: { kind: 'appeal' | 'retry'; id: number }) => action.kind === 'appeal' + ? skillAvatarApi.appeal(namespace, slug, action.id) + : skillAvatarApi.retry(namespace, slug, action.id), + onSuccess: async () => { setError('') - if (action.kind === 'upload' || action.kind === 'remove') { - setEdit(KEEP_AVATAR) - setEditorVersion(value => value + 1) - } await Promise.all([queryClient.invalidateQueries({ queryKey: key }), queryClient.invalidateQueries({ queryKey: ['skill'] }), queryClient.invalidateQueries({ queryKey: ['skills'] }), queryClient.invalidateQueries({ queryKey: ['skill-avatar-review'] })]) }, onError: (cause) => setError(cause instanceof Error ? cause.message : t('skillAvatar.failed')), }) const candidate = status.data const retryCoolingDown = !!candidate?.machineNextRetryAt && Date.parse(candidate.machineNextRetryAt) > Date.now() + if (!candidate || !['PENDING', 'REJECTED'].includes(candidate.candidateStatus ?? '')) return null return ( - -

{t('skillAvatar.title')}

-

{t(readOnly ? 'skillAvatar.detailHelp' : 'skillAvatar.ownerHelp')}

- {candidate?.candidateStatus &&

{t('skillAvatar.status')}: {t(`skillAvatar.states.${candidate.candidateStatus}`)} · {t(`skillAvatar.machine.${candidate.machineStatus ?? 'SKIPPED'}`)}

} +
+ {candidate?.candidateStatus &&

{t('skillAvatar.status')}: {t(`skillAvatar.states.${candidate.candidateStatus}`)}

} {candidate?.reviewReason &&

{candidate.reviewReason}

} {candidate?.machineStatus === 'ERROR' && retryCoolingDown &&

{t('skillAvatar.retryAfter', { time: new Date(candidate.machineNextRetryAt!).toLocaleString() })}

} {candidate?.machineStatus === 'ERROR' && candidate.automaticRetryExhausted &&

{t('skillAvatar.automaticRetryStopped')}

} - {candidate?.candidateUrl && {t('skillAvatar.candidatePreview')}} - {!readOnly && <> - - - } - {status.isError &&

{t('skillAvatar.failed')}

}
- {!readOnly && candidate?.candidateStatus === 'PENDING' && candidate.machineStatus === 'BLOCKED' + {candidate?.candidateStatus === 'PENDING' && candidate.machineStatus === 'BLOCKED' && candidate.reviewStatus === 'NOT_REQUIRED' && candidate.candidateId - && } - {!readOnly && candidate?.machineStatus === 'ERROR' && candidate.candidateId && } + && } + {candidate?.machineStatus === 'ERROR' && candidate.candidateId && }
{error &&

{error}

} - +
) } diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 3843f05c..62949c2e 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -1833,7 +1833,12 @@ "frontmatterFailedTitle": "SKILL.md format is invalid", "frontmatterFailedDescription": "Please check the YAML frontmatter at the top of SKILL.md. If a field value contains a colon, wrap it in quotes.", "selectRequired": "Please select namespace and file", - "folderPackagingFailed": "Could not package the selected folder. Make sure it contains files." + "folderPackagingFailed": "Could not package the selected folder. Make sure it contains files.", + "updateTitle": "Update skill", + "updateSubtitle": "Save avatar changes independently, or upload a package to publish a new version.", + "optionalPackage": "A package is optional when changing only the avatar.", + "saveAvatar": "Save avatar", + "avatarSaved": "Avatar changes submitted" }, "toast": { "success": "Success", @@ -2667,6 +2672,11 @@ "PASSED": "Machine review passed", "BLOCKED": "Machine review blocked", "ERROR": "Machine review failed" - } + }, + "cropTitle": "Crop avatar", + "cropConfirm": "Confirm and upload", + "change": "Change image", + "stagedReady": "Uploaded; applied after submission", + "removedDraft": "Removed after submission" } } diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index 37ced423..65b95619 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -1824,7 +1824,12 @@ "frontmatterFailedTitle": "Неверный формат SKILL.md", "frontmatterFailedDescription": "Проверьте YAML frontmatter в начале SKILL.md. Если значение поля содержит двоеточие, заключите его в кавычки.", "selectRequired": "Выберите пространство имён и файл", - "folderPackagingFailed": "Не удалось упаковать выбранную папку. Убедитесь, что она содержит файлы." + "folderPackagingFailed": "Не удалось упаковать выбранную папку. Убедитесь, что она содержит файлы.", + "updateTitle": "Обновить навык", + "updateSubtitle": "Сохраните аватар отдельно или загрузите пакет для публикации новой версии.", + "optionalPackage": "Для изменения только аватара пакет не требуется.", + "saveAvatar": "Сохранить аватар", + "avatarSaved": "Изменения аватара отправлены" }, "ratingInput": { "yourRating": "Ваша оценка: {{score}} зв." @@ -2669,6 +2674,11 @@ "PASSED": "Проверка пройдена", "BLOCKED": "Заблокирован", "ERROR": "Ошибка проверки" - } + }, + "cropTitle": "Обрезка аватара", + "cropConfirm": "Подтвердить и загрузить", + "change": "Заменить изображение", + "stagedReady": "Загружено; применится после отправки", + "removedDraft": "Удалится после отправки" } } diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 6ee65bf7..3ee4790c 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -1832,7 +1832,12 @@ "frontmatterFailedTitle": "SKILL.md 格式有误", "frontmatterFailedDescription": "请检查 SKILL.md 顶部 frontmatter 的 YAML 格式。若字段值中包含冒号，请用引号包裹。", "selectRequired": "请选择命名空间和文件", - "folderPackagingFailed": "无法打包所选文件夹，请确认其中包含文件。" + "folderPackagingFailed": "无法打包所选文件夹，请确认其中包含文件。", + "updateTitle": "更新技能", + "updateSubtitle": "头像可单独保存；上传技能包将发布新版本。", + "optionalPackage": "仅更新头像时无需上传技能包。", + "saveAvatar": "保存头像", + "avatarSaved": "头像修改已提交" }, "toast": { "success": "成功", @@ -2666,6 +2671,11 @@ "PASSED": "机器审核通过", "BLOCKED": "机器审核阻止", "ERROR": "机器审核失败" - } + }, + "cropTitle": "裁剪头像", + "cropConfirm": "确认并上传", + "change": "更换图片", + "stagedReady": "已上传，提交后生效", + "removedDraft": "提交后移除头像" } } diff --git a/web/src/pages/dashboard/my-skills.test.ts b/web/src/pages/dashboard/my-skills.test.ts index 81c2bb43..28501665 100644 --- a/web/src/pages/dashboard/my-skills.test.ts +++ b/web/src/pages/dashboard/my-skills.test.ts @@ -101,6 +101,7 @@ vi.mock('@/shared/lib/toast', () => ({ })) vi.mock('@/api/client', () => ({ + buildApiUrl: (url: string) => url, ApiError: class ApiError extends Error { serverMessageKey?: string }, @@ -163,6 +164,14 @@ describe('MySkillsPage', () => { }) }) + it('displays the active avatar beside the skill name and exposes only the existing update entry', () => { + useMySkillsMock.mockReturnValue({ data: { items: [{ id: 1, displayName: 'Team Agent', namespace: 'team-ai', slug: 'team-agent', avatarUrl: '/skill-avatar.png', status: 'PUBLISHED', downloadCount: 0 }], total: 1 }, isLoading: false }) + const html = renderToStaticMarkup(createElement(MySkillsPage)) + expect(html).toContain('src="/skill-avatar.png"') + expect(buttonRecords.some(button => button.label === 'skillAvatar.manage')).toBe(false) + expect(buttonRecords.some(button => button.label === 'mySkills.update')).toBe(true) + }) + it('does not render update action for archived skills', () => { useMySkillsMock.mockReturnValue({ data: { diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 6b05d2a8..14fdcc17 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -18,8 +18,7 @@ import { getHeadlineVersion, getPublishedVersion, getOwnerPreviewVersion, hasPen import { formatCompactCount } from '@/shared/lib/number-format' import { toast } from '@/shared/lib/toast' import { buildReturnTo } from '@/shared/lib/auth-route' -import { SkillAvatarManager } from '@/features/skill/skill-avatar-manager' -import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from '@/shared/ui/dialog' +import { SkillAvatar } from '@/features/skill/skill-avatar' import { ApiError } from '@/api/client' import { getMySkillEmptyStateKey, getMySkillFilters, type MySkillFilter } from './my-skill-filters' @@ -59,7 +58,6 @@ export function MySkillsPage() { // Input remains local until the user explicitly submits the search. const [keywordInput, setKeywordInput] = useState(keyword) - const [avatarTarget, setAvatarTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [archiveTarget, setArchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [unarchiveTarget, setUnarchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [withdrawTarget, setWithdrawTarget] = useState<{ namespace: string; slug: string; name: string; version: string } | null>(null) @@ -382,11 +380,14 @@ export function MySkillsPage() { className={`p-5 group animate-fade-up delay-${Math.min(idx + 1, 6)} ${filter === 'HIDDEN' ? '' : 'cursor-pointer'}`} onClick={filter === 'HIDDEN' ? undefined : () => handleSkillClick(skill.namespace, skill.slug)} > -
-
-

- {skill.displayName} -

+
+
+
+ +

+ {skill.displayName} +

+
{skill.summary && (

{skill.summary}

)} @@ -423,11 +424,7 @@ export function MySkillsPage() { ) : null}
-
- {filter !== 'HIDDEN' && } +
{canRestoreHidden ? (