From efaa70e67cd85f35cf30b0d74e87903dba20627c Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:40:27 +0800 Subject: [PATCH] feat(web): streamline my skills cards and management actions Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/skillhub/en/guide/skill-publish.md | 8 + docs/skillhub/guide/skill-publish.md | 8 + .../improve-my-skills-management/design.md | 14 + .../improve-my-skills-management/tasks.md | 6 +- .../skill/skill-version-compare-content.tsx | 296 ++++++++++++++++ .../skill/skill-visibility-dialog.test.tsx | 18 + .../skill/skill-visibility-dialog.tsx | 16 +- web/src/i18n/locales/en.json | 9 + web/src/i18n/locales/zh.json | 9 + .../dashboard/my-skills-interactions.test.tsx | 192 +++++++++++ .../pages/dashboard/my-skills-scroll.test.ts | 19 + web/src/pages/dashboard/my-skills-scroll.ts | 16 + web/src/pages/dashboard/my-skills.test.ts | 23 +- web/src/pages/dashboard/my-skills.tsx | 325 ++++++++---------- web/src/pages/skill-version-compare.test.tsx | 1 + web/src/pages/skill-version-compare.tsx | 293 +--------------- 16 files changed, 778 insertions(+), 475 deletions(-) create mode 100644 web/src/features/skill/skill-version-compare-content.tsx create mode 100644 web/src/pages/dashboard/my-skills-interactions.test.tsx create mode 100644 web/src/pages/dashboard/my-skills-scroll.test.ts create mode 100644 web/src/pages/dashboard/my-skills-scroll.ts diff --git a/docs/skillhub/en/guide/skill-publish.md b/docs/skillhub/en/guide/skill-publish.md index f8d34c602..4f6bdfc82 100644 --- a/docs/skillhub/en/guide/skill-publish.md +++ b/docs/skillhub/en/guide/skill-publish.md @@ -29,6 +29,14 @@ Use Update in My Skills or publish again to the same namespace and skill name. V Existing local installations do not update automatically. Consumers explicitly upgrade through the CLI. Suite members remain pinned until a new Suite version is published. +### Tracking and Managing My Skills + +At `/dashboard/skills`, submit a keyword and combine namespace and status capsule filters. Changing a filter resets pagination; clearing filters removes the keyword, namespace and status together. The result count comes from the server for the current conditions, not the cards on the current page or per-status totals. + +Select a card title to open its actual detail page; returning preserves filters, page and scroll position. **Update** opens the existing update form. **More** exposes visibility, review withdrawal, promotion or archive actions according to the current permissions and state. Changing the frontend entry does not grant permissions; owners who have left a namespace remain read-only. When a published version and a pending change coexist, one main status describes the change while the effective version or visibility remains separately identified. + +Version comparison opens in a centered dialog and only compares accessible published versions; at least two are required. Comparing and closing do not change publication state. Details show content and comparison shows differences; neither replaces installation and execution checks. + ### Recovering from Submission Problems | Symptom | Next step | diff --git a/docs/skillhub/guide/skill-publish.md b/docs/skillhub/guide/skill-publish.md index 846b6f1cc..0f25acdda 100644 --- a/docs/skillhub/guide/skill-publish.md +++ b/docs/skillhub/guide/skill-publish.md @@ -29,6 +29,14 @@ my-skill/ 本地已有安装不会因服务端发布新版而自动更新。使用者需要明确执行 CLI 升级；套装成员还受套装固定版本约束，不能以成员发布新版本代替发布新的套装版本。 +### 在“我的技能”跟踪与管理 + +在 `/dashboard/skills` 提交关键字搜索，结合命名空间和状态胶囊缩小结果；修改筛选后从第一页显示，清除筛选会同时清除关键字、命名空间和状态。面板数量为当前条件下的服务端结果总数，不是当前页卡片数，也不是每种状态的总数。 + +卡片标题进入技能详情；返回列表时保留筛选、页码和滚动位置。“更新”进入原更新表单；“更多”菜单按当前权限和状态提供可见范围、撤回审核、推广或归档等操作。不可用操作不能通过更换前端入口获得权限；退出命名空间的所有者继续只读。已发布版本和待审变更共存时，主状态说明本次变更，当前生效版本或范围仍独立标注。 + +版本比较在居中弹窗内进行，仅比较有权读取的已发布版本；不足两个版本时无法比较。比较与关闭弹窗不改变发布状态。详情入口用于查看内容，比较入口用于检查差异；两者都不能替代实际安装和调用验证。 + ### 发布失败与处理方法 | 现象 | 下一步 | diff --git a/openspec/changes/improve-my-skills-management/design.md b/openspec/changes/improve-my-skills-management/design.md index f7e27fefa..0109f59ae 100644 --- a/openspec/changes/improve-my-skills-management/design.md +++ b/openspec/changes/improve-my-skills-management/design.md @@ -12,6 +12,20 @@ “更新”保留图标与文字，继续进入现有更新表单。“更多”采用具备可访问名称、键盘操作和焦点恢复的现有菜单组件。将当前操作条件从平铺按钮迁入菜单，不扩大权限、不增加删除能力。治理隐藏视图继续仅允许现有角色恢复。 +现有操作契约以列表实现、`managementState` 及后端授权为准。操作矩阵如下；“普通视图”指非隐藏筛选且所有者非只读。 + +| 操作 | 可见条件 | 说明 | +| --- | --- | --- | +| 更新 | 普通视图、容器非 ARCHIVED | 进入原更新表单 | +| 可见范围 | 普通视图、ACTIVE、有发布版 | 存在待审版本时保留禁用反馈 | +| 撤回审核 | 普通视图、ownerPreview 为 PENDING_REVIEW | 次要动作互斥链的第一分支 | +| 推广或更新全局版 | 未命中撤审，服务端 canSubmitPromotion 且有发布版 | 使用真实推广关系和发布版 ID | +| 取消归档 | 未命中撤审及推广，容器为 ARCHIVED | 保留原确认步骤 | +| 归档 | 未命中以上分支，有发布版 | 保留原确认步骤 | +| 恢复隐藏 | HIDDEN 视图且 SUPER_ADMIN | 不展示所有者更新等操作 | + +头像候选继续使用原 `previewOnly` 展示；不增加独立头像管理按钮。版本比较沿用现页规则，仅选择可访问的 PUBLISHED 版本且至少两个，不允许同一版本互比。 + 技能标题进入实际详情路由。返回上下文包含已有 `returnTo` 和列表滚动位置；导航状态不得跨技能、跨筛选错误复用。比较版本使用现有版本查询及差异组件，在用户打开居中 Dialog 时加载，保留加载、错误、空数据和重试状态。不得以抽屉替代。 ## 兼容与风险 diff --git a/openspec/changes/improve-my-skills-management/tasks.md b/openspec/changes/improve-my-skills-management/tasks.md index a40df9eb8..664787851 100644 --- a/openspec/changes/improve-my-skills-management/tasks.md +++ b/openspec/changes/improve-my-skills-management/tasks.md @@ -1,7 +1,7 @@ ## 1. 设计与契约 - [x] 明确独立 PR 范围、原型交互及现有业务约束 -- [ ] 核对完整状态/权限操作矩阵 -- [ ] 完成原型中的最终交互示意 +- [x] 核对完整状态/权限操作矩阵 +- [x] 完成原型中的最终交互示意（Figma 新增居中比较对照稿；只写入，未视觉核验） ## 2. 实现 - [x] 统一筛选面板及真实结果数量 @@ -11,7 +11,7 @@ ## 3. 验证与交付 - [x] 筛选首片针对性测试与静态检查 -- [ ] 创建 Draft PR 并持续更新 +- [x] 创建 Draft PR 并持续更新（#1062） - [ ] 集中独立 Review 与修复 - [ ] 冻结代码后的需求场景矩阵、源检查与精确提交运行时验证 - [ ] 保留人工预览入口并交付验证结果 diff --git a/web/src/features/skill/skill-version-compare-content.tsx b/web/src/features/skill/skill-version-compare-content.tsx new file mode 100644 index 000000000..355786b72 --- /dev/null +++ b/web/src/features/skill/skill-version-compare-content.tsx @@ -0,0 +1,296 @@ +import { useEffect, useMemo, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { useSkillVersionCompare, useSkillVersions } from '@/shared/hooks/use-skill-queries' +import { Button } from '@/shared/ui/button' +import { Input } from '@/shared/ui/input' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select' + +function toDiffSectionId(path: string) { + return `diff-${encodeURIComponent(path)}` +} + +function getChangeTypeLabel(t: (key: string) => string, changeType: string) { + if (changeType === 'ADDED') { + return t('skillCompare.changeTypeAdded') + } + if (changeType === 'REMOVED') { + return t('skillCompare.changeTypeRemoved') + } + return t('skillCompare.changeTypeModified') +} + +export function SkillVersionCompareContent({ namespace, slug, from = '', to = '', onVersionsChange }: { + namespace: string + slug: string + from?: string + to?: string + onVersionsChange: (versions: { from: string; to: string }, replace?: boolean) => void +}) { + const { t } = useTranslation() + const [keyword, setKeyword] = useState('') + const [activePath, setActivePath] = useState(null) + const { data: versions, isLoading: isVersionsLoading, error: versionsError, refetch: refetchVersions } = useSkillVersions(namespace, slug) + + const publishedVersions = useMemo( + () => (versions ?? []).filter((version) => version.status === 'PUBLISHED'), + [versions] + ) + + const hasEnoughVersions = publishedVersions.length >= 2 + const defaultFrom = publishedVersions[1]?.version ?? publishedVersions[0]?.version ?? '' + const defaultTo = publishedVersions[0]?.version ?? '' + + const normalizedVersions = useMemo(() => { + if (!hasEnoughVersions) { + return { from: '', to: '' } + } + + const isPublishedVersion = (value: string) => publishedVersions.some((version) => version.version === value) + let nextFrom = from && isPublishedVersion(from) ? from : defaultFrom + let nextTo = to && isPublishedVersion(to) ? to : defaultTo + + if (nextFrom === nextTo) { + const alternative = publishedVersions.find((version) => version.version !== nextFrom)?.version ?? '' + if (!from && !to) { + nextFrom = defaultFrom + nextTo = defaultTo + } else if (from && !to) { + nextTo = alternative + } else if (!from && to) { + nextFrom = alternative + } else { + nextTo = alternative + } + } + + return { from: nextFrom, to: nextTo } + }, [defaultFrom, defaultTo, from, hasEnoughVersions, publishedVersions, to]) + + useEffect(() => { + if (!hasEnoughVersions || !normalizedVersions.from || !normalizedVersions.to) { + return + } + if (from === normalizedVersions.from && to === normalizedVersions.to) { + return + } + onVersionsChange(normalizedVersions, true) + }, [from, hasEnoughVersions, onVersionsChange, normalizedVersions, to]) + + const { data, isLoading: isCompareLoading, error, refetch: refetchCompare } = useSkillVersionCompare( + namespace, + slug, + normalizedVersions.from, + normalizedVersions.to, + hasEnoughVersions && !!normalizedVersions.from && !!normalizedVersions.to + ) + + const filteredFiles = useMemo(() => { + const files = data?.files ?? [] + if (!keyword.trim()) { + return files + } + return files.filter((file) => file.path.toLowerCase().includes(keyword.trim().toLowerCase())) + }, [data?.files, keyword]) + + useEffect(() => { + if (!activePath && filteredFiles.length > 0) { + setActivePath(filteredFiles[0].path) + return + } + if (activePath && !filteredFiles.some((file) => file.path === activePath)) { + setActivePath(filteredFiles[0]?.path ?? null) + } + }, [activePath, filteredFiles]) + + if (isVersionsLoading) { + return
{t('skillCompare.loading')}
+ } + + if (versionsError) { + return
+

{t('skillCompare.errorLoadingCompare')}

+ +
+ } + + if (!hasEnoughVersions) { + return
{t('skillCompare.notEnoughPublishedVersions')}
+ } + + if (isCompareLoading) { + return
{t('skillCompare.loading')}
+ } + + if (error || !data) { + return
+

{t('skillCompare.errorLoadingCompare')}

+ +
+ } + + return ( +
+ + +
+ {filteredFiles.length > 0 ? ( + filteredFiles.map((file) => ( +
+
+
{file.path}
+
{getChangeTypeLabel(t, file.changeType)}
+
+ + {file.binary ? ( +
{t('skillCompare.binaryFile')}
+ ) : file.truncated ? ( +
{t('skillCompare.truncatedFile')}
+ ) : ( +
+ {file.hunks.flatMap((hunk) => hunk.lines).map((line, index) => ( +
+ + {line.oldLineNumber ?? ''} + + + {line.newLineNumber ?? ''} + + + {line.type === 'ADD' ? '+' : line.type === 'DELETE' ? '-' : ' '} + {line.content} + +
+ ))} +
+ )} +
+ )) + ) : ( +
+ {t('skillCompare.noFilesFound')} +
+ )} +
+
+ ) +} diff --git a/web/src/features/skill/skill-visibility-dialog.test.tsx b/web/src/features/skill/skill-visibility-dialog.test.tsx index ca61f8364..68e98dbcf 100644 --- a/web/src/features/skill/skill-visibility-dialog.test.tsx +++ b/web/src/features/skill/skill-visibility-dialog.test.tsx @@ -38,6 +38,24 @@ async function mount(visibility = 'PUBLIC', language = 'zh') { describe('SkillVisibilityDialog', () => { afterEach(() => { cleanup(); vi.clearAllMocks() }) + it('opens from a controlled menu without changing the selected private scope', async () => { + api.getVisibility.mockResolvedValue({ visibility: 'PRIVATE' }) + const i18n = createInstance() + await i18n.init({ lng: 'en', resources: { en: { translation: en } } }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const onOpenChange = vi.fn() + render( + + ) + await waitFor(() => expect(api.getVisibility).toHaveBeenCalledWith('team', 'foo')) + expect((screen.getByRole('combobox') as HTMLSelectElement).value).toBe('PRIVATE') + expect(screen.getByRole('button', { name: 'Save' }).hasAttribute('disabled')).toBe(true) + expect(screen.queryByRole('button', { name: en.skillVisibility.title })).toBeNull() + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(onOpenChange).toHaveBeenCalledWith(false) + expect(api.changeVisibility).not.toHaveBeenCalled() + }) + it.each([ ['zh', '取消', '保存'], ['en', 'Cancel', 'Save'], diff --git a/web/src/features/skill/skill-visibility-dialog.tsx b/web/src/features/skill/skill-visibility-dialog.tsx index e13b9281a..464119abb 100644 --- a/web/src/features/skill/skill-visibility-dialog.tsx +++ b/web/src/features/skill/skill-visibility-dialog.tsx @@ -10,13 +10,15 @@ import { toast } from '@/shared/lib/toast' const scopes = ['PUBLIC', 'NAMESPACE_ONLY', 'PRIVATE'] as const type Visibility = typeof scopes[number] -export function SkillVisibilityDialog({ namespace, slug, visibility, disabled = false }: { - namespace: string; slug: string; visibility: string; disabled?: boolean +export function SkillVisibilityDialog({ namespace, slug, visibility, disabled = false, open: controlledOpen, onOpenChange, onCloseAutoFocus, hideTrigger = false }: { + namespace: string; slug: string; visibility: string; disabled?: boolean; open?: boolean; onOpenChange?: (open: boolean) => void; onCloseAutoFocus?: (event: Event) => void; hideTrigger?: boolean }) { const { t } = useTranslation() const queryClient = useQueryClient() - const [open, setOpen] = useState(false) - const [target, setTarget] = useState('PUBLIC') + const [internalOpen, setInternalOpen] = useState(false) + const open = controlledOpen ?? internalOpen + const setOpen = onOpenChange ?? setInternalOpen + const [target, setTarget] = useState(() => scopes.find(scope => scope === visibility) ?? 'PUBLIC') const [error, setError] = useState('') const state = useQuery({ queryKey: ['skills', namespace, slug, 'visibility'], @@ -41,14 +43,14 @@ export function SkillVisibilityDialog({ namespace, slug, visibility, disabled = }) const expanding = scopes.indexOf(target) < scopes.indexOf(scopes.find(scope => scope === currentVisibility) ?? 'PUBLIC') return <> - + }}>{t('skillVisibility.title')}} { if (!mutation.isPending) setOpen(value) }}> - event.stopPropagation()}> + event.stopPropagation()}> {t('skillVisibility.title')} {t('skillVisibility.description')} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 92fca2390..adc647aba 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -906,6 +906,15 @@ "namespaceFilterLabel": "Filter by namespace", "namespaceFilterAll": "All namespaces", "clearSearch": "Clear filters", + "moreActions": "More actions", + "moreActionsFor": "More actions for {{skill}}", + "viewDetail": "View details", + "copyCoordinate": "Copy coordinate", + "coordinateCopied": "Skill coordinate copied", + "coordinateCopyFailed": "Could not copy the coordinate. Try again.", + "compareVersions": "Compare versions", + "downloadCount": "Downloads", + "retry": "Retry", "filterLabel": "Skill filters", "statusFilterLabel": "Filter by status", "resultCount_one": "{{count}} skill", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 234e3c977..82384989d 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -906,6 +906,15 @@ "namespaceFilterLabel": "按命名空间过滤", "namespaceFilterAll": "全部命名空间", "clearSearch": "清除筛选", + "moreActions": "更多操作", + "moreActionsFor": "{{skill}}的更多操作", + "viewDetail": "查看详情", + "copyCoordinate": "复制技能坐标", + "coordinateCopied": "技能坐标已复制", + "coordinateCopyFailed": "复制技能坐标失败，请重试。", + "compareVersions": "版本比较", + "downloadCount": "下载次数", + "retry": "重试", "filterLabel": "技能筛选", "statusFilterLabel": "按状态筛选", "resultCount": "{{count}} 个技能", diff --git a/web/src/pages/dashboard/my-skills-interactions.test.tsx b/web/src/pages/dashboard/my-skills-interactions.test.tsx new file mode 100644 index 000000000..13ba0c59a --- /dev/null +++ b/web/src/pages/dashboard/my-skills-interactions.test.tsx @@ -0,0 +1,192 @@ +/** @vitest-environment jsdom */ +import { StrictMode } from 'react' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const navigate = vi.fn() +const copy = vi.fn() +const retryVersions = vi.fn() +const retryCompare = vi.fn() +const useVersions = vi.fn() +const useCompare = vi.fn() +let skill = makeSkill() +let superAdmin = false +let search: { filter?: string; q?: string; namespace?: string; page?: number } = {} + +function makeSkill() { + return { + id: 1, namespace: 'team', slug: 'demo', displayName: 'Demo', summary: 'Summary', + status: 'ACTIVE', visibility: 'PRIVATE', downloadCount: 3, canSubmitPromotion: false, + managementState: { ownerReadOnly: false }, + publishedVersion: { id: 11, version: '1.1.0', status: 'PUBLISHED' }, + } +} + +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => navigate, + useLocation: () => ({ pathname: '/dashboard/skills', searchStr: '?page=2' }), + useSearch: () => search, +})) +vi.mock('react-i18next', async () => ({ + ...await vi.importActual('react-i18next'), + useTranslation: () => ({ t: (key: string) => key }), +})) +vi.mock('@/features/auth/use-auth', () => ({ useAuth: () => ({ user: { userId: 'reader' }, hasRole: () => superAdmin }) })) +vi.mock('@/shared/hooks/use-user-queries', () => ({ + useMySkills: () => ({ data: { items: [skill], total: 1, size: 10 }, isLoading: false }), + useSubmitPromotion: () => ({ mutateAsync: vi.fn() }), +})) +vi.mock('@/shared/hooks/use-namespace-queries', () => ({ useMyNamespaces: () => ({ data: [] }) })) +vi.mock('@/shared/hooks/use-skill-queries', () => ({ + useArchiveSkill: () => ({ mutateAsync: vi.fn() }), + useUnarchiveSkill: () => ({ mutateAsync: vi.fn() }), + useWithdrawSkillReview: () => ({ mutateAsync: vi.fn() }), + useSkillVersions: (...args: unknown[]) => useVersions(...args), + useSkillVersionCompare: (...args: unknown[]) => useCompare(...args), +})) +vi.mock('@/features/admin/use-admin-skills', () => ({ useRestoreHiddenSkill: () => ({ mutateAsync: vi.fn() }) })) +vi.mock('@/features/skill/skill-avatar-manager', () => ({ SkillAvatarManager: () => null })) +vi.mock('@/features/skill/skill-visibility-dialog', () => ({ SkillVisibilityDialog: () => null })) +vi.mock('@/shared/lib/clipboard', () => ({ copyToClipboard: (...args: unknown[]) => copy(...args) })) +vi.mock('@/shared/lib/toast', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) + +import { MySkillsPage } from './my-skills' + +async function openMenu() { + const trigger = screen.getByRole('button', { name: 'mySkills.moreActionsFor' }) + trigger.focus() + fireEvent.keyDown(trigger, { key: 'ArrowDown' }) + await screen.findByRole('menu') + return trigger +} + +beforeEach(() => { + skill = makeSkill() + search = {} + superAdmin = false + navigate.mockReset() + copy.mockReset() + retryVersions.mockReset() + retryCompare.mockReset() + useVersions.mockReturnValue({ data: [ + { id: 11, version: '1.1.0', status: 'PUBLISHED' }, + { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + { id: 12, version: '1.2.0', status: 'PENDING_REVIEW' }, + ], isLoading: false, refetch: retryVersions }) + useCompare.mockReturnValue({ data: { from: '1.0.0', to: '1.1.0', summary: { totalFiles: 0, addedLines: 0, removedLines: 0 }, files: [] }, isLoading: false, refetch: retryCompare }) + HTMLElement.prototype.scrollIntoView = vi.fn() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => setTimeout(() => callback(0), 0)) + vi.stubGlobal('cancelAnimationFrame', clearTimeout) + vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) +}) +afterEach(() => { cleanup(); vi.restoreAllMocks(); vi.unstubAllGlobals() }) + +describe('My Skills interactions with real menu and dialog', () => { + it('loads comparison on demand and returns focus after Escape without navigating', async () => { + render() + expect(useVersions).not.toHaveBeenCalled() + const trigger = await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.compareVersions' })) + await screen.findByRole('dialog', { name: 'mySkills.compareVersions' }) + expect(useVersions).toHaveBeenCalledWith('team', 'demo') + expect(useCompare).toHaveBeenLastCalledWith('team', 'demo', '1.0.0', '1.1.0', true) + screen.getByRole('button', { name: 'mySkills.update', hidden: true }).focus() + expect(screen.getByRole('dialog').contains(document.activeElement)).toBe(true) + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + expect(document.activeElement).toBe(trigger) + expect(navigate).not.toHaveBeenCalled() + }) + + it('copies the actual skill coordinate without opening details', async () => { + render() + await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.copyCoordinate' })) + expect(copy).toHaveBeenCalledWith('@team/demo') + expect(navigate).not.toHaveBeenCalled() + }) + + it('keeps owner write operations absent after leaving the namespace', async () => { + skill.managementState.ownerReadOnly = true + render() + expect(screen.queryByRole('button', { name: 'mySkills.update' })).toBeNull() + await openMenu() + for (const name of ['mySkills.archive', 'skillVisibility.title', 'mySkills.withdrawReview', 'mySkills.promoteToGlobal']) { + expect(screen.queryByRole('menuitem', { name })).toBeNull() + } + expect(screen.getByRole('menuitem', { name: 'mySkills.viewDetail' })).toBeTruthy() + }) + + it('preserves the separate hidden-governance restore action', async () => { + search = { filter: 'HIDDEN' } + superAdmin = true + render() + await openMenu() + expect(screen.getByRole('menuitem', { name: 'mySkills.restoreHidden' })).toBeTruthy() + expect(screen.queryByRole('menuitem', { name: 'mySkills.viewDetail' })).toBeNull() + expect(screen.queryByRole('menuitem', { name: 'mySkills.archive' })).toBeNull() + expect(screen.queryByRole('button', { name: 'mySkills.update' })).toBeNull() + }) + + it('only navigates to the existing upload-update flow when Update is clicked', () => { + render() + fireEvent.click(screen.getByRole('button', { name: 'mySkills.update' })) + expect(navigate).toHaveBeenCalledTimes(1) + expect(navigate).toHaveBeenCalledWith({ to: '/dashboard/publish', search: { namespace: 'team', updateSkill: 'demo', updateName: 'Demo', visibility: 'PRIVATE' } }) + }) + + it('restores scroll after detail navigation under the same URL and identity', async () => { + Object.defineProperty(window, 'scrollY', { configurable: true, value: 450 }) + search = { filter: 'PUBLISHED', page: 2 } + const first = render() + fireEvent.click(screen.getByRole('link', { name: 'Demo' })) + expect(navigate.mock.calls[0][0].search.returnTo).toBe('/dashboard/skills?page=2') + first.unmount() + render() + await waitFor(() => expect(window.scrollTo).toHaveBeenCalledWith({ top: 450, behavior: 'instant' })) + }) + + it('shows a retryable version-load failure instead of a false not-enough-versions result', async () => { + useVersions.mockReturnValue({ isLoading: false, error: new Error('offline'), refetch: retryVersions }) + render() + await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.compareVersions' })) + await screen.findByRole('alert') + expect(screen.queryByText('skillCompare.notEnoughPublishedVersions')).toBeNull() + fireEvent.click(screen.getByRole('button', { name: 'mySkills.retry' })) + expect(retryVersions).toHaveBeenCalledOnce() + }) + + it('does not compare when fewer than two published versions exist', async () => { + useVersions.mockReturnValue({ data: [ + { id: 1, version: '1.0.0', status: 'PUBLISHED' }, + { id: 2, version: '2.0.0', status: 'PENDING_REVIEW' }, + ], isLoading: false, refetch: retryVersions }) + render() + await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.compareVersions' })) + await screen.findByText('skillCompare.notEnoughPublishedVersions') + expect(useCompare).toHaveBeenLastCalledWith('team', 'demo', '', '', false) + }) + + it('keeps comparison errors retryable without closing or navigating', async () => { + useCompare.mockReturnValue({ isLoading: false, error: new Error('offline'), refetch: retryCompare }) + render() + await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.compareVersions' })) + await screen.findByRole('alert') + fireEvent.click(screen.getByRole('button', { name: 'mySkills.retry' })) + expect(retryCompare).toHaveBeenCalledOnce() + expect(screen.getByRole('dialog')).toBeTruthy() + expect(navigate).not.toHaveBeenCalled() + }) + + it('preserves the loading and empty-files outcomes in the comparison dialog', async () => { + useVersions.mockReturnValueOnce({ isLoading: true }) + render() + await openMenu() + fireEvent.click(screen.getByRole('menuitem', { name: 'mySkills.compareVersions' })) + await screen.findByText('skillCompare.loading') + }) + +}) diff --git a/web/src/pages/dashboard/my-skills-scroll.test.ts b/web/src/pages/dashboard/my-skills-scroll.test.ts new file mode 100644 index 000000000..e2a5141fa --- /dev/null +++ b/web/src/pages/dashboard/my-skills-scroll.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' +import { rememberMySkillsScroll, takeMySkillsScroll } from './my-skills-scroll' + +describe('My Skills scroll context', () => { + it('restores only the matching identity and filters once', () => { + rememberMySkillsScroll('user-a:page-2', 540) + expect(takeMySkillsScroll('user-b:page-2')).toBeUndefined() + expect(takeMySkillsScroll('user-a:page-1')).toBeUndefined() + expect(takeMySkillsScroll('user-a:page-2')).toBe(540) + expect(takeMySkillsScroll('user-a:page-2')).toBeUndefined() + }) + + it('keeps only the latest departure context', () => { + rememberMySkillsScroll('user-a:page-1', 100) + rememberMySkillsScroll('user-a:page-2', 600) + expect(takeMySkillsScroll('user-a:page-1')).toBeUndefined() + expect(takeMySkillsScroll('user-a:page-2')).toBe(600) + }) +}) diff --git a/web/src/pages/dashboard/my-skills-scroll.ts b/web/src/pages/dashboard/my-skills-scroll.ts new file mode 100644 index 000000000..2fa1a186c --- /dev/null +++ b/web/src/pages/dashboard/my-skills-scroll.ts @@ -0,0 +1,16 @@ +let savedPosition: { key: string; top: number } | undefined + +export function rememberMySkillsScroll(key: string, top: number) { + savedPosition = { key, top } +} + +export function readMySkillsScroll(key: string): number | undefined { + return savedPosition?.key === key ? savedPosition.top : undefined +} + +export function takeMySkillsScroll(key: string): number | undefined { + if (savedPosition?.key !== key) return undefined + const top = savedPosition.top + savedPosition = undefined + return top +} diff --git a/web/src/pages/dashboard/my-skills.test.ts b/web/src/pages/dashboard/my-skills.test.ts index 34223074f..abef009e0 100644 --- a/web/src/pages/dashboard/my-skills.test.ts +++ b/web/src/pages/dashboard/my-skills.test.ts @@ -1,10 +1,16 @@ -import { createElement, type ReactNode } from 'react' +import { createElement, isValidElement, type ReactNode } from 'react' import { renderToStaticMarkup } from 'react-dom/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import { createInstance, type i18n } from 'i18next' import zh from '@/i18n/locales/zh.json' import en from '@/i18n/locales/en.json' +function textContent(node: ReactNode): string { + if (Array.isArray(node)) return node.map(textContent).join('') + if (isValidElement<{ children?: ReactNode }>(node)) return textContent(node.props.children) + return typeof node === 'string' || typeof node === 'number' ? String(node) : '' +} + const navigateMock = vi.fn() const buttonRecords: Array<{ label: string; onClick?: ((event?: { stopPropagation: () => void }) => void) | undefined }> = [] const useMySkillsMock = vi.fn() @@ -42,12 +48,23 @@ vi.mock('@/shared/ui/button', () => ({ children?: ReactNode onClick?: (event?: { stopPropagation: () => void }) => void }) => { - const label = Array.isArray(children) ? children.join('') : String(children ?? '') + const label = textContent(children) buttonRecords.push({ label, onClick }) return createElement('button', null, children) }, })) +vi.mock('@/shared/ui/dropdown-menu', () => ({ + DropdownMenu: ({ children }: { children: ReactNode }) => children, + DropdownMenuTrigger: ({ children }: { children: ReactNode }) => children, + DropdownMenuContent: ({ children }: { children: ReactNode }) => children, + DropdownMenuSeparator: () => null, + DropdownMenuItem: ({ children, onSelect }: { children: ReactNode; onSelect?: () => void }) => { + buttonRecords.push({ label: textContent(children), onClick: onSelect }) + return createElement('button', null, children) + }, +})) + vi.mock('@/shared/ui/card', () => ({ Card: ({ children }: { children: ReactNode }) => createElement('div', null, children), })) @@ -229,7 +246,7 @@ describe('MySkillsPage', () => { }) const markup = renderToStaticMarkup(createElement(MySkillsPage)) - expect(markup).toMatch(/]*>示例技能<\/h3>/) + expect(markup).toMatch(/]*>]*>示例技能<\/a><\/h3>/) }) it('navigates to publish page with namespace and visibility when update is clicked', () => { diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 397fce058..ba9704ac6 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -1,8 +1,13 @@ -import { useCallback, useEffect, useState } from 'react' -import { Search } from 'lucide-react' +import { useCallback, useEffect, useRef, useState } from 'react' +import { Archive, ArrowUpRight, Copy, Download, Eye, GitCompareArrows, Globe, MoreHorizontal, RotateCcw, Search, Undo2, Upload } from 'lucide-react' import { useLocation, useNavigate, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' +import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/shared/ui/dialog' +import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuSeparator, DropdownMenuTrigger } from '@/shared/ui/dropdown-menu' +import { copyToClipboard } from '@/shared/lib/clipboard' +import { SkillVersionCompareContent } from '@/features/skill/skill-version-compare-content' +import { rememberMySkillsScroll, readMySkillsScroll, takeMySkillsScroll } from './my-skills-scroll' import { Button } from '@/shared/ui/button' import { Card } from '@/shared/ui/card' import { Input } from '@/shared/ui/input' @@ -49,7 +54,7 @@ export function MySkillsPage() { const location = useLocation() const search = useSearch({ from: '/dashboard/skills' }) const { t } = useTranslation() - const { hasRole } = useAuth() + const { hasRole, user } = useAuth() // The URL is the source of truth for page / filter / namespace / keyword so the // search context survives navigating into a skill and back via the returnTo link. @@ -61,6 +66,11 @@ export function MySkillsPage() { // Input remains local until the user explicitly submits the search. const [keywordInput, setKeywordInput] = useState(keyword) + const [compareTarget, setCompareTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) + const [compareVersions, setCompareVersions] = useState({ from: '', to: '' }) + const compareTrigger = useRef(null) + const [visibilityTarget, setVisibilityTarget] = useState<{ namespace: string; slug: string; visibility: string } | null>(null) + const scrollKey = JSON.stringify([user?.userId, keyword, namespaceFilter, filter, page]) 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) @@ -102,7 +112,19 @@ export function MySkillsPage() { const submitPromotionMutation = useSubmitPromotion() const restoreHiddenMutation = useRestoreHiddenSkill() + useEffect(() => { + if (isLoading) return + const top = readMySkillsScroll(scrollKey) + if (top === undefined) return + const frame = requestAnimationFrame(() => { + takeMySkillsScroll(scrollKey) + window.scrollTo({ top, behavior: 'instant' }) + }) + return () => cancelAnimationFrame(frame) + }, [isLoading, scrollKey]) + const handleSkillClick = (namespace: string, slug: string, version?: string) => { + rememberMySkillsScroll(scrollKey, window.scrollY) navigate({ to: `/space/${namespace}/${encodeURIComponent(slug)}`, search: { returnTo: buildReturnTo(location), version }, @@ -380,186 +402,123 @@ export function MySkillsPage() { {skillPage && skillPage.total > 0 ? ( <> -
- {skills.map((skill, idx) => ( - (() => { - const ownerReadOnly = skill.managementState?.ownerReadOnly === true - const relatedNamespace = skill.managementState?.relatedNamespace - const relatedSlug = skill.managementState?.relatedSlug - const headlineVersion = getHeadlineVersion(skill) - const publishedVersion = getPublishedVersion(skill) - const ownerPreviewVersion = getOwnerPreviewVersion(skill) - const hasPendingPreview = hasPendingOwnerPreview(skill) - const preview = ownerPreviewVersion && ownerPreviewVersion.id !== publishedVersion?.id && ownerPreviewVersion.status !== 'YANKED' - ? ownerPreviewVersion : null - const displayName = preview?.displayName ?? skill.displayName - const summary = preview?.summary ?? skill.summary - const mainStatus = filter === 'HIDDEN' ? 'HIDDEN' : skill.status === 'ARCHIVED' || skill.status === 'HIDDEN' - ? skill.status : preview?.status ?? headlineVersion?.status ?? skill.status +
+ {skills.map((skill) => { + const ownerReadOnly = skill.managementState?.ownerReadOnly === true + const hidden = filter === 'HIDDEN' + const relatedNamespace = skill.managementState?.relatedNamespace + const relatedSlug = skill.managementState?.relatedSlug + const headlineVersion = getHeadlineVersion(skill) + const publishedVersion = getPublishedVersion(skill) + const ownerPreviewVersion = getOwnerPreviewVersion(skill) + const hasPendingPreview = hasPendingOwnerPreview(skill) + const preview = ownerPreviewVersion && ownerPreviewVersion.id !== publishedVersion?.id && ownerPreviewVersion.status !== 'YANKED' + ? ownerPreviewVersion : null + const displayName = preview?.displayName ?? skill.displayName + const summary = preview?.summary ?? skill.summary + const mainStatus = hidden ? 'HIDDEN' : skill.status === 'ARCHIVED' || skill.status === 'HIDDEN' + ? skill.status : preview?.status ?? headlineVersion?.status ?? skill.status + const writable = !hidden && !ownerReadOnly + const canChangeVisibility = writable && skill.status === 'ACTIVE' && !!publishedVersion + const coordinate = `@${skill.namespace}/${skill.slug}` + const openDetail = () => handleSkillClick(skill.namespace, skill.slug, preview?.version) + const detailSearch = new URLSearchParams({ returnTo: buildReturnTo(location) }) + if (preview?.version) detailSearch.set('version', preview.version) - - return ( - handleSkillClick(skill.namespace, skill.slug, preview?.version)} - > -
-
-
- -

- {displayName} + return ( + +
+ +
+
+
+

+ {hidden ? displayName : { + event.stopPropagation() + if (event.button !== 0 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return + event.preventDefault() + openDetail() + }}>{displayName}}

+ {mainStatus && + {hasPendingPreview && publishedVersion && mainStatus === 'PENDING_REVIEW' + ? t('mySkills.statusUpdatePendingReview') : resolveStatusLabel(mainStatus)} + }
- {ownerReadOnly &&

{t('mySkills.ownerReadOnly')}

} - {skill.managementState?.promotionRole && ( -
- {t(`mySkills.promotionRoles.${skill.managementState.promotionRole}`)} - {relatedNamespace && relatedSlug && ( - - )} -
- )} - {skill.status === 'ACTIVE' && filter !== 'HIDDEN' && } - {summary && ( -

{summary}

- )} -
- @{skill.namespace} - {t('skillVisibility.current', { visibility: t(`skillVisibility.${skill.visibility ?? 'PUBLIC'}`) })} - {preview && publishedVersion ? ( - {t('mySkills.versionDifference', { version: preview.version, published: publishedVersion.version })} - ) : headlineVersion ? ( - v{headlineVersion.version} - ) : null} - - - - - {formatCompactCount(skill.downloadCount)} - - {mainStatus && ( - - {hasPendingPreview && publishedVersion && mainStatus === 'PENDING_REVIEW' - ? t('mySkills.statusUpdatePendingReview') : resolveStatusLabel(mainStatus)} - - )} -
- {skill.managementState?.requestedVisibility && ( -

- {t('skillVisibility.pendingList', { visibility: t(`skillVisibility.${skill.managementState.requestedVisibility}`) })} -

- )} -
-
- {canRestoreHidden ? ( - - ) : filter === 'HIDDEN' || ownerReadOnly ? null : skill.status !== 'ARCHIVED' && ( - - )} - {!ownerReadOnly && filter !== 'HIDDEN' && skill.status === 'ACTIVE' && publishedVersion && ( - - )} - {filter === 'HIDDEN' || ownerReadOnly ? null : hasPendingPreview && ownerPreviewVersion ? ( - - ) : skill.canSubmitPromotion && publishedVersion ? ( - - ) : skill.status === 'ARCHIVED' ? ( - - ) : publishedVersion ? ( - - ) : null} - {filter === 'HIDDEN' ? null : ( - - - - )} + }}>
+ {summary &&

{summary}

} +
+ {coordinate} + {t('skillVisibility.current', { visibility: t(`skillVisibility.${skill.visibility ?? 'PUBLIC'}`) })} + {preview && publishedVersion ? {t('mySkills.versionDifference', { version: preview.version, published: publishedVersion.version })} + : headlineVersion ? v{headlineVersion.version} : null} + +
+ {ownerReadOnly &&

{t('mySkills.ownerReadOnly')}

} + {skill.managementState?.promotionRole &&

{t(`mySkills.promotionRoles.${skill.managementState.promotionRole}`)}

} + {skill.status === 'ACTIVE' && !hidden && } + {skill.managementState?.requestedVisibility &&

+ {t('skillVisibility.pendingList', { visibility: t(`skillVisibility.${skill.managementState.requestedVisibility}`) })} +

}
- - ) - })() - ))} +

+ + ) + })}
{skillPage.total > PAGE_SIZE ? ( @@ -580,6 +539,20 @@ export function MySkillsPage() { /> )} + {visibilityTarget && { if (!open) setVisibilityTarget(null) }} + onCloseAutoFocus={event => { event.preventDefault(); compareTrigger.current?.focus() }} />} + { if (!open) setCompareTarget(null) }}> + { event.preventDefault(); compareTrigger.current?.focus() }}> + + {t('mySkills.compareVersions')} + {compareTarget?.name} + + {compareTarget && } + + + { diff --git a/web/src/pages/skill-version-compare.test.tsx b/web/src/pages/skill-version-compare.test.tsx index 6535d2530..1f58fc1e0 100644 --- a/web/src/pages/skill-version-compare.test.tsx +++ b/web/src/pages/skill-version-compare.test.tsx @@ -34,6 +34,7 @@ import { SkillVersionComparePage } from './skill-version-compare' describe('SkillVersionComparePage', () => { beforeEach(() => { navigateMock.mockReset() + HTMLElement.prototype.scrollIntoView = vi.fn() useSkillVersionsMock.mockReturnValue({ data: [ diff --git a/web/src/pages/skill-version-compare.tsx b/web/src/pages/skill-version-compare.tsx index ef721dd57..c90786d2c 100644 --- a/web/src/pages/skill-version-compare.tsx +++ b/web/src/pages/skill-version-compare.tsx @@ -1,292 +1,13 @@ -import { useEffect, useMemo, useState } from 'react' +import { useCallback } from 'react' import { useNavigate, useParams, useSearch } from '@tanstack/react-router' -import { useTranslation } from 'react-i18next' -import { useSkillVersionCompare, useSkillVersions } from '@/shared/hooks/use-skill-queries' -import { Input } from '@/shared/ui/input' -import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select' - -function toDiffSectionId(path: string) { - return `diff-${encodeURIComponent(path)}` -} - -function getChangeTypeLabel(t: (key: string) => string, changeType: string) { - if (changeType === 'ADDED') { - return t('skillCompare.changeTypeAdded') - } - if (changeType === 'REMOVED') { - return t('skillCompare.changeTypeRemoved') - } - return t('skillCompare.changeTypeModified') -} +import { SkillVersionCompareContent } from '@/features/skill/skill-version-compare-content' export function SkillVersionComparePage() { - const { t } = useTranslation() const navigate = useNavigate() const { namespace, slug } = useParams({ from: '/space/$namespace/$slug/compare' }) - const { from, to } = useSearch({ from: '/space/$namespace/$slug/compare' }) as { from: string; to: string } - const [keyword, setKeyword] = useState('') - const [activePath, setActivePath] = useState(null) - const { data: versions, isLoading: isVersionsLoading } = useSkillVersions(namespace, slug) - - const publishedVersions = useMemo( - () => (versions ?? []).filter((version) => version.status === 'PUBLISHED'), - [versions] - ) - - const hasEnoughVersions = publishedVersions.length >= 2 - const defaultFrom = publishedVersions[1]?.version ?? publishedVersions[0]?.version ?? '' - const defaultTo = publishedVersions[0]?.version ?? '' - - const normalizedVersions = useMemo(() => { - if (!hasEnoughVersions) { - return { from: '', to: '' } - } - - const isPublishedVersion = (value: string) => publishedVersions.some((version) => version.version === value) - let nextFrom = from && isPublishedVersion(from) ? from : defaultFrom - let nextTo = to && isPublishedVersion(to) ? to : defaultTo - - if (nextFrom === nextTo) { - const alternative = publishedVersions.find((version) => version.version !== nextFrom)?.version ?? '' - if (!from && !to) { - nextFrom = defaultFrom - nextTo = defaultTo - } else if (from && !to) { - nextTo = alternative - } else if (!from && to) { - nextFrom = alternative - } else { - nextTo = alternative - } - } - - return { from: nextFrom, to: nextTo } - }, [defaultFrom, defaultTo, from, hasEnoughVersions, publishedVersions, to]) - - useEffect(() => { - if (!hasEnoughVersions || !normalizedVersions.from || !normalizedVersions.to) { - return - } - if (from === normalizedVersions.from && to === normalizedVersions.to) { - return - } - navigate({ - to: '/space/$namespace/$slug/compare', - params: { namespace, slug }, - search: { from: normalizedVersions.from, to: normalizedVersions.to }, - replace: true, - }) - }, [from, hasEnoughVersions, namespace, navigate, normalizedVersions.from, normalizedVersions.to, slug, to]) - - const { data, isLoading: isCompareLoading, error } = useSkillVersionCompare( - namespace, - slug, - normalizedVersions.from, - normalizedVersions.to, - hasEnoughVersions && !!normalizedVersions.from && !!normalizedVersions.to - ) - - const filteredFiles = useMemo(() => { - const files = data?.files ?? [] - if (!keyword.trim()) { - return files - } - return files.filter((file) => file.path.toLowerCase().includes(keyword.trim().toLowerCase())) - }, [data?.files, keyword]) - - useEffect(() => { - if (!activePath && filteredFiles.length > 0) { - setActivePath(filteredFiles[0].path) - return - } - if (activePath && !filteredFiles.some((file) => file.path === activePath)) { - setActivePath(filteredFiles[0]?.path ?? null) - } - }, [activePath, filteredFiles]) - - if (isVersionsLoading) { - return
{t('skillCompare.loading')}
- } - - if (!hasEnoughVersions) { - return
{t('skillCompare.notEnoughPublishedVersions')}
- } - - if (isCompareLoading) { - return
{t('skillCompare.loading')}
- } - - if (error || !data) { - return
{t('skillCompare.errorLoadingCompare')}
- } - - return ( -
- - -
- {filteredFiles.length > 0 ? ( - filteredFiles.map((file) => ( -
-
-
{file.path}
-
{getChangeTypeLabel(t, file.changeType)}
-
- - {file.binary ? ( -
{t('skillCompare.binaryFile')}
- ) : file.truncated ? ( -
{t('skillCompare.truncatedFile')}
- ) : ( -
- {file.hunks.flatMap((hunk) => hunk.lines).map((line, index) => ( -
- - {line.oldLineNumber ?? ''} - - - {line.newLineNumber ?? ''} - - - {line.type === 'ADD' ? '+' : line.type === 'DELETE' ? '-' : ' '} - {line.content} - -
- ))} -
- )} -
- )) - ) : ( -
- {t('skillCompare.noFilesFound')} -
- )} -
-
- ) + const { from, to } = useSearch({ from: '/space/$namespace/$slug/compare' }) + const onVersionsChange = useCallback((versions: { from: string; to: string }, replace = false) => { + navigate({ to: '/space/$namespace/$slug/compare', params: { namespace, slug }, search: versions, replace }) + }, [navigate, namespace, slug]) + return }