fix(review): improve presentation hierarchy and comparison reading

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-11-05 17:46:12 +08:00
parent 4b33cf91fb
commit 7303814452
11 changed files with 338 additions and 162 deletions

View file

@ -39,8 +39,8 @@
{"name":"dashboardReviewsRoute","path":"dashboard/reviews","declaration":{"file":"web/src/app/router.tsx","line":598},"implementation":{"file":"web/src/pages/dashboard/reviews.tsx","line":31,"export":"ReviewsPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardReviewProgressRoute","path":"dashboard/review-progress","declaration":{"file":"web/src/app/router.tsx","line":606},"implementation":{"file":"web/src/pages/dashboard/review-progress.tsx","line":27,"export":"ReviewProgressPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardReportsRoute","path":"dashboard/reports","declaration":{"file":"web/src/app/router.tsx","line":628},"implementation":{"file":"web/src/pages/dashboard/reports.tsx","line":31,"export":"ReportsPage"},"guarded":true,"roleProtected":true},
{"name":"dashboardReviewDetailRoute","path":"dashboard/reviews/$id","declaration":{"file":"web/src/app/router.tsx","line":635},"implementation":{"file":"web/src/pages/dashboard/review-detail.tsx","line":452,"export":"ReviewDetailPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardNamespaceReviewDetailRoute","path":"dashboard/namespaces/$slug/reviews/$id","declaration":{"file":"web/src/app/router.tsx","line":643},"implementation":{"file":"web/src/pages/dashboard/review-detail.tsx","line":465,"export":"NamespaceReviewDetailPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardReviewDetailRoute","path":"dashboard/reviews/$id","declaration":{"file":"web/src/app/router.tsx","line":635},"implementation":{"file":"web/src/pages/dashboard/review-detail.tsx","line":483,"export":"ReviewDetailPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardNamespaceReviewDetailRoute","path":"dashboard/namespaces/$slug/reviews/$id","declaration":{"file":"web/src/app/router.tsx","line":643},"implementation":{"file":"web/src/pages/dashboard/review-detail.tsx","line":496,"export":"NamespaceReviewDetailPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardPromotionsRoute","path":"dashboard/promotions","declaration":{"file":"web/src/app/router.tsx","line":651},"implementation":{"file":"web/src/pages/dashboard/promotions.tsx","line":408,"export":"PromotionsPage"},"guarded":true,"roleProtected":true},
{"name":"dashboardStarsRoute","path":"dashboard/stars","declaration":{"file":"web/src/app/router.tsx","line":658},"implementation":{"file":"web/src/pages/dashboard/stars.tsx","line":12,"export":"MyStarsPage"},"guarded":true,"roleProtected":false},
{"name":"dashboardSubscriptionsRoute","path":"dashboard/subscriptions","declaration":{"file":"web/src/app/router.tsx","line":668},"implementation":{"file":"web/src/pages/dashboard/subscriptions.tsx","line":12,"export":"MySubscriptionsPage"},"guarded":true,"roleProtected":false},
@ -276,15 +276,15 @@
{"file":"web/src/features/review/review-center-filter-form.tsx","line":34,"host":"SelectItem","prop":"data-analytics-id","event":"radix-item-intent","ids":["review.filter.kind.select","review.filter.namespace.select","review.filter.origin.select","review.filter.status.select"],"conditional":true},
{"file":"web/src/features/review/review-center-filter-form.tsx","line":55,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.filter.reset"]},
{"file":"web/src/features/review/review-center-filter-form.tsx","line":56,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.filter.apply"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":45,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.content.retry"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":86,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.package.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":91,"host":"summary","prop":"data-analytics-id","event":"click","ids":["review.files.expand"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":97,"host":"FileTree","prop":"analyticsId","event":"click","ids":["review.file.open"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":110,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":123,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":132,"host":"FilePreviewDialog","prop":"closeAnalyticsId","event":"click","ids":["review.file.close"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":132,"host":"FilePreviewDialog","prop":"downloadAnalyticsId","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":132,"host":"FilePreviewDialog","prop":"copyAnalyticsId","event":"click","ids":["review.file.copy"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":47,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.content.retry"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":75,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.files.expand"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":79,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.package.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":96,"host":"FileTree","prop":"analyticsId","event":"click","ids":["review.file.open"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":109,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":122,"host":"a","prop":"data-analytics-id","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":139,"host":"FilePreviewDialog","prop":"closeAnalyticsId","event":"click","ids":["review.file.close"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":139,"host":"FilePreviewDialog","prop":"downloadAnalyticsId","event":"click","ids":["review.file.download"]},
{"file":"web/src/features/review/review-skill-detail-section.tsx","line":139,"host":"FilePreviewDialog","prop":"copyAnalyticsId","event":"click","ids":["review.file.copy"]},
{"file":"web/src/features/search/search-bar.tsx","line":55,"host":"form","prop":"data-analytics-id","event":"submit","ids":[],"conditional":true,"forward":"analyticsId"},
{"file":"web/src/features/search/search-bar.tsx","line":67,"host":"button","prop":"data-analytics-id","event":"click","ids":["search.query.clear"],"conditional":true},
{"file":"web/src/features/security-audit/security-audit-section.tsx","line":114,"host":"Button","prop":"data-analytics-id","event":"click","ids":["security.findings.toggle"]},
@ -669,23 +669,23 @@
{"file":"web/src/pages/dashboard/reports.tsx","line":210,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["report.dismissed.tab"]},
{"file":"web/src/pages/dashboard/reports.tsx","line":224,"host":"ConfirmDialog","prop":"confirmAnalyticsId","event":"click","ids":["report.archive.confirm","report.dismiss.confirm","report.hide.confirm","report.resolve.confirm"],"conditional":true},
{"file":"web/src/pages/dashboard/reports.tsx","line":224,"host":"ConfirmDialog","prop":"cancelAnalyticsId","event":"click","ids":["report.archive.cancel","report.dismiss.cancel","report.hide.cancel","report.resolve.cancel"],"conditional":true},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":194,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.list.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":205,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.list.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":221,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.content.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":222,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.changes.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":223,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.source.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":224,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.history.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":260,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.suite.retry"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":326,"host":"button","prop":"data-analytics-id","event":"click","ids":["review.security.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":335,"host":"button","prop":"data-analytics-id","event":"click","ids":["review.source.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":361,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":372,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.request"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":387,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.cancel"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":398,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.approve.request"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":430,"host":"ConfirmDialog","prop":"confirmAnalyticsId","event":"click","ids":["review.approve.confirm"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":430,"host":"ConfirmDialog","prop":"cancelAnalyticsId","event":"click","ids":["review.approve.cancel"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":439,"host":"ConfirmDialog","prop":"confirmAnalyticsId","event":"click","ids":["review.reject.confirm"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":439,"host":"ConfirmDialog","prop":"cancelAnalyticsId","event":"click","ids":["review.reject.cancel"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":213,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.list.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":225,"host":"button","prop":"data-analytics-id","event":"click","ids":["review.security.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":234,"host":"button","prop":"data-analytics-id","event":"click","ids":["review.source.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":243,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.list.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":266,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.content.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":267,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.changes.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":268,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.source.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":269,"host":"TabsTrigger","prop":"data-analytics-id","event":"click","ids":["review.history.tab"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":307,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.suite.retry"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":392,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.open"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":403,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.request"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":418,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.reject.cancel"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":429,"host":"Button","prop":"data-analytics-id","event":"click","ids":["review.approve.request"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":461,"host":"ConfirmDialog","prop":"confirmAnalyticsId","event":"click","ids":["review.approve.confirm"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":461,"host":"ConfirmDialog","prop":"cancelAnalyticsId","event":"click","ids":["review.approve.cancel"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":470,"host":"ConfirmDialog","prop":"confirmAnalyticsId","event":"click","ids":["review.reject.confirm"]},
{"file":"web/src/pages/dashboard/review-detail.tsx","line":470,"host":"ConfirmDialog","prop":"cancelAnalyticsId","event":"click","ids":["review.reject.cancel"]},
{"file":"web/src/pages/dashboard/review-progress.tsx","line":79,"host":"form","prop":"data-analytics-id","event":"submit","ids":["review.progress.search"]},
{"file":"web/src/pages/dashboard/review-progress.tsx","line":100,"host":"SelectTrigger","prop":"data-analytics-id","event":"click+keydown-intent","ids":["review.progress.type.interact"]},
{"file":"web/src/pages/dashboard/review-progress.tsx","line":104,"host":"SelectItem","prop":"data-analytics-id","event":"radix-item-intent","ids":["review.progress.type"]},

View file

@ -51,7 +51,7 @@ 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.
Switching tabs or opening evidence smoothly returns to the detail page top, including its heading; reduced motion uses direct positioning. Query completion and comment edits do not reset the reading position. The summary has a distinct section, with package download and file browsing available before the overview. Comparison uses the available main width and stacks on narrow screens. Content uses the main page scroll.
### Understand Changes

View file

@ -51,7 +51,7 @@
这些是前端/接口实现限制,不由审核页面配置;依据[审核详情](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`。
切换内容页签或点击右侧核查入口时,页面定位至内容容器顶部;异步加载及填写审核意见不重复改变阅读位置。正文随主页面滚动,不另设内部滚动区域。
切换内容页签或点击核查入口时,页面平滑回到包含标题的详情页顶部;系统开启减少动态效果时直接定位。异步加载及填写审核意见不重复改变阅读位置。简介独立展示,下载与文件浏览位于内容顶部;变更对比使用主区域全宽,窄屏上下排列。正文随主页面滚动。
### 理解变更对比

View file

@ -22,11 +22,13 @@
详情顶部仅任务标题与事项,主区有效简介/概述及原文件,右侧标准Card集中摘要和证据。按任务决定Tab,包内容加载失败不能用已发布旧内容冒充;扫描、头像、许可证和并发决定仍遵守原服务条件。已有任务已处理后只读。
### 人工预览后的内容容器与定位修订
### 人工预览后的内容层次与定位修订
审核详情的Tab导航与当前内容合入同一有边界的内容容器,内部保留阅读间距;主页面统一滚动,不增加正文内部滚动条。短内容使用与视口相关的最小高度,保证导航能在固定页头下方对齐,不复制上一长面板的整段空白高度。
Tab导航与正文保留统一边界。简介使用独立浅底摘要区;详细概述保留清晰的正文标题与阅读间距。下载与浏览原文件入口放在内容顶部,文件区在简介之后、概述之前按需展开,不要求读完长概述后寻找入口。
用户切换Tab或点击右侧证据入口后,在新面板布局提交时立即定位到容器顶部;重复点击当前证据入口同样定位,并将焦点移至所选页签;普通页签导航保留自身焦点。异步查询完成、首次渲染及审核意见输入不重复触发定位。减少动态偏好下关闭内容淡入,不以长距离平滑滚动延迟内容可用。
变更对比占详情可用主区域全宽,申请摘要的折叠入口位于标题之后、对比之前;其他Tab保留双栏。对比按容器宽度自适应并排或上下展示,避免窄列。已修改字段默认展开,以状态文字和修改后底色凸显;未变化字段可展开查看,缺失概述不得标为未变化。Markdown字号仅在比较区局部收敛。
用户切换Tab或点击右侧证据入口后,平滑回到详情页顶部(包含标题),而非仅对齐Tab容器;减少动态偏好下直接定位。重复点击当前证据入口同样响应,并将焦点移至所选页签;普通页签导航保留自身焦点。异步查询完成、首次渲染及审核意见输入不重复触发定位,不维护动画队列。
## 取舍

View file

@ -60,9 +60,20 @@ The confirmed dual-column detail MUST put readable target content in the main co
### 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.
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 return to the detail page top, including its heading, 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
- **THEN** the detail heading is visible at the page top rather than leaving the reviewer at the document bottom
- **AND** later query completion does not reset the reviewer's reading position
#### Scenario: Navigation respects motion preferences
- **WHEN** the reviewer switches tabs or opens evidence
- **THEN** normal motion preferences use smooth scrolling to the page top
- **AND** reduced motion uses direct positioning without a queued animation
#### Scenario: Presentation and files are discoverable before a long overview
- **THEN** the summary is visually distinct from the overview
- **AND** file browsing and package download are available before the long overview
- **AND** comparison uses the available main width with responsive columns and explicit changed states

View file

@ -1,10 +1,14 @@
/** @vitest-environment jsdom */
import { renderToStaticMarkup } from 'react-dom/server'
import { describe, expect, it, vi } from 'vitest'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import type { ReviewSkillDetail } from '@/api/types'
import { ReviewPresentationComparison } from './review-presentation-comparison'
vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) }))
vi.mock('@/features/skill/markdown-renderer', () => ({ MarkdownRenderer: ({ content }: { content: string }) => <div>{content}</div> }))
vi.mock('@/features/skill/markdown-renderer', () => ({ MarkdownRenderer: ({ content, className }: { content: string; className?: string }) => <div data-testid="comparison-markdown" className={className}>{content}</div> }))
afterEach(cleanup)
function detail(): ReviewSkillDetail {
return {
@ -15,7 +19,7 @@ function detail(): ReviewSkillDetail {
baseVersion: '1.0.0',
baseline: { displayName: 'Meeting notes', summary: 'Original summary', overview: 'Original overview', overviewSource: 'WEB' },
current: { displayName: 'Meeting notes', summary: 'New summary', overview: 'New overview', overviewSource: 'WEB' },
files: [{ path: 'SKILL.md', changeType: 'MODIFIED' }, { path: 'README.md', changeType: 'UNCHANGED' }, { path: 'removed.txt', changeType: 'REMOVED' }],
files: [{ path: 'added.md', changeType: 'ADDED' }, { path: 'SKILL.md', changeType: 'MODIFIED' }, { path: 'README.md', changeType: 'UNCHANGED' }, { path: 'removed.txt', changeType: 'REMOVED' }],
},
}
}
@ -52,4 +56,62 @@ describe('ReviewPresentationComparison', () => {
expect(html).toContain('reviewWorkbench.noBaseline')
expect(html).not.toContain('1.0.0')
})
it('opens changed fields and leaves unchanged fields available behind a keyboard-operable disclosure', () => {
render(<ReviewPresentationComparison detail={detail()} />)
const name = screen.getByText('publish.displayName').closest('details')!
const summary = screen.getByText('reviewWorkbench.summary').closest('details')!
const overview = screen.getByText('reviewWorkbench.overview').closest('details')!
expect(name.open).toBe(false)
expect(summary.open).toBe(true)
expect(overview.open).toBe(true)
fireEvent.click(name.querySelector('summary')!)
expect(name.open).toBe(true)
fireEvent.click(summary.querySelector('summary')!)
expect(summary.open).toBe(false)
})
it('keeps unavailable overview expanded and explicitly distinct from unchanged content', () => {
const value = detail()
value.comparison!.baseline.overview = null
value.comparison!.current.overview = null
value.comparison!.baseline.overviewSource = 'README'
value.comparison!.current.overviewSource = 'README'
render(<ReviewPresentationComparison detail={value} />)
const overview = screen.getByText('reviewWorkbench.overview').closest('details')!
expect(overview.open).toBe(true)
expect(overview.querySelector('summary')?.textContent).toContain('reviewWorkbench.unavailableComparison')
expect(overview.querySelector('summary')?.textContent).not.toContain('reviewWorkbench.unchanged')
})
it('uses container-sized columns, scopes compact markdown and emphasizes the modified snapshot', () => {
render(<ReviewPresentationComparison detail={detail()} />)
const oldSummary = screen.getByText('Original summary').parentElement!
const newSummary = screen.getByText('New summary').parentElement!
expect(oldSummary.parentElement).toBe(newSummary.parentElement)
expect(newSummary.parentElement?.className).toContain('grid-cols-[repeat(auto-fit,minmax(min(100%,28rem),1fr))]')
expect(newSummary.parentElement?.className).not.toContain('md:grid-cols-2')
expect(newSummary.className).toContain('bg-amber-500/5')
expect(newSummary.className).toContain('dark:border-amber-400/30')
expect(oldSummary.className).not.toContain('bg-amber-500/5')
expect(screen.getAllByTestId('comparison-markdown')).toHaveLength(2)
for (const markdown of screen.getAllByTestId('comparison-markdown')) {
expect(markdown.className).toContain('[&_h1]:text-xl')
expect(markdown.className).toContain('[&_h2]:text-lg')
expect(markdown.className).toContain('[&_p]:text-sm')
expect(markdown.className).toContain('[&_p]:leading-6')
}
})
it('retains labels alongside distinct semantic file-change colors in both themes', () => {
render(<ReviewPresentationComparison detail={detail()} />)
for (const [kind, color] of [['ADDED', 'emerald'], ['MODIFIED', 'amber'], ['REMOVED', 'red']]) {
const badge = screen.getByText(`reviewWorkbench.file${kind}`)
expect(badge.className).toContain(`bg-${color}-500/10`)
expect(badge.className).toContain(`dark:text-${color}-200`)
}
expect(screen.getByText('reviewWorkbench.fileUNCHANGED').className).toContain('text-muted-foreground')
expect(screen.getAllByText('reviewWorkbench.modified').every((badge) => badge.className.includes('bg-amber-500/10'))).toBe(true)
})
})

View file

@ -1,12 +1,22 @@
import { ChevronDown } from 'lucide-react'
import { useTranslation } from 'react-i18next'
import type { ReviewSkillDetail } from '@/api/types'
import { MarkdownRenderer } from '@/features/skill/markdown-renderer'
import { Card } from '@/shared/ui/card'
import { cn } from '@/shared/lib/utils'
interface ReviewPresentationComparisonProps {
detail: ReviewSkillDetail
}
const COMPARISON_GRID = 'grid min-w-0 grid-cols-[repeat(auto-fit,minmax(min(100%,28rem),1fr))] gap-3'
const COMPARISON_MARKDOWN = 'text-sm leading-6 [&_h1]:mb-4 [&_h1]:pb-2 [&_h1]:text-xl [&_h1]:leading-7 [&_h2]:mb-3 [&_h2]:mt-6 [&_h2]:pb-2 [&_h2]:text-lg [&_h2]:leading-7 [&_h3]:mb-2 [&_h3]:mt-5 [&_h3]:text-base [&_h3]:leading-6 [&_h4]:text-sm [&_p]:my-3 [&_p]:text-sm [&_p]:leading-6 [&_li]:text-sm [&_li]:leading-6 [&_pre]:text-xs [&_table]:text-sm [&>:first-child]:mt-0 [&>:last-child]:mb-0'
const CHANGED_STATUS = 'border-amber-500/30 bg-amber-500/10 text-amber-900 dark:text-amber-200'
const FILE_STATUS: Record<string, string> = {
ADDED: 'border-emerald-500/30 bg-emerald-500/10 text-emerald-800 dark:text-emerald-200',
MODIFIED: CHANGED_STATUS,
REMOVED: 'border-red-500/30 bg-red-500/10 text-red-800 dark:text-red-200',
}
/** Compares only snapshots authorized and selected by the review endpoint. */
export function ReviewPresentationComparison({ detail }: ReviewPresentationComparisonProps) {
const { t } = useTranslation()
@ -20,8 +30,8 @@ export function ReviewPresentationComparison({ detail }: ReviewPresentationCompa
] as const
return (
<section className="space-y-6" aria-label={t('reviewWorkbench.presentationChanges')}>
<div className="grid grid-cols-2 gap-4 rounded-md bg-secondary/70 p-4 text-sm">
<section className="space-y-5" aria-label={t('reviewWorkbench.presentationChanges')}>
<div className="grid grid-cols-2 gap-4 rounded-lg bg-secondary/70 p-4 text-sm">
<div><p className="text-xs text-muted-foreground">{t('reviewWorkbench.baseline')}</p><p className="mt-1 break-all font-medium">{comparison.baseVersion}</p></div>
<div><p className="text-xs text-muted-foreground">{t('reviewWorkbench.pending')}</p><p className="mt-1 break-all font-medium">{detail.activeVersion}</p></div>
</div>
@ -31,30 +41,33 @@ export function ReviewPresentationComparison({ detail }: ReviewPresentationCompa
const unavailable = markdown && ((!before && comparison.baseline.overviewSource !== 'NONE') || (!after && comparison.current.overviewSource !== 'NONE'))
const changed = before !== after
return (
<section key={key} className="space-y-3">
<div className="flex flex-wrap items-center justify-between gap-2">
<h3 className="text-base font-semibold">{title}</h3>
<span className="rounded-full bg-secondary px-2.5 py-1 text-xs text-muted-foreground">{t(unavailable ? 'reviewWorkbench.unavailableComparison' : changed ? 'reviewWorkbench.modified' : 'reviewWorkbench.unchanged')}</span>
</div>
<Card className="grid min-w-0 divide-y divide-border/60 overflow-hidden md:grid-cols-2 md:divide-x md:divide-y-0">
<details key={key} open={changed || unavailable} className="group rounded-lg border border-border/60">
<summary className="flex min-h-11 cursor-pointer list-none flex-wrap items-center gap-3 rounded-lg px-4 py-3 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring [&::-webkit-details-marker]:hidden">
<ChevronDown aria-hidden="true" className="h-4 w-4 shrink-0 -rotate-90 text-muted-foreground group-open:rotate-0" />
<h3 className="min-w-0 flex-1 text-sm font-semibold">{title}</h3>
<span className={cn('rounded-full border px-2.5 py-1 text-xs font-medium', changed || unavailable ? CHANGED_STATUS : 'border-border/60 bg-secondary text-muted-foreground')}>
{t(unavailable ? 'reviewWorkbench.unavailableComparison' : changed ? 'reviewWorkbench.modified' : 'reviewWorkbench.unchanged')}
</span>
</summary>
<div className={cn(COMPARISON_GRID, 'border-t border-border/60 p-3 sm:p-4')}>
{([{ label: 'before', value: before, snapshot: comparison.baseline }, { label: 'after', value: after, snapshot: comparison.current }] as const).map(({ label, value, snapshot }) => (
<div key={label} className="min-w-0 space-y-3 p-5">
<p className="text-xs font-medium text-muted-foreground">{t(`reviewWorkbench.${label}`)}</p>
{value ? markdown ? <MarkdownRenderer content={value} /> : <p className="whitespace-pre-wrap break-words text-sm leading-6">{value}</p>
<div key={label} className={cn('min-w-0 space-y-3 rounded-lg border border-border/60 bg-card p-4 sm:p-5', label === 'after' && changed && 'border-amber-500/40 bg-amber-500/5 dark:border-amber-400/30 dark:bg-amber-400/5')}>
<p className={cn('text-xs font-medium', label === 'after' && changed ? 'text-amber-900 dark:text-amber-200' : 'text-muted-foreground')}>{t(`reviewWorkbench.${label}`)}</p>
{value ? markdown ? <MarkdownRenderer content={value} className={COMPARISON_MARKDOWN} /> : <p className="whitespace-pre-wrap break-words text-sm leading-6">{value}</p>
: <p className="text-sm text-muted-foreground">{t(markdown && snapshot.overviewSource !== 'NONE' ? 'reviewWorkbench.overviewUnavailable' : 'reviewWorkbench.notProvided')}</p>}
</div>
))}
</Card>
</section>
</div>
</details>
)
})}
<section className="space-y-3">
<h3 className="text-base font-semibold">{t('reviewWorkbench.fileChanges')}</h3>
<div className="divide-y divide-border/60 rounded-md border border-border/60">
<div className="divide-y divide-border/60 rounded-lg border border-border/60">
{comparison.files.map((file) => (
<div key={file.path} className="flex min-w-0 flex-wrap items-center justify-between gap-3 px-4 py-3 text-sm">
<span className="min-w-0 break-all font-mono text-xs">{file.path}</span>
<span className="shrink-0 rounded-full bg-secondary px-2.5 py-1 text-xs">{t(`reviewWorkbench.file${file.changeType}`)}</span>
<span className={cn('shrink-0 rounded-full border px-2.5 py-1 text-xs font-medium', FILE_STATUS[file.changeType] ?? 'border-border/60 bg-secondary text-muted-foreground')}>{t(`reviewWorkbench.file${file.changeType}`)}</span>
</div>
))}
{comparison.files.length === 0 && <p className="p-4 text-sm text-muted-foreground">{t('skillDetail.noFiles')}</p>}

View file

@ -1,5 +1,7 @@
/** @vitest-environment jsdom */
import { renderToStaticMarkup } from 'react-dom/server'
import { describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { cleanup, fireEvent, render, screen, within } from '@testing-library/react'
import type { ReviewSkillDetail } from '@/api/types'
import { ReviewSkillDetailSection } from './review-skill-detail-section'
@ -15,10 +17,10 @@ vi.mock('react-i18next', async () => {
}
})
// Mock the review file hook to avoid QueryClient dependency in static rendering
vi.mock('./use-review-file', () => ({
useReviewFile: () => ({ data: null, isLoading: false, error: null }),
}))
const mocks = vi.hoisted(() => ({ file: vi.fn(), refetch: vi.fn() }))
vi.mock('./use-review-file', () => ({ useReviewFile: (...args: unknown[]) => mocks.file(...args) }))
beforeEach(() => { vi.clearAllMocks(); mocks.file.mockReturnValue({ data: null, isLoading: false, error: null, refetch: mocks.refetch }) })
afterEach(cleanup)
function createDetail(): ReviewSkillDetail {
return {
@ -77,19 +79,66 @@ describe('ReviewSkillDetailSection', () => {
expect(html).toContain('Custom pending summary')
expect(html).toContain('Custom pending overview')
expect(html).not.toContain('Old public summary')
expect(html).toContain('Original README content')
expect(html.indexOf('Custom pending overview')).toBeLessThan(html.indexOf('Original README content'))
expect(html).toContain('reviewWorkbench.originalFiles')
expect(html).toContain('<details')
expect(html).not.toContain('Original README content')
expect(html).toContain('reviewWorkbench.browseFiles')
expect(html).toContain('aria-expanded="false"')
})
it('renders the download link, file tree, and review-bound documentation together', () => {
const html = renderToStaticMarkup(<ReviewSkillDetailSection detail={createDetail()} />)
expect(html).toContain('/api/v1/reviews/1/download')
expect(html).toContain('README.md')
expect(html).toContain('reviewWorkbench.browseFiles')
expect(html).toContain('Demo Skill')
expect(html).toContain('fileTree.title')
expect(html).not.toContain('fileTree.title')
})
it('opens original files near the summary before a long overview and closes them from the same toolbar', () => {
const detail = createDetail()
detail.presentation = { displayName: 'Pending name', summary: 'Summary for review', overview: '# Long custom overview\n\n' + 'Overview paragraph. '.repeat(300), overviewSource: 'WEB' }
detail.documentationContent = '# Original README content'
render(<ReviewSkillDetailSection detail={detail} reviewId={1} />)
const browse = screen.getByRole('button', { name: 'reviewWorkbench.browseFiles' })
const summary = screen.getByRole('region', { name: 'reviewWorkbench.summary' })
const overview = screen.getByRole('region', { name: 'reviewWorkbench.overview' })
expect(browse.compareDocumentPosition(summary) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy()
expect(screen.getAllByRole('link', { name: 'review.downloadSkillZip' })).toHaveLength(1)
expect(screen.queryByText('Original README content')).toBeNull()
fireEvent.click(browse)
expect(browse.getAttribute('aria-expanded')).toBe('true')
const files = screen.getByRole('region', { name: 'reviewWorkbench.packageFiles' })
expect(files.compareDocumentPosition(overview) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy()
expect(within(files).getByText('Original README content')).toBeTruthy()
fireEvent.click(browse)
expect(browse.getAttribute('aria-expanded')).toBe('false')
expect(screen.queryByRole('region', { name: 'reviewWorkbench.packageFiles' })).toBeNull()
})
it('requests missing original documentation only after browsing, and preserves the preview size limit', () => {
const detail = createDetail()
detail.documentationContent = null
const view = render(<ReviewSkillDetailSection detail={detail} reviewId={1} />)
expect(mocks.file).toHaveBeenCalledWith(1, 'README.md', false)
fireEvent.click(screen.getByRole('button', { name: 'reviewWorkbench.browseFiles' }))
expect(mocks.file).toHaveBeenCalledWith(1, 'README.md', true)
mocks.file.mockClear()
view.rerender(<ReviewSkillDetailSection detail={detail} reviewId={1} documentationTooLarge />)
expect(mocks.file).toHaveBeenCalledWith(1, 'README.md', false)
expect(screen.getByRole('link', { name: 'filePreview.downloadFile' })).toBeTruthy()
})
it('keeps original-document retry and download available without replacing a custom overview', () => {
const detail = createDetail()
detail.documentationContent = null
detail.presentation = { displayName: 'Pending name', summary: 'Custom summary', overview: '# Custom overview', overviewSource: 'WEB' }
mocks.file.mockReturnValue({ data: null, isLoading: false, error: new Error('read failed'), refetch: mocks.refetch })
render(<ReviewSkillDetailSection detail={detail} reviewId={1} />)
fireEvent.click(screen.getByRole('button', { name: 'reviewWorkbench.browseFiles' }))
expect(screen.getByText('review.documentationLoadFailed')).toBeTruthy()
expect(screen.getByText('Custom overview')).toBeTruthy()
fireEvent.click(screen.getByRole('button', { name: 'reviewWorkbench.retry' }))
expect(mocks.refetch).toHaveBeenCalledOnce()
expect(screen.getByRole('link', { name: 'filePreview.downloadFile' })).toBeTruthy()
})
it('renders loading state while review detail is being fetched', () => {

View file

@ -1,4 +1,5 @@
import { useState } from 'react'
import { useId, useState } from 'react'
import { Download, FolderOpen, ChevronDown } from 'lucide-react'
import { useTranslation } from 'react-i18next'
import { buildApiUrl, WEB_API_PREFIX } from '@/api/client'
import type { ReviewSkillDetail } from '@/api/types'
@ -32,6 +33,7 @@ export function ReviewSkillDetailSection({ detail, isLoading, hasError, reviewId
const [previewNode, setPreviewNode] = useState<FileTreeNode | null>(null)
const [previewDialogOpen, setPreviewDialogOpen] = useState(false)
const [filesExpanded, setFilesExpanded] = useState(false)
const filesRegionId = useId()
const originalFile = useReviewFile(reviewId, detail?.documentationPath ?? null, filesExpanded && !detail?.documentationContent && !documentationTooLarge)
const { data: previewContent, isLoading: isLoadingPreview, error: previewError } = useReviewFile(
@ -64,71 +66,76 @@ export function ReviewSkillDetailSection({ detail, isLoading, hasError, reviewId
return (
<div className="space-y-8">
<section className="space-y-6" aria-label={t('reviewWorkbench.displayContent')}>
<div className="space-y-2">
<h2 className="text-xl font-semibold">{t('reviewWorkbench.displayContent')}</h2>
<p className="text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.boundContent', { version: detail.activeVersion })}</p>
<div className="flex flex-wrap items-start justify-between gap-4 border-b border-border/60 pb-5">
<div className="space-y-2">
<h2 className="text-xl font-semibold">{t('reviewWorkbench.displayContent')}</h2>
<p className="text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.boundContent', { version: detail.activeVersion })}</p>
</div>
<div className="flex flex-wrap gap-2">
<Button data-analytics-id="review.files.expand" type="button" size="sm" variant="outline" aria-expanded={filesExpanded} aria-controls={filesRegionId} onClick={() => setFilesExpanded((expanded) => !expanded)}>
<FolderOpen className="mr-2 h-4 w-4" aria-hidden="true" />{t('reviewWorkbench.browseFiles')}
<ChevronDown className={`ml-2 h-3.5 w-3.5 ${filesExpanded ? 'rotate-180' : ''}`} aria-hidden="true" />
</Button>
{downloadHref && <a data-analytics-id="review.package.download" className={buttonVariants({ variant: 'outline', size: 'sm' })} href={buildApiUrl(downloadHref)}>
<Download className="mr-2 h-4 w-4" aria-hidden="true" />{t('review.downloadSkillZip')}
</a>}
</div>
</div>
<div className="space-y-3 border-b border-border/60 pb-6">
<h3 className="text-xs font-medium text-muted-foreground">{t('reviewWorkbench.summary')}</h3>
<section className="space-y-3 rounded-lg border border-border/60 bg-secondary/50 p-5" aria-label={t('reviewWorkbench.summary')}>
<h3 className="text-sm font-semibold">{t('reviewWorkbench.summary')}</h3>
<p className="whitespace-pre-wrap break-words text-base leading-7">{summary || t('reviewWorkbench.notProvided')}</p>
</section>
<div id={filesRegionId} hidden={!filesExpanded}>
{filesExpanded && <section className="space-y-4 rounded-lg border border-border/60 p-4" aria-label={t('reviewWorkbench.packageFiles')}>
<div><h3 className="text-base font-semibold">{t('reviewWorkbench.packageFiles')}</h3><p className="mt-1 text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.originalFiles')}</p></div>
<div className="grid min-w-0 gap-3 xl:grid-cols-[minmax(160px,200px)_minmax(0,1fr)]">
<Card className="min-w-0 p-3">
<h3 className="mb-2 text-sm font-semibold">{t('fileTree.title')} <span className="font-normal text-muted-foreground">{detail.files.length}</span></h3>
{detail.files.length > 0 ? (
<div className="max-h-[620px] overflow-auto">
<FileTree analyticsId="review.file.open" files={detail.files} onFileClick={(node) => { setPreviewNode(node); setPreviewDialogOpen(true) }} bare compact />
</div>
) : <p className="text-sm text-muted-foreground">{t('skillDetail.noFiles')}</p>}
</Card>
<Card className="min-w-0 p-4">
<h3 className="mb-3 text-sm font-semibold">{t('review.documentationTitle')}</h3>
{documentationLoading || originalFile.isLoading ? (
<div className="h-24 animate-shimmer rounded-lg" />
) : documentationError || originalFile.error ? (
<div className="space-y-2 text-sm text-muted-foreground">
<p className="text-destructive">{t('review.documentationLoadFailed')}</p>
<Button size="sm" variant="outline" onClick={() => void originalFile.refetch?.()}>{t('reviewWorkbench.retry')}</Button>
{detail.documentationPath && reviewId && (
<a data-analytics-id="review.file.download" className="text-primary underline" href={buildApiUrl(`${WEB_API_PREFIX}/reviews/${reviewId}/file?path=${encodeURIComponent(detail.documentationPath)}`)} download>
{t('filePreview.downloadFile')}
</a>
)}
</div>
) : documentation ? (
<div className="min-w-0">
<p className="mb-3 break-all font-mono text-xs text-muted-foreground">{documentation.path}</p>
<MarkdownRenderer content={documentation.content} />
</div>
) : documentationTooLarge && detail.documentationPath && reviewId ? (
<div className="space-y-2 text-sm text-muted-foreground">
<p>{t('review.documentationTooLarge')}</p>
<a data-analytics-id="review.file.download" className="text-primary underline" href={buildApiUrl(`${WEB_API_PREFIX}/reviews/${reviewId}/file?path=${encodeURIComponent(detail.documentationPath)}`)} download>
{t('filePreview.downloadFile')}
</a>
</div>
) : <p className="text-sm text-muted-foreground">{t('review.noDocumentation')}</p>}
</Card>
</div>
</section>}
</div>
<div className="space-y-4">
<h3 className="text-xs font-medium text-muted-foreground">{t('reviewWorkbench.overview')}</h3>
<section className="space-y-5" aria-label={t('reviewWorkbench.overview')}>
<h3 className="text-lg font-semibold">{t('reviewWorkbench.overview')}</h3>
{documentationLoading && !overview ? <div className="h-28 animate-shimmer rounded-lg" role="status" aria-label={t('review.scanLoading')} />
: documentationError && !overview ? <div role="alert" className="space-y-3"><p className="text-sm text-destructive">{t('review.documentationLoadFailed')}</p>{onRetry && <Button size="sm" variant="outline" onClick={onRetry}>{t('reviewWorkbench.retry')}</Button>}</div>
: overview ? <MarkdownRenderer content={overview} />
: <p className="text-sm text-muted-foreground">{t(documentationTooLarge ? 'review.documentationTooLarge' : 'reviewWorkbench.notProvided')}</p>}
</div>
</section>
</section>
<div className="space-y-4 border-t border-border/60 pt-5">
<div className="flex flex-wrap items-center justify-between gap-3">
<div><h2 className="text-base font-semibold">{t('reviewWorkbench.packageFiles')}</h2><p className="mt-1 text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.originalFiles')}</p></div>
{downloadHref && <a data-analytics-id="review.package.download" className={buttonVariants({ variant: 'outline', size: 'sm', className: 'px-2.5' })} href={buildApiUrl(downloadHref)}>
{t('review.downloadSkillZip')}
</a>}
</div>
<details className="group" onToggle={(event) => setFilesExpanded(event.currentTarget.open)}>
<summary data-analytics-id="review.files.expand" className="cursor-pointer py-2 text-sm font-medium text-foreground underline-offset-4 hover:underline">{t('reviewWorkbench.browseFiles')}</summary>
<div className="mt-3 grid min-w-0 gap-3 xl:grid-cols-[minmax(160px,200px)_minmax(0,1fr)]">
<Card className="min-w-0 p-3">
<h3 className="mb-2 text-sm font-semibold">{t('fileTree.title')} <span className="font-normal text-muted-foreground">{detail.files.length}</span></h3>
{detail.files.length > 0 ? (
<div className="max-h-[620px] overflow-auto">
<FileTree analyticsId="review.file.open" files={detail.files} onFileClick={(node) => { setPreviewNode(node); setPreviewDialogOpen(true) }} bare compact />
</div>
) : <p className="text-sm text-muted-foreground">{t('skillDetail.noFiles')}</p>}
</Card>
<Card className="min-w-0 p-4">
<h3 className="mb-3 text-sm font-semibold">{t('review.documentationTitle')}</h3>
{documentationLoading || originalFile.isLoading ? (
<div className="h-24 animate-shimmer rounded-lg" />
) : documentationError || originalFile.error ? (
<div className="space-y-2 text-sm text-muted-foreground">
<p className="text-destructive">{t('review.documentationLoadFailed')}</p>
<Button size="sm" variant="outline" onClick={() => void originalFile.refetch?.()}>{t('reviewWorkbench.retry')}</Button>
{detail.documentationPath && reviewId && (
<a data-analytics-id="review.file.download" className="text-primary underline" href={buildApiUrl(`${WEB_API_PREFIX}/reviews/${reviewId}/file?path=${encodeURIComponent(detail.documentationPath)}`)} download>
{t('filePreview.downloadFile')}
</a>
)}
</div>
) : documentation ? (
<div className="min-w-0">
<p className="mb-3 break-all font-mono text-xs text-muted-foreground">{documentation.path}</p>
<MarkdownRenderer content={documentation.content} />
</div>
) : documentationTooLarge && detail.documentationPath && reviewId ? (
<div className="space-y-2 text-sm text-muted-foreground">
<p>{t('review.documentationTooLarge')}</p>
<a data-analytics-id="review.file.download" className="text-primary underline" href={buildApiUrl(`${WEB_API_PREFIX}/reviews/${reviewId}/file?path=${encodeURIComponent(detail.documentationPath)}`)} download>
{t('filePreview.downloadFile')}
</a>
</div>
) : <p className="text-sm text-muted-foreground">{t('review.noDocumentation')}</p>}
</Card>
</div>
</details>
</div>
<FilePreviewDialog closeAnalyticsId="review.file.close" downloadAnalyticsId="review.file.download"
open={previewDialogOpen}
onOpenChange={setPreviewDialogOpen}

View file

@ -6,7 +6,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { ReviewSkillDetail } from '@/api/types'
const navigateMock = vi.fn()
const scrollIntoViewMock = vi.fn()
const scrollToMock = vi.fn()
const matchMediaMock = vi.fn()
const route = vi.hoisted(() => ({ id: '13', search: {} as Record<string, unknown> }))
vi.mock('@tanstack/react-router', () => ({
@ -172,8 +173,10 @@ import { NamespaceReviewDetailPage, ReviewDetailPage } from './review-detail'
describe('ReviewDetailPage', () => {
beforeEach(() => {
scrollIntoViewMock.mockReset()
HTMLElement.prototype.scrollIntoView = scrollIntoViewMock
scrollToMock.mockReset()
matchMediaMock.mockReset().mockReturnValue({ matches: false })
vi.stubGlobal('matchMedia', matchMediaMock)
window.scrollTo = scrollToMock
route.id = '13'
route.search = {}
navigateMock.mockReset()
@ -259,7 +262,7 @@ describe('ReviewDetailPage', () => {
})
})
afterEach(cleanup)
afterEach(() => { cleanup(); vi.unstubAllGlobals() })
it('clears comment, reject form and confirmation when the route switches to another review', () => {
const view = render(<ReviewDetailPage />)
@ -557,8 +560,20 @@ describe('ReviewDetailPage', () => {
expect(screen.getByText('Pending custom summary')).toBeTruthy()
expect(screen.getByRole('tab', { name: 'review.tabChanges' })).toBeTruthy()
fireEvent.click(screen.getByRole('tab', { name: 'review.tabChanges' }))
expect(screen.getByTestId('review-detail-layout').className).not.toContain('268px')
const applicationDetails = screen.getByText('reviewWorkbench.requestSummary', { selector: 'summary' }).closest('details')
expect(applicationDetails?.open).toBe(false)
expect((applicationDetails?.compareDocumentPosition(screen.getByTestId('review-detail-workbench')) ?? 0) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy()
expect(screen.getByText('Published summary')).toBeTruthy()
expect(screen.getByText('1.0.0')).toBeTruthy()
fireEvent.click(screen.getByText('reviewWorkbench.requestSummary', { selector: 'summary' }))
expect(applicationDetails?.open).toBe(true)
const scanShortcut = screen.getByRole('button', { name: /review.scanSummary/ })
scanShortcut.focus()
fireEvent.click(scanShortcut)
expect(document.activeElement).toBe(screen.getByRole('tab', { name: 'review.tabSourceSecurity' }))
expect(scrollToMock).toHaveBeenLastCalledWith({ top: 0, behavior: 'smooth' })
expect(screen.getByTestId('review-detail-layout').className).toContain('268px')
})
it('keeps approval blocked after a failed comparison when switching to history', () => {
@ -628,14 +643,13 @@ describe('ReviewDetailPage', () => {
expect(query.get('page')).toBe('2')
})
it('wraps navigation and panels in a single responsive surface with a header scroll offset', () => {
it('wraps navigation and panels in a single responsive surface', () => {
render(<ReviewDetailPage />)
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('min-h-[28rem]')
expect(surface?.className).toContain('border-border/60')
expect(surface?.className).not.toContain('overflow-auto')
expect(screen.getByRole('tabpanel').className).toContain('motion-reduce:animate-none')
@ -647,17 +661,17 @@ describe('ReviewDetailPage', () => {
presentation: { displayName: 'Long pending content', summary: 'Pending summary', overview: 'Long review paragraph.\n\n'.repeat(80), overviewSource: 'WEB' },
} })
const view = render(<ReviewDetailPage />)
expect(scrollIntoViewMock).not.toHaveBeenCalled()
expect(scrollToMock).not.toHaveBeenCalled()
let panelAtScroll = ''
scrollIntoViewMock.mockImplementation(() => { panelAtScroll = screen.getByRole('tabpanel').textContent ?? '' })
scrollToMock.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' })
expect(scrollToMock).toHaveBeenCalledTimes(1)
expect(scrollToMock).toHaveBeenCalledWith({ top: 0, behavior: 'smooth' })
view.rerender(<ReviewDetailPage />)
fireEvent.change(screen.getByLabelText('review.commentLabel'), { target: { value: 'A review comment' } })
expect(scrollIntoViewMock).toHaveBeenCalledTimes(1)
expect(scrollToMock).toHaveBeenCalledTimes(1)
})
it('moves focus from a shortcut to the selected tab before one scroll, including repeated clicks', () => {
@ -674,15 +688,15 @@ describe('ReviewDetailPage', () => {
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])
expect(scrollToMock).toHaveBeenCalledTimes(2)
expect(scrollToMock).toHaveBeenNthCalledWith(1, { top: 0, behavior: 'smooth' })
expect(scrollToMock).toHaveBeenNthCalledWith(2, { top: 0, behavior: 'smooth' })
const comment = screen.getByLabelText('review.commentLabel')
comment.focus()
fireEvent.change(comment, { target: { value: 'Continue reviewing' } })
view.rerender(<ReviewDetailPage />)
expect(document.activeElement).toBe(comment)
expect(scrollIntoViewMock).toHaveBeenCalledTimes(2)
expect(scrollToMock).toHaveBeenCalledTimes(2)
})
it('restores the content top when keyboard navigation changes the panel', () => {
@ -693,7 +707,7 @@ describe('ReviewDetailPage', () => {
const history = screen.getByRole('tab', { name: 'review.tabHistory' })
expect(history.getAttribute('aria-selected')).toBe('true')
expect(document.activeElement).toBe(history)
expect(scrollIntoViewMock).toHaveBeenCalledTimes(1)
expect(scrollToMock).toHaveBeenCalledTimes(1)
})
it('moves the license shortcut focus to the source tab', () => {
@ -704,7 +718,16 @@ describe('ReviewDetailPage', () => {
shortcut.focus()
fireEvent.click(shortcut)
expect(document.activeElement).toBe(screen.getByRole('tab', { name: 'review.tabSourceSecurity' }))
expect(scrollIntoViewMock).toHaveBeenCalledTimes(1)
expect(scrollToMock).toHaveBeenCalledTimes(1)
})
it('returns directly to the detail heading when reduced motion is requested', () => {
matchMediaMock.mockReturnValue({ matches: true })
render(<ReviewDetailPage />)
fireEvent.click(screen.getByRole('tab', { name: 'review.tabHistory' }))
expect(matchMediaMock).toHaveBeenCalledWith('(prefers-reduced-motion: reduce)')
expect(scrollToMock).toHaveBeenCalledOnce()
expect(scrollToMock).toHaveBeenCalledWith({ top: 0, behavior: 'instant' })
})
})

View file

@ -75,7 +75,8 @@ function ReviewDetailScreen({
tabsRef.current?.querySelector<HTMLElement>('[role="tab"][aria-selected="true"]')?.focus({ preventScroll: true })
focusTabAfterNavigation.current = false
}
tabsRef.current?.scrollIntoView({ block: 'start', behavior: 'instant' })
const reducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches
window.scrollTo({ top: 0, behavior: reducedMotion ? 'instant' : 'smooth' })
}, [tabNavigation])
const activeTab = selectedTab ?? (isGithubSyncReview && githubSource?.baseVersionId ? 'changes' : 'content')
const {
@ -217,6 +218,25 @@ function ReviewDetailScreen({
)
}
const requestSummary = (
<ReviewTaskSummary review={review} detail={reviewSkillDetail} hasBaseline={hasBaseline} submittedAt={formatDate(review.submittedAt)}>
{!isSuiteReview && !isMetadataReview && <div className="space-y-3">
<h3 className="text-base font-semibold">{t('reviewWorkbench.checks')}</h3>
<button data-analytics-id="review.security.open" type="button" onClick={() => selectTab('source', true)} className="block w-full space-y-2 text-left text-sm underline-offset-4 hover:underline">
<span className="block font-medium">{t('review.scanSummary')}</span>
{review.requestedVisibility
? <span className="block text-xs text-muted-foreground">{reviewSkillDetail?.visibilityChange ? t(`skillVisibility.scan${reviewSkillDetail.visibilityChange.scanStatus}`) : t('review.scanLoadFailed')}</span>
: isLoadingReviewSkillDetail || (skillId && versionId && isLoadingSecurityAudits) ? <span className="block text-xs text-muted-foreground">{t('review.scanLoading')}</span>
: reviewSkillDetailError || isSecurityAuditsError ? <span className="block text-xs text-destructive">{t('review.scanLoadFailed')}</span>
: securityAudits?.length ? <span className="flex flex-wrap gap-2">{securityAudits.map((audit) => <VerdictBadge key={audit.id} displayState={getSecurityAuditDisplayState(audit, activeReviewVersion?.status)} compact />)}</span>
: <span className="block text-xs text-muted-foreground">{t(activeReviewVersion?.status === 'SCANNING' ? 'review.scanPending' : activeReviewVersion?.status === 'SCAN_FAILED' ? 'review.scanFailed' : 'review.scanUnavailable')}</span>}
</button>
{isGithubSyncReview && <button type="button" data-analytics-id="review.source.open" onClick={() => selectTab('source', true)} className="space-y-1 text-left text-sm underline-offset-4 hover:underline"><span className="block font-medium">{t('review.githubLicenseStatus')}</span><span className="block text-xs text-muted-foreground">{githubSource ? t(`review.githubLicense${githubSource.evidence.status}`, { defaultValue: githubSource.evidence.status }) : t(isGithubSourceError ? 'review.githubSourceError' : 'review.scanLoading')}</span></button>}
<p className="text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.scanBoundary')}</p>
</div>}
</ReviewTaskSummary>
)
return (
<div className="flex min-h-[calc(100dvh-5.25rem)] w-full flex-col gap-4 animate-fade-up">
<div className="space-y-4">
@ -232,9 +252,15 @@ function ReviewDetailScreen({
</div>
</div>
</div>
<div className="grid min-w-0 items-start gap-6 xl:grid-cols-[minmax(0,1fr)_268px]">
{availableTab === 'changes' && (
<details className="rounded-xl border border-border/60 bg-card">
<summary className="cursor-pointer px-5 py-4 text-sm font-medium">{t('reviewWorkbench.requestSummary')} <span className="ml-3 text-xs font-normal text-muted-foreground">{reviewSkillDetail?.activeVersion ?? review.version}</span></summary>
<div className="max-w-sm p-4 pt-0">{requestSummary}</div>
</details>
)}
<div data-testid="review-detail-layout" className={`grid min-w-0 items-start gap-6 ${availableTab === 'changes' ? 'grid-cols-1' : 'xl:grid-cols-[minmax(0,1fr)_268px]'}`}>
<div className="order-2 min-w-0 xl:order-1">
<Tabs data-testid="review-detail-workbench" ref={tabsRef} value={availableTab} onValueChange={selectTab} className="min-h-[calc(100svh-6rem)] min-w-0 scroll-mt-24 rounded-xl border border-border/60 bg-card text-card-foreground [overflow-anchor:none]">
<Tabs data-testid="review-detail-workbench" ref={tabsRef} value={availableTab} onValueChange={selectTab} className="min-h-[28rem] min-w-0 rounded-xl border border-border/60 bg-card text-card-foreground [overflow-anchor:none]">
<div className="border-b border-border/60 p-3 sm:px-5 sm:py-4">
<TabsList className="grid w-full grid-cols-2 gap-1 rounded-2xl border-0 bg-secondary/70 p-1 sm:inline-flex sm:w-auto sm:flex-wrap sm:rounded-full" aria-label={t('reviewWorkbench.contentNavigation')}>
<TabsTrigger data-analytics-id="review.content.tab" value="content" className="mb-0 min-h-9 rounded-xl border-0 px-4 py-2 font-normal data-[state=active]:bg-background data-[state=active]:font-medium data-[state=active]:shadow-sm sm:rounded-full">{t('review.tabContent')}</TabsTrigger>
@ -341,24 +367,7 @@ function ReviewDetailScreen({
</div>
</Tabs>
</div>
<div className="order-1 min-w-0 xl:order-2">
<ReviewTaskSummary review={review} detail={reviewSkillDetail} hasBaseline={hasBaseline} submittedAt={formatDate(review.submittedAt)}>
{!isSuiteReview && !isMetadataReview && <div className="space-y-3">
<h3 className="text-base font-semibold">{t('reviewWorkbench.checks')}</h3>
<button data-analytics-id="review.security.open" type="button" onClick={() => selectTab('source', true)} className="block w-full space-y-2 text-left text-sm underline-offset-4 hover:underline">
<span className="block font-medium">{t('review.scanSummary')}</span>
{review.requestedVisibility
? <span className="block text-xs text-muted-foreground">{reviewSkillDetail?.visibilityChange ? t(`skillVisibility.scan${reviewSkillDetail.visibilityChange.scanStatus}`) : t('review.scanLoadFailed')}</span>
: isLoadingReviewSkillDetail || (skillId && versionId && isLoadingSecurityAudits) ? <span className="block text-xs text-muted-foreground">{t('review.scanLoading')}</span>
: reviewSkillDetailError || isSecurityAuditsError ? <span className="block text-xs text-destructive">{t('review.scanLoadFailed')}</span>
: securityAudits?.length ? <span className="flex flex-wrap gap-2">{securityAudits.map((audit) => <VerdictBadge key={audit.id} displayState={getSecurityAuditDisplayState(audit, activeReviewVersion?.status)} compact />)}</span>
: <span className="block text-xs text-muted-foreground">{t(activeReviewVersion?.status === 'SCANNING' ? 'review.scanPending' : activeReviewVersion?.status === 'SCAN_FAILED' ? 'review.scanFailed' : 'review.scanUnavailable')}</span>}
</button>
{isGithubSyncReview && <button type="button" data-analytics-id="review.source.open" onClick={() => selectTab('source', true)} className="space-y-1 text-left text-sm underline-offset-4 hover:underline"><span className="block font-medium">{t('review.githubLicenseStatus')}</span><span className="block text-xs text-muted-foreground">{githubSource ? t(`review.githubLicense${githubSource.evidence.status}`, { defaultValue: githubSource.evidence.status }) : t(isGithubSourceError ? 'review.githubSourceError' : 'review.scanLoading')}</span></button>}
<p className="text-xs leading-5 text-muted-foreground">{t('reviewWorkbench.scanBoundary')}</p>
</div>}
</ReviewTaskSummary>
</div>
{availableTab !== 'changes' && <div className="order-1 min-w-0 xl:order-2">{requestSummary}</div>}
</div>
<div className="sticky bottom-0 z-20 -mx-3 mt-auto border-t border-border bg-background px-3 py-3 pb-[calc(0.75rem+env(safe-area-inset-bottom))] shadow-[0_-5px_16px_-12px_rgb(0_0_0/0.28)] sm:-mx-4 sm:px-4">