diff --git a/docs/skillhub/en/guide/review.md b/docs/skillhub/en/guide/review.md index 978f9c55e..55349843e 100644 --- a/docs/skillhub/en/guide/review.md +++ b/docs/skillhub/en/guide/review.md @@ -51,6 +51,8 @@ Online preview limits are listed together below. Exceeding a limit does not mean These are frontend/API implementation limits, not review-page settings. Sources: [review detail page](https://github.com/iflytek/skillhub/blob/main/web/src/pages/dashboard/review-detail.tsx) and [review API](https://github.com/iflytek/skillhub/blob/main/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java). `GET /api/v1/reviews/{id}/skill-detail?includeDocumentation=false` reads metadata, custom web/package presentation and the documentation path without fetching object-storage content; README or SKILL.md fallback content is loaded on demand; the default is `true`. +Switching content tabs or opening evidence from the side panel aligns the content container below the header. Query completion and comment edits do not reset the reading position. Content uses the main page scroll rather than a nested scrolling region. + ### Understand Changes Ordinary updates compare the actual published baseline on the left with the pending version on the right: effective display name, summary and overview, plus added, modified and removed files. Initial publication has no published baseline; the page states this instead of inventing an empty-version comparison. GitHub updates retain fixed source revisions and line-by-line file comparison. Avatar and visibility requests compare their own candidate or target rather than content versions. diff --git a/docs/skillhub/guide/review.md b/docs/skillhub/guide/review.md index 95eeb7bc8..3bd0f4089 100644 --- a/docs/skillhub/guide/review.md +++ b/docs/skillhub/guide/review.md @@ -51,6 +51,8 @@ 这些是前端/接口实现限制，不由审核页面配置；依据[审核详情](https://github.com/iflytek/skillhub/blob/main/web/src/pages/dashboard/review-detail.tsx)及[审核接口](https://github.com/iflytek/skillhub/blob/main/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java)。`GET /api/v1/reviews/{id}/skill-detail?includeDocumentation=false` 读取元数据、网页或包内自定义展示内容及说明路径，不读取对象存储正文；README 或 SKILL.md 回退正文需按需读取；缺省为 `true`。 +切换内容页签或点击右侧核查入口时，页面定位至内容容器顶部；异步加载及填写审核意见不重复改变阅读位置。正文随主页面滚动，不另设内部滚动区域。 + ### 理解变更对比 普通技能更新的“变更对比”以真实已发布基线为左侧，以本次待审版本为右侧，比较有效展示名称、简要描述、详细概述，并列出新增、修改和删除的文件。首次发布没有已发布基线，页面明确说明，不生成虚假的空版本差异。GitHub 更新保留固定来源修订及逐行文件比较；头像和可见范围申请比较各自候选或目标，不生成内容版本差异。 diff --git a/openspec/changes/unify-review-workbench/design.md b/openspec/changes/unify-review-workbench/design.md index c077639c8..ad041595c 100644 --- a/openspec/changes/unify-review-workbench/design.md +++ b/openspec/changes/unify-review-workbench/design.md @@ -22,6 +22,12 @@ 详情顶部仅任务标题与事项，主区有效简介/概述及原文件，右侧标准Card集中摘要和证据。按任务决定Tab，包内容加载失败不能用已发布旧内容冒充；扫描、头像、许可证和并发决定仍遵守原服务条件。已有任务已处理后只读。 +### 人工预览后的内容容器与定位修订 + +审核详情的Tab导航与当前内容合入同一有边界的内容容器，内部保留阅读间距；主页面统一滚动，不增加正文内部滚动条。短内容使用与视口相关的最小高度，保证导航能在固定页头下方对齐，不复制上一长面板的整段空白高度。 + +用户切换Tab或点击右侧证据入口后，在新面板布局提交时立即定位到容器顶部；重复点击当前证据入口同样定位，并将焦点移至所选页签；普通页签导航保留自身焦点。异步查询完成、首次渲染及审核意见输入不重复触发定位。减少动态偏好下关闭内容淡入，不以长距离平滑滚动延迟内容可用。 + ## 取舍 只做样式覆盖会保留重复入口与缺失待审内容，因此同时收敛读模型。前端聚合各命名空间分页会导致排序与计数不正确，因此采用服务端授权过滤。新增完整通用工作流引擎或普通zip逐行diff均超出当前需求。 diff --git a/openspec/changes/unify-review-workbench/specs/review-workbench/spec.md b/openspec/changes/unify-review-workbench/specs/review-workbench/spec.md index 9909c0ef3..59a5f1bb2 100644 --- a/openspec/changes/unify-review-workbench/specs/review-workbench/spec.md +++ b/openspec/changes/unify-review-workbench/specs/review-workbench/spec.md @@ -57,3 +57,12 @@ The confirmed dual-column detail MUST put readable target content in the main co - **WHEN** its task is viewed - **THEN** the previous and requested scopes and bound version are clear - **AND** its bound scan evidence is not contradicted by an unrelated generic no-scan message + + +### Requirement: Stable detail navigation and content boundaries +The review detail MUST group the navigation and active content in one bounded surface without a nested page scrollbar. A user-initiated tab switch or evidence shortcut MUST align that surface below the fixed header after the new panel commits, including selecting the current evidence tab again. Query completion, initial rendering and comment edits MUST NOT trigger this navigation scroll. + +#### Scenario: A long comparison is replaced with a short history panel +- **WHEN** the reviewer switches from a scrolled long comparison to history or opens scan evidence from the side panel +- **THEN** the selected panel starts below the fixed header rather than at the document bottom +- **AND** later query completion does not reset the reviewer's reading position diff --git a/web/src/pages/dashboard/review-detail.test.tsx b/web/src/pages/dashboard/review-detail.test.tsx index e6d79f0fb..dd3e18aaa 100644 --- a/web/src/pages/dashboard/review-detail.test.tsx +++ b/web/src/pages/dashboard/review-detail.test.tsx @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ReviewSkillDetail } from '@/api/types' const navigateMock = vi.fn() +const scrollIntoViewMock = vi.fn() const route = vi.hoisted(() => ({ id: '13', search: {} as Record })) vi.mock('@tanstack/react-router', () => ({ @@ -171,6 +172,8 @@ import { NamespaceReviewDetailPage, ReviewDetailPage } from './review-detail' describe('ReviewDetailPage', () => { beforeEach(() => { + scrollIntoViewMock.mockReset() + HTMLElement.prototype.scrollIntoView = scrollIntoViewMock route.id = '13' route.search = {} navigateMock.mockReset() @@ -625,4 +628,83 @@ describe('ReviewDetailPage', () => { expect(query.get('page')).toBe('2') }) + it('wraps navigation and panels in a single responsive surface with a header scroll offset', () => { + render() + const navigation = screen.getByRole('tablist') + const surface = screen.getByTestId('review-detail-workbench') + expect(navigation.parentElement?.parentElement).toBe(surface) + expect(surface?.contains(screen.getByRole('tabpanel'))).toBe(true) + expect(surface?.className).toContain('scroll-mt-24') + expect(surface?.className).toContain('min-h-[calc(100svh-6rem)]') + expect(surface?.className).toContain('border-border/60') + expect(surface?.className).not.toContain('overflow-auto') + expect(screen.getByRole('tabpanel').className).toContain('motion-reduce:animate-none') + }) + + it('scrolls after long content is replaced and not on data or comment updates', () => { + const snapshot = useReviewSkillDetailMock() as { data: ReviewSkillDetail } + useReviewSkillDetailMock.mockReturnValue({ ...snapshot, data: { ...snapshot.data, + presentation: { displayName: 'Long pending content', summary: 'Pending summary', overview: 'Long review paragraph.\n\n'.repeat(80), overviewSource: 'WEB' }, + } }) + const view = render() + expect(scrollIntoViewMock).not.toHaveBeenCalled() + let panelAtScroll = '' + scrollIntoViewMock.mockImplementation(() => { panelAtScroll = screen.getByRole('tabpanel').textContent ?? '' }) + fireEvent.click(screen.getByRole('tab', { name: 'review.tabHistory' })) + expect(panelAtScroll).toContain('review.attemptHistory') + expect(panelAtScroll).not.toContain('Long review paragraph.') + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1) + expect(scrollIntoViewMock).toHaveBeenCalledWith({ block: 'start', behavior: 'instant' }) + view.rerender() + fireEvent.change(screen.getByLabelText('review.commentLabel'), { target: { value: 'A review comment' } }) + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1) + }) + + it('moves focus from a shortcut to the selected tab before one scroll, including repeated clicks', () => { + const view = render() + const shortcut = screen.getByRole('button', { name: /review.scanSummary/ }) + shortcut.focus() + fireEvent.click(shortcut) + const sourceTab = screen.getByRole('tab', { name: 'review.tabSourceSecurity' }) + expect(sourceTab.getAttribute('aria-selected')).toBe('true') + expect(document.activeElement).toBe(sourceTab) + const focus = vi.spyOn(sourceTab, 'focus') + shortcut.focus() + fireEvent.click(shortcut) + expect(document.activeElement).toBe(sourceTab) + expect(focus).toHaveBeenCalledWith({ preventScroll: true }) + focus.mockRestore() + expect(scrollIntoViewMock).toHaveBeenCalledTimes(2) + const target = screen.getByRole('tablist').parentElement?.parentElement + expect(scrollIntoViewMock.mock.contexts).toEqual([target, target]) + const comment = screen.getByLabelText('review.commentLabel') + comment.focus() + fireEvent.change(comment, { target: { value: 'Continue reviewing' } }) + view.rerender() + expect(document.activeElement).toBe(comment) + expect(scrollIntoViewMock).toHaveBeenCalledTimes(2) + }) + + it('restores the content top when keyboard navigation changes the panel', () => { + render() + const content = screen.getByRole('tab', { name: 'review.tabContent' }) + content.focus() + fireEvent.keyDown(content, { key: 'End' }) + const history = screen.getByRole('tab', { name: 'review.tabHistory' }) + expect(history.getAttribute('aria-selected')).toBe('true') + expect(document.activeElement).toBe(history) + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1) + }) + + it('moves the license shortcut focus to the source tab', () => { + const task = useReviewDetailMock() as { data: Record } + useReviewDetailMock.mockReturnValue({ ...task, data: { ...task.data, originType: 'GITHUB_SYNC' } }) + render() + const shortcut = screen.getByRole('button', { name: /review.githubLicenseStatus/ }) + shortcut.focus() + fireEvent.click(shortcut) + expect(document.activeElement).toBe(screen.getByRole('tab', { name: 'review.tabSourceSecurity' })) + expect(scrollIntoViewMock).toHaveBeenCalledTimes(1) + }) + }) diff --git a/web/src/pages/dashboard/review-detail.tsx b/web/src/pages/dashboard/review-detail.tsx index 3ae37bd29..bf752e638 100644 --- a/web/src/pages/dashboard/review-detail.tsx +++ b/web/src/pages/dashboard/review-detail.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useEffect, useLayoutEffect, useRef, useState } from 'react' import { useNavigate, useParams, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { buildReviewReturnPath } from '@/features/review/review-route-search' @@ -59,6 +59,24 @@ function ReviewDetailScreen({ const { data: githubSource, isLoading: isLoadingGithubSource, isError: isGithubSourceError } = useGithubSyncReviewSource(taskId, isGithubSyncReview) const [selectedTab, setSelectedTab] = useState(null) + const tabsRef = useRef(null) + const [tabNavigation, setTabNavigation] = useState(0) + const focusTabAfterNavigation = useRef(false) + const selectTab = (tab: string, fromShortcut = false) => { + focusTabAfterNavigation.current = fromShortcut + setSelectedTab(tab) + setTabNavigation((request) => request + 1) + } + // Scroll after the new panel commits, before paint: a shorter panel can clamp + // the old document scroll position. Query completion must never repeat this. + useLayoutEffect(() => { + if (tabNavigation === 0) return + if (focusTabAfterNavigation.current) { + tabsRef.current?.querySelector('[role="tab"][aria-selected="true"]')?.focus({ preventScroll: true }) + focusTabAfterNavigation.current = false + } + tabsRef.current?.scrollIntoView({ block: 'start', behavior: 'instant' }) + }, [tabNavigation]) const activeTab = selectedTab ?? (isGithubSyncReview && githubSource?.baseVersionId ? 'changes' : 'content') const { data: reviewAttempts, @@ -216,14 +234,17 @@ function ReviewDetailScreen({
- + +
{t('review.tabContent')} {hasBaseline && {t('review.tabChanges')}} {!isSuiteReview && !isMetadataReview && {t('review.tabSourceSecurity')}} {t('review.tabHistory')} - +
+
+ {reviewSkillDetail?.avatarChange && } {isSuiteReview && ( @@ -275,7 +296,7 @@ function ReviewDetailScreen({ onRetry={() => { void refetchReviewSkillDetail?.(); void refetchDocumentation?.() }} /> )} - + {isGithubSyncReview && ( isLoadingGithubSource ? : githubSource ? @@ -292,7 +313,7 @@ function ReviewDetailScreen({ hasError={Boolean(comparisonQuery.error)} onRetry={() => void comparisonQuery.refetch?.()} /> )} - {!isSuiteReview && !isMetadataReview && + {!isSuiteReview && !isMetadataReview && {review.requestedVisibility ?

{t('reviewWorkbench.boundScan')}

{reviewSkillDetail?.visibilityChange ? t(`skillVisibility.scan${reviewSkillDetail.visibilityChange.scanStatus}`) : t('review.scanLoadFailed')}

{t('reviewWorkbench.boundScanDescription')}

 : skillId && versionId ? @@ -301,13 +322,13 @@ function ReviewDetailScreen({ : githubSource ? : {t('review.githubSourceError')})} } - - + +

{t('review.attemptHistory')}

{isLoadingReviewAttempts ?
: isReviewAttemptsError ?

{t('review.attemptHistoryError')}

: } - +
{(review.reviewedBy || review.reviewComment) && (

{t('review.reviewResult')}

@@ -317,13 +338,14 @@ function ReviewDetailScreen({ )} +
{!isSuiteReview && !isMetadataReview &&

{t('reviewWorkbench.checks')}

- - {isGithubSyncReview && } + {isGithubSyncReview && }

{t('reviewWorkbench.scanBoundary')}

}