mirror of
https://github.com/iflytek/skillhub.git
synced 2026-11-14 09:01:16 +00:00
fix(review): stabilize detail tabs and scroll position
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS
parent
6978da63a4
commit
4b33cf91fb
6 changed files with 133 additions and 10 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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 更新保留固定来源修订及逐行文件比较；头像和可见范围申请比较各自候选或目标，不生成内容版本差异。
|
||||
|
|
|
|||
|
|
@ -22,6 +22,12 @@
|
|||
|
||||
详情顶部仅任务标题与事项，主区有效简介/概述及原文件，右侧标准Card集中摘要和证据。按任务决定Tab，包内容加载失败不能用已发布旧内容冒充；扫描、头像、许可证和并发决定仍遵守原服务条件。已有任务已处理后只读。
|
||||
|
||||
### 人工预览后的内容容器与定位修订
|
||||
|
||||
审核详情的Tab导航与当前内容合入同一有边界的内容容器，内部保留阅读间距；主页面统一滚动，不增加正文内部滚动条。短内容使用与视口相关的最小高度，保证导航能在固定页头下方对齐，不复制上一长面板的整段空白高度。
|
||||
|
||||
用户切换Tab或点击右侧证据入口后，在新面板布局提交时立即定位到容器顶部；重复点击当前证据入口同样定位，并将焦点移至所选页签；普通页签导航保留自身焦点。异步查询完成、首次渲染及审核意见输入不重复触发定位。减少动态偏好下关闭内容淡入，不以长距离平滑滚动延迟内容可用。
|
||||
|
||||
## 取舍
|
||||
|
||||
只做样式覆盖会保留重复入口与缺失待审内容，因此同时收敛读模型。前端聚合各命名空间分页会导致排序与计数不正确，因此采用服务端授权过滤。新增完整通用工作流引擎或普通zip逐行diff均超出当前需求。
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> }))
|
||||
|
||||
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(<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('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(<ReviewDetailPage />)
|
||||
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(<ReviewDetailPage />)
|
||||
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(<ReviewDetailPage />)
|
||||
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(<ReviewDetailPage />)
|
||||
expect(document.activeElement).toBe(comment)
|
||||
expect(scrollIntoViewMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('restores the content top when keyboard navigation changes the panel', () => {
|
||||
render(<ReviewDetailPage />)
|
||||
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<string, unknown> }
|
||||
useReviewDetailMock.mockReturnValue({ ...task, data: { ...task.data, originType: 'GITHUB_SYNC' } })
|
||||
render(<ReviewDetailPage />)
|
||||
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)
|
||||
})
|
||||
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string | null>(null)
|
||||
const tabsRef = useRef<HTMLDivElement>(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<HTMLElement>('[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({
|
|||
</div>
|
||||
<div className="grid min-w-0 items-start gap-6 xl:grid-cols-[minmax(0,1fr)_268px]">
|
||||
<div className="order-2 min-w-0 xl:order-1">
|
||||
<Tabs value={availableTab} onValueChange={setSelectedTab} className="min-w-0 space-y-4">
|
||||
<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]">
|
||||
<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>
|
||||
{hasBaseline && <TabsTrigger data-analytics-id="review.changes.tab" value="changes" 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.tabChanges')}</TabsTrigger>}
|
||||
{!isSuiteReview && !isMetadataReview && <TabsTrigger data-analytics-id="review.source.tab" value="source" 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.tabSourceSecurity')}</TabsTrigger>}
|
||||
<TabsTrigger data-analytics-id="review.history.tab" value="history" 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.tabHistory')}</TabsTrigger>
|
||||
</TabsList>
|
||||
<TabsContent value="content" className="space-y-4">
|
||||
</div>
|
||||
<div className="min-w-0 p-4 sm:p-6">
|
||||
<TabsContent value="content" className="space-y-4 motion-reduce:animate-none">
|
||||
{reviewSkillDetail?.avatarChange && <ReviewAvatarChangeSection change={reviewSkillDetail.avatarChange} name={reviewSkillDetail.skill.displayName || review.skillSlug || ''} />}
|
||||
|
||||
{isSuiteReview && (
|
||||
|
|
@ -275,7 +296,7 @@ function ReviewDetailScreen({
|
|||
onRetry={() => { void refetchReviewSkillDetail?.(); void refetchDocumentation?.() }} />
|
||||
)}
|
||||
</TabsContent>
|
||||
<TabsContent value="changes" className="space-y-4">
|
||||
<TabsContent value="changes" className="space-y-4 motion-reduce:animate-none">
|
||||
{isGithubSyncReview && (
|
||||
isLoadingGithubSource ? <Card className="h-32 animate-shimmer" />
|
||||
: githubSource ? <GithubSyncReviewDiff source={githubSource} reviewId={taskId} />
|
||||
|
|
@ -292,7 +313,7 @@ function ReviewDetailScreen({
|
|||
hasError={Boolean(comparisonQuery.error)} onRetry={() => void comparisonQuery.refetch?.()} />
|
||||
)}
|
||||
</TabsContent>
|
||||
{!isSuiteReview && !isMetadataReview && <TabsContent value="source" className="space-y-4">
|
||||
{!isSuiteReview && !isMetadataReview && <TabsContent value="source" className="space-y-4 motion-reduce:animate-none">
|
||||
{review.requestedVisibility ? <Card className="space-y-3 p-5"><h2 className="text-lg font-semibold">{t('reviewWorkbench.boundScan')}</h2><p className="text-sm">{reviewSkillDetail?.visibilityChange ? t(`skillVisibility.scan${reviewSkillDetail.visibilityChange.scanStatus}`) : t('review.scanLoadFailed')}</p><p className="text-xs text-muted-foreground">{t('reviewWorkbench.boundScanDescription')}</p></Card>
|
||||
: skillId && versionId ? <SecurityAuditSection skillId={skillId} versionId={versionId}
|
||||
versionStatus={activeReviewVersion?.status} showEmptyState compact />
|
||||
|
|
@ -301,13 +322,13 @@ function ReviewDetailScreen({
|
|||
: githubSource ? <GithubSyncReviewEvidence source={githubSource} />
|
||||
: <Card className="p-4 text-sm text-destructive">{t('review.githubSourceError')}</Card>)}
|
||||
</TabsContent>}
|
||||
<TabsContent value="history" className="space-y-4">
|
||||
<Card className="space-y-3 p-4">
|
||||
<TabsContent value="history" className="space-y-4 motion-reduce:animate-none">
|
||||
<section className="space-y-3">
|
||||
<h2 className="text-base font-semibold">{t('review.attemptHistory')}</h2>
|
||||
{isLoadingReviewAttempts ? <div className="h-20 animate-shimmer rounded-lg" />
|
||||
: isReviewAttemptsError ? <p className="text-sm text-destructive">{t('review.attemptHistoryError')}</p>
|
||||
: <ReviewAttemptTimeline attempts={reviewAttempts ?? []} locale={i18n.language} compact />}
|
||||
</Card>
|
||||
</section>
|
||||
{(review.reviewedBy || review.reviewComment) && (
|
||||
<Card className="space-y-3 p-4">
|
||||
<h2 className="text-base font-semibold">{t('review.reviewResult')}</h2>
|
||||
|
|
@ -317,13 +338,14 @@ function ReviewDetailScreen({
|
|||
</Card>
|
||||
)}
|
||||
</TabsContent>
|
||||
</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={() => setSelectedTab('source')} className="block w-full space-y-2 text-left text-sm underline-offset-4 hover:underline">
|
||||
<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>
|
||||
|
|
@ -332,7 +354,7 @@ function ReviewDetailScreen({
|
|||
: 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={() => setSelectedTab('source')} 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>}
|
||||
{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>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue