From 15d51c8ee9053e22261283cd6aa34b41536dc002 Mon Sep 17 00:00:00 2001
From: yun-zhi-ztl <15071461069@163.com>
Date: Thu, 19 Mar 2026 20:26:56 +0800
Subject: [PATCH] fix: collapse review skill detail by default

---
 .../review-skill-detail-section.test.tsx      |   8 +-
 .../review/review-skill-detail-section.tsx    | 190 ++++++++------
 .../pages/dashboard/review-detail.test.tsx    |   6 +-
 web/src/pages/dashboard/review-detail.tsx     | 232 +++++++++---------
 4 files changed, 239 insertions(+), 197 deletions(-)

diff --git a/web/src/features/review/review-skill-detail-section.test.tsx b/web/src/features/review/review-skill-detail-section.test.tsx
index d0832a3b..fed3f16f 100644
--- a/web/src/features/review/review-skill-detail-section.test.tsx
+++ b/web/src/features/review/review-skill-detail-section.test.tsx
@@ -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', () => {
diff --git a/web/src/features/review/review-skill-detail-section.tsx b/web/src/features/review/review-skill-detail-section.tsx
index ccb8ed5b..55e8665d 100644
--- a/web/src/features/review/review-skill-detail-section.tsx
+++ b/web/src/features/review/review-skill-detail-section.tsx
@@ -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>
   )
 }
diff --git a/web/src/pages/dashboard/review-detail.test.tsx b/web/src/pages/dashboard/review-detail.test.tsx
index a925db1e..b6cbece2 100644
--- a/web/src/pages/dashboard/review-detail.test.tsx
+++ b/web/src/pages/dashboard/review-detail.test.tsx
@@ -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"')
   })
 })
diff --git a/web/src/pages/dashboard/review-detail.tsx b/web/src/pages/dashboard/review-detail.tsx
index 5a5c1b07..4079d31d 100644
--- a/web/src/pages/dashboard/review-detail.tsx
+++ b/web/src/pages/dashboard/review-detail.tsx
@@ -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}