fix: collapse review skill detail by default

This commit is contained in:
yun-zhi-ztl 2026-03-19 20:26:56 +08:00
parent 08b708c25b
commit 15d51c8ee9
4 changed files with 239 additions and 197 deletions

View file

@ -55,14 +55,14 @@ describe('ReviewSkillDetailSection', () => {
const html = renderToStaticMarkup(<ReviewSkillDetailSection detail={createDetail()} />)
expect(html).toContain('1.2.0')
expect(html).toContain('/api/v1/reviews/1/download')
expect(html).toContain('Expand full overview')
})
it('renders the detail card as an independently scrollable pane on large screens', () => {
it('renders the detail content inside a collapsed disclosure card by default', () => {
const html = renderToStaticMarkup(<ReviewSkillDetailSection detail={createDetail()} />)
expect(html).toContain('xl:max-h-[calc(100vh-8rem)]')
expect(html).toContain('xl:overflow-y-auto')
expect(html).toContain('aria-expanded="false"')
expect(html).not.toContain('data-review-skill-detail-panel')
})
it('renders inline error state without requiring detail data', () => {

View file

@ -1,9 +1,11 @@
import { useState } from 'react'
import { ChevronDown, ChevronUp } from 'lucide-react'
import { useTranslation } from 'react-i18next'
import { buildApiUrl } from '@/api/client'
import type { ReviewSkillDetail } from '@/api/types'
import { FileTree } from '@/features/skill/file-tree'
import { MarkdownRenderer } from '@/features/skill/markdown-renderer'
import { buttonVariants } from '@/shared/ui/button'
import { Button, buttonVariants } from '@/shared/ui/button'
import { Card } from '@/shared/ui/card'
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs'
import { getReviewDownloadHref, getReviewSkillDocumentation, isActiveReviewVersion } from './review-skill-detail'
@ -16,6 +18,7 @@ interface ReviewSkillDetailSectionProps {
export function ReviewSkillDetailSection({ detail, isLoading, hasError }: ReviewSkillDetailSectionProps) {
const { t } = useTranslation()
const [isExpanded, setIsExpanded] = useState(false)
if (isLoading) {
return (
@ -42,8 +45,13 @@ export function ReviewSkillDetailSection({ detail, isLoading, hasError }: Review
const documentation = getReviewSkillDocumentation(detail)
return (
<Card className="p-8 space-y-6 xl:max-h-[calc(100vh-8rem)] xl:overflow-y-auto">
<div className="flex flex-col gap-4 md:flex-row md:items-start md:justify-between">
<Card className="p-6 space-y-4">
<button
type="button"
className="flex w-full items-start justify-between gap-4 text-left"
aria-expanded={isExpanded}
onClick={() => setIsExpanded((current) => !current)}
>
<div className="space-y-2">
<div className="flex flex-wrap items-center gap-2">
<h2 className="text-xl font-bold font-heading">{t('review.skillDetailTitle')}</h2>
@ -60,80 +68,118 @@ export function ReviewSkillDetailSection({ detail, isLoading, hasError }: Review
})}
</p>
</div>
<a className={buttonVariants()} href={buildApiUrl(getReviewDownloadHref(detail))}>
{t('review.downloadSkillZip')}
</a>
</div>
<span className="mt-1 shrink-0 rounded-full border border-border/70 p-2 text-muted-foreground">
{isExpanded ? <ChevronUp className="h-4 w-4" /> : <ChevronDown className="h-4 w-4" />}
</span>
</button>
<Tabs defaultValue="overview" className="space-y-4">
<TabsList>
<TabsTrigger value="overview">{t('skillDetail.tabOverview')}</TabsTrigger>
<TabsTrigger value="files">{t('skillDetail.tabFiles')}</TabsTrigger>
<TabsTrigger value="versions">{t('skillDetail.tabVersions')}</TabsTrigger>
</TabsList>
{isExpanded ? (
<div data-review-skill-detail-panel className="space-y-6 border-t border-border/60 pt-4">
<div className="flex justify-start">
<a className={buttonVariants()} href={buildApiUrl(getReviewDownloadHref(detail))}>
{t('review.downloadSkillZip')}
</a>
</div>
<TabsContent value="overview" className="space-y-4">
{documentation ? (
<div className="space-y-3">
<p className="text-sm font-mono text-muted-foreground">{documentation.path}</p>
<div className="rounded-2xl border border-border/60 bg-card/60 p-6">
<MarkdownRenderer content={documentation.content} />
</div>
</div>
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('review.noDocumentation')}
</div>
)}
</TabsContent>
<Tabs defaultValue="overview" className="space-y-4">
<TabsList>
<TabsTrigger value="overview">{t('skillDetail.tabOverview')}</TabsTrigger>
<TabsTrigger value="files">{t('skillDetail.tabFiles')}</TabsTrigger>
<TabsTrigger value="versions">{t('skillDetail.tabVersions')}</TabsTrigger>
</TabsList>
<TabsContent value="files">
{detail.files.length > 0 ? (
<FileTree files={detail.files} />
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('skillDetail.noFiles')}
</div>
)}
</TabsContent>
<TabsContent value="versions">
{detail.versions.length > 0 ? (
<div className="space-y-3">
{detail.versions.map((version) => (
<div
key={version.id}
className="flex flex-col gap-3 rounded-2xl border border-border/70 bg-card/70 p-4 md:flex-row md:items-center md:justify-between"
>
<div className="space-y-2">
<div className="flex flex-wrap items-center gap-2">
<span className="font-semibold font-mono">{version.version}</span>
<span className="inline-flex items-center rounded-full border border-border px-2.5 py-0.5 text-xs font-medium text-foreground">
{version.status}
</span>
{isActiveReviewVersion(version, detail) ? (
<span className="inline-flex items-center rounded-full bg-brand-gradient px-2.5 py-0.5 text-xs font-medium text-white">
{t('review.activeReviewVersion')}
</span>
) : null}
</div>
{version.changelog ? (
<p className="text-sm text-muted-foreground">{version.changelog}</p>
) : null}
</div>
<div className="text-sm text-muted-foreground">
{t('skillDetail.fileCount', { count: version.fileCount })}
<TabsContent value="overview" className="space-y-4">
{documentation ? (
<div className="space-y-3">
<p className="text-sm font-mono text-muted-foreground">{documentation.path}</p>
<div className="rounded-2xl border border-border/60 bg-card/60 p-6">
<MarkdownRenderer content={documentation.content} />
</div>
</div>
))}
</div>
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('skillDetail.noVersions')}
</div>
)}
</TabsContent>
</Tabs>
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('review.noDocumentation')}
</div>
)}
</TabsContent>
<TabsContent value="files">
{detail.files.length > 0 ? (
<FileTree files={detail.files} />
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('skillDetail.noFiles')}
</div>
)}
</TabsContent>
<TabsContent value="versions">
{detail.versions.length > 0 ? (
<div className="space-y-3">
{detail.versions.map((version) => (
<div
key={version.id}
className="flex flex-col gap-3 rounded-2xl border border-border/70 bg-card/70 p-4 md:flex-row md:items-center md:justify-between"
>
<div className="space-y-2">
<div className="flex flex-wrap items-center gap-2">
<span className="font-semibold font-mono">{version.version}</span>
<span className="inline-flex items-center rounded-full border border-border px-2.5 py-0.5 text-xs font-medium text-foreground">
{version.status}
</span>
{isActiveReviewVersion(version, detail) ? (
<span className="inline-flex items-center rounded-full bg-brand-gradient px-2.5 py-0.5 text-xs font-medium text-white">
{t('review.activeReviewVersion')}
</span>
) : null}
</div>
{version.changelog ? (
<p className="text-sm text-muted-foreground">{version.changelog}</p>
) : null}
</div>
<div className="text-sm text-muted-foreground">
{t('skillDetail.fileCount', { count: version.fileCount })}
</div>
</div>
))}
</div>
) : (
<div className="rounded-2xl border border-dashed border-border/70 bg-muted/20 p-6 text-sm text-muted-foreground">
{t('skillDetail.noVersions')}
</div>
)}
</TabsContent>
</Tabs>
<div className="flex justify-start">
<Button
type="button"
variant="outline"
size="sm"
className="gap-2 rounded-full border-border/70 bg-background/90 px-5 shadow-sm backdrop-blur-sm"
aria-expanded={isExpanded}
onClick={() => setIsExpanded(false)}
>
<ChevronUp className="h-4 w-4" />
{t('skillDetail.collapseOverview')}
</Button>
</div>
</div>
) : (
<div className="flex justify-start">
<Button
type="button"
variant="outline"
size="sm"
className="gap-2 rounded-full border-border/70 bg-background/90 px-5 shadow-sm backdrop-blur-sm"
aria-expanded={isExpanded}
onClick={() => setIsExpanded(true)}
>
<ChevronDown className="h-4 w-4" />
{t('skillDetail.expandOverview')}
</Button>
</div>
)}
</Card>
)
}

View file

@ -110,10 +110,10 @@ describe('ReviewDetailPage', () => {
navigateMock.mockReset()
})
it('uses a two-column desktop layout that keeps moderation controls in a sticky sidebar', () => {
it('keeps the page in a single-column flow and leaves the skill detail behind a collapsed section', () => {
const html = renderToStaticMarkup(<ReviewDetailPage />)
expect(html).toContain('xl:grid xl:grid-cols-[minmax(0,24rem)_minmax(0,1fr)]')
expect(html).toContain('xl:sticky xl:top-6')
expect(html).toContain('max-w-3xl animate-fade-up')
expect(html).toContain('aria-expanded="false"')
})
})

View file

@ -62,8 +62,6 @@ export function ReviewDetailPage() {
}
const handleReject = async () => {
// Rejections require explicit operator feedback so submitters can understand
// what must change before the package is resubmitted.
if (!comment.trim()) {
toast.error(t('review.rejectReasonRequired'))
return
@ -89,7 +87,7 @@ export function ReviewDetailPage() {
}
return (
<div className="max-w-7xl animate-fade-up space-y-8 xl:grid xl:grid-cols-[minmax(0,24rem)_minmax(0,1fr)] xl:items-start xl:gap-8 xl:space-y-0">
<div className="space-y-8 max-w-3xl animate-fade-up">
<div className="flex items-center justify-between">
<div>
<h1 className="text-4xl font-bold font-heading mb-2">{t('review.detail')}</h1>
@ -100,129 +98,127 @@ export function ReviewDetailPage() {
</Button>
</div>
<div className="space-y-8 xl:sticky xl:top-6">
<Card className="p-8 space-y-6">
<div className="grid grid-cols-2 gap-6">
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.namespace')}</Label>
<p className="font-semibold font-mono">{review.namespace}/{review.skillSlug}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.version')}</Label>
<p className="font-semibold">
<span className="px-2.5 py-0.5 rounded-full bg-primary/10 text-primary text-sm font-mono">
{review.version}
</span>
</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.status')}</Label>
<p className="font-semibold">
{review.status === 'PENDING' && (
<span className="px-2.5 py-0.5 rounded-full bg-amber-500/10 text-amber-400 text-sm">{t('review.statusPending')}</span>
)}
{review.status === 'APPROVED' && (
<span className="px-2.5 py-0.5 rounded-full bg-emerald-500/10 text-emerald-400 text-sm">{t('review.statusApproved')}</span>
)}
{review.status === 'REJECTED' && (
<span className="px-2.5 py-0.5 rounded-full bg-red-500/10 text-red-400 text-sm">{t('review.statusRejected')}</span>
)}
</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.submitter')}</Label>
<p className="font-semibold">{review.submittedByName || review.submittedBy}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.submitTime')}</Label>
<p className="font-semibold text-muted-foreground">{formatDate(review.submittedAt)}</p>
</div>
{review.reviewedBy && (
<>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewer')}</Label>
<p className="font-semibold">{review.reviewedByName || review.reviewedBy}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewTime')}</Label>
<p className="font-semibold text-muted-foreground">
{review.reviewedAt ? formatDate(review.reviewedAt) : '—'}
</p>
</div>
</>
)}
</div>
{review.reviewComment && (
<div className="space-y-2">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewComment')}</Label>
<p className="p-4 bg-secondary/50 rounded-xl text-sm leading-relaxed">{review.reviewComment}</p>
</div>
)}
</Card>
{review.status === 'PENDING' && (
<Card className="p-8 space-y-6">
<div className="grid grid-cols-2 gap-6">
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.namespace')}</Label>
<p className="font-semibold font-mono">{review.namespace}/{review.skillSlug}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.version')}</Label>
<p className="font-semibold">
<span className="px-2.5 py-0.5 rounded-full bg-primary/10 text-primary text-sm font-mono">
{review.version}
</span>
</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.status')}</Label>
<p className="font-semibold">
{review.status === 'PENDING' && (
<span className="px-2.5 py-0.5 rounded-full bg-amber-500/10 text-amber-400 text-sm">{t('review.statusPending')}</span>
)}
{review.status === 'APPROVED' && (
<span className="px-2.5 py-0.5 rounded-full bg-emerald-500/10 text-emerald-400 text-sm">{t('review.statusApproved')}</span>
)}
{review.status === 'REJECTED' && (
<span className="px-2.5 py-0.5 rounded-full bg-red-500/10 text-red-400 text-sm">{t('review.statusRejected')}</span>
)}
</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.submitter')}</Label>
<p className="font-semibold">{review.submittedByName || review.submittedBy}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.submitTime')}</Label>
<p className="font-semibold text-muted-foreground">{formatDate(review.submittedAt)}</p>
</div>
{review.reviewedBy && (
<h2 className="text-xl font-bold font-heading">{t('review.actions')}</h2>
<div className="space-y-3">
<Label htmlFor="comment" className="text-sm font-semibold font-heading">{t('review.commentLabel')}</Label>
<Textarea
id="comment"
placeholder={t('review.commentPlaceholder')}
value={comment}
onChange={(e) => setComment(e.target.value)}
rows={4}
/>
</div>
<div className="flex gap-3">
<Button
onClick={() => setApproveDialog(true)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.approve')}
</Button>
{!showRejectForm ? (
<Button
variant="destructive"
onClick={() => setShowRejectForm(true)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.reject')}
</Button>
) : (
<>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewer')}</Label>
<p className="font-semibold">{review.reviewedByName || review.reviewedBy}</p>
</div>
<div className="space-y-1">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewTime')}</Label>
<p className="font-semibold text-muted-foreground">
{review.reviewedAt ? formatDate(review.reviewedAt) : '—'}
</p>
</div>
<Button
variant="destructive"
onClick={() => {
if (!comment.trim()) {
toast.error(t('review.rejectReasonRequired'))
return
}
setRejectDialog(true)
}}
disabled={approveMutation.isPending || rejectMutation.isPending || !comment.trim()}
>
{t('review.confirmReject')}
</Button>
<Button
variant="outline"
onClick={() => setShowRejectForm(false)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.cancelReject')}
</Button>
</>
)}
</div>
{review.reviewComment && (
<div className="space-y-2">
<Label className="text-xs text-muted-foreground uppercase tracking-wider">{t('review.reviewComment')}</Label>
<p className="p-4 bg-secondary/50 rounded-xl text-sm leading-relaxed">{review.reviewComment}</p>
</div>
{showRejectForm && !comment.trim() && (
<p className="text-sm text-destructive">{t('review.rejectReasonRequired')}</p>
)}
</Card>
{review.status === 'PENDING' && (
<Card className="p-8 space-y-6">
<h2 className="text-xl font-bold font-heading">{t('review.actions')}</h2>
<div className="space-y-3">
<Label htmlFor="comment" className="text-sm font-semibold font-heading">{t('review.commentLabel')}</Label>
<Textarea
id="comment"
placeholder={t('review.commentPlaceholder')}
value={comment}
onChange={(e) => setComment(e.target.value)}
rows={4}
/>
</div>
<div className="flex gap-3">
<Button
onClick={() => setApproveDialog(true)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.approve')}
</Button>
{!showRejectForm ? (
<Button
variant="destructive"
onClick={() => setShowRejectForm(true)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.reject')}
</Button>
) : (
<>
<Button
variant="destructive"
onClick={() => {
if (!comment.trim()) {
toast.error(t('review.rejectReasonRequired'))
return
}
setRejectDialog(true)
}}
disabled={approveMutation.isPending || rejectMutation.isPending || !comment.trim()}
>
{t('review.confirmReject')}
</Button>
<Button
variant="outline"
onClick={() => setShowRejectForm(false)}
disabled={approveMutation.isPending || rejectMutation.isPending}
>
{t('review.cancelReject')}
</Button>
</>
)}
</div>
{showRejectForm && !comment.trim() && (
<p className="text-sm text-destructive">{t('review.rejectReasonRequired')}</p>
)}
</Card>
)}
</div>
)}
<ReviewSkillDetailSection
detail={reviewSkillDetail}