feat(portal): 期刊详情页 /journals/[id] (PAP-44)#133
Conversation
消费 PAP-42 的 GET /portal/venues/{id} 端点,RSC 服务端取数实现期刊详情页。
核心决策:VenueDetail 类型贴 BE 扁平响应零映射;hi-fi 只取 UI 不照搬 mock 结构;
缺失字段 FE 派生组装、推不出则隐藏;404→notFound / 失败→全局 error.tsx;
状态屏连全局一起落地供 PAP-45 复用;首页期刊卡接通、浏览全部留 PAP-47。
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- spec 对照真实代码校正 5 处(not-found/error 改 modify、子类型字段补全、 测试位置、封面调色板、坏 URL 校验)+ 派生视图对象/wire 类型澄清 - 新增 10 任务 TDD 实现 plan - v0.5 detail-pages hi-fi handoff 入库为快照(剔除 22M/1.3M standalone HTML) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
子代理驱动执行完成:10 个任务两阶段审查通过,data-status 全 done; notes 记录 4 change / 2 other(commitlint case / 去检索按钮 / Disclosure a11y / 面包屑 a11y / generateMetadata 延后 / e2e click-card 待 BE 环境复跑)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
app/error.tsx 是段级 error boundary,原导出名 GlobalError 易与 global-error.tsx 混淆;改名 RootError 匹配文件语义(最终审查 I-1)。纯重命名,行为不变。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Warning Review limit reached
More reviews will be available in 5 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthrough该 PR 实现了期刊详情页面 Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested labels
设计风险与隐患1. VenueDetail 数据契约中的扁平化快照字段与派生逻辑的耦合在 // 伪代码示意
jcr: jcrRatings.length
? { impactFactor: latest(jcrRatings)?.impactFactor, ... }
: { impactFactor: venue.impactFactor, ... }风险:当后端在更新期刊数据时,顶层快照与列表历史数据可能存在不一致(例如 list 中最新年份的值与 top-level 不同),派生逻辑没有明确的优先级说明("list 优先还是 snapshot 优先"),容易导致前端展示与后端意图不符的数据。建议在设计文档中明确数据来源优先级规则,或在类型注释中标注。 2. 深度嵌套的 DisclosureSection + 三态字段组合易产生空白区块在 风险:用户可能点击展开折叠区,期待看到内容但只看到空白,造成交互混淆。建议在区块级别(而非单字段级别)做条件渲染,或在空内容时渲染占位符提示(如"暂无数据")。 3. IdentifierChip 的剪贴板异常静默处理navigator.clipboard.writeText(value).catch(() => {
// 静默处理
})当剪贴板写入失败(例如 HTTPS 环境、权限拒绝),用户无法感知失败,可能重复点击复制按钮而不知道为何没有工作。 风险:无反馈的失败会降低可用性。建议在捕获异常后:(1) 至少在控制台记录错误,或 (2) 显示一个短时的 toast 提示("复制失败,请手动复制"),或 (3) 降级到 4. RatingTable 中 CAS 分类的"无数据"占位仅在客户端判断{cas ? <System ...>...</System> : <div>暂无中科院分区数据</div>}当 API 返回的 风险:用户可能误解为期刊不支持 CAS 分区,而实际可能是数据未采集。建议区分两种状态:(1) API 返回空列表 → "暂未收录 CAS 分区数据",(2) API 返回的 ranking=null 或特殊标记 → "该期刊不在 CAS 分区范围内"。 5. loading.tsx 的骨架占位与实际布局的适配风险
风险:移动端用户看到的骨架布局(单列)与桌面端(多列)可能差异较大,且骨架消失后 DOM 结构重排会导致 CLS(Cumulative Layout Shift)。建议将响应式断点的逻辑抽成常量共享给 6. error.tsx 中 RootError 去掉 error 参数后的调试困难新的全局错误处理器 风险:团队成员在本地调试时,如果网关返回 5xx,只能通过浏览器 DevTools 网络标签查看响应,容易漏掉某些错误排查。建议:(1) 在开发模式下条件性地展示 error message,或 (2) 始终将完整 error 信息输出到控制台(不在 UI 中展示)。 7. JournalCoverCard 链接化后的无障碍性
风险:视障用户可能无法快速理解卡片内容的层级与关键信息。建议在 Link 中添加 其他观察
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
patra-portal/tests/e2e/journal-detail.spec.ts (1)
11-11: ⚡ Quick win测试断言过于宽松,缺乏有效验证。
第 11 行使用
/.+/匹配任意非空文本,无法验证详情页是否渲染了正确的期刊标题。建议改为检查具体字段(如期刊名)或至少断言标题长度 > 5 字符以排除占位符。♻️ 建议加强断言
- await expect(page.getByRole("heading", { level: 1 })).toContainText(/.+/); + const h1 = page.getByRole("heading", { level: 1 }); + await expect(h1).toBeVisible(); + await expect(h1).not.toHaveText(""); + await expect(h1).not.toHaveText(/^(loading|—|N\/A)$/i);或如果能访问到路由参数,可断言 h1 包含实际期刊名。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patra-portal/tests/e2e/journal-detail.spec.ts` at line 11, The assertion using page.getByRole("heading", { level: 1 }) with /.+/ is too permissive; update the test in journal-detail.spec.ts to assert a concrete value or at least a stronger property: either verify the H1 contains the expected journal title (if you can access the route/fixture value) or assert the heading text length is greater than 5 characters to avoid matching placeholders; locate the assertion around page.getByRole("heading", { level: 1 }) and replace the regex check with a targeted check against the known journal name or a length-based check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@patra-portal/src/app/error.tsx`:
- Around line 6-11: The RootError component's param type declares an unused
parameter `error: Error & { digest?: string }` while the function only
destructures `reset`, causing a TypeScript strict-mode unused-parameter error;
fix by removing `error` from the parameter type in the RootError signature (or
alternatively keep it but rename to a prefixed unused name like `_error` in the
type) so the function signature and destructuring match (update the RootError
function parameter/type declaration accordingly).
In `@patra-portal/src/components/portal/DisclosureSection.tsx`:
- Around line 24-49: Replace Tailwind arbitrary value bracket syntax with the v4
parenthesis form throughout the component: change className usages like
border-[var(--border-default)], shadow-[var(--ring-focus)], text-[var(--fg-3)],
and border-[var(--border-subtle)] in DisclosureSection.tsx to the new
parenthesis form (e.g., border-(--border-default), shadow-(--ring-focus),
text-(--fg-3), border-(--border-subtle)); apply the same pattern to the other
listed components (JournalCoverCard, IdentifierChip, JournalRail, NotFoundState,
ErrorState, JournalPositioning, RatingTable, MetricBadge, JournalDetailView,
TrendChart, JournalMasthead, src/lib/portal-ui.ts,
app/journals/[id]/loading.tsx) so Tailwind v4 generates the styles correctly.
In `@patra-portal/src/components/portal/journal-detail/JournalMasthead.tsx`:
- Line 22: The Tailwind v3 arbitrary-value variable syntax is used in
JournalMasthead className strings (e.g., border-[var(--border-default)],
bg-[var(--cover-bg)], text-[var(--cover-ink)], text-[var(--fg-1/2/3)]); update
all such occurrences in this component to Tailwind v4 variable syntax by
replacing bracketed forms with parentheses (for example change
border-[var(--border-default)] to border-(--border-default),
bg-[var(--cover-bg)] to bg-(--cover-bg), text-[var(--fg-3)] to text-(--fg-3),
etc.) and verify every className in JournalMasthead that referenced var(...) is
converted similarly.
---
Nitpick comments:
In `@patra-portal/tests/e2e/journal-detail.spec.ts`:
- Line 11: The assertion using page.getByRole("heading", { level: 1 }) with /.+/
is too permissive; update the test in journal-detail.spec.ts to assert a
concrete value or at least a stronger property: either verify the H1 contains
the expected journal title (if you can access the route/fixture value) or assert
the heading text length is greater than 5 characters to avoid matching
placeholders; locate the assertion around page.getByRole("heading", { level: 1
}) and replace the regex check with a targeted check against the known journal
name or a length-based check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ce56276-438a-45ba-9206-a122939cb31b
⛔ Files ignored due to path filters (1)
docs/patra/design/snapshots/2026-06-06-detail-pages-hifi.zipis excluded by!**/*.zip
📒 Files selected for processing (34)
docs/patra/design/snapshots/README.mddocs/patra/plans/2026-06-06-pap-44-journal-detail-page.htmldocs/patra/specs/2026-06-05-portal-journal-detail-page-design.htmlpatra-portal/src/app/error.tsxpatra-portal/src/app/journals/[id]/loading.tsxpatra-portal/src/app/journals/[id]/not-found.tsxpatra-portal/src/app/journals/[id]/page.tsxpatra-portal/src/app/not-found.tsxpatra-portal/src/components/portal/DisclosureSection.tsxpatra-portal/src/components/portal/IdentifierChip.tsxpatra-portal/src/components/portal/JournalCoverCard.tsxpatra-portal/src/components/portal/journal-detail/JournalDetailView.tsxpatra-portal/src/components/portal/journal-detail/JournalMasthead.tsxpatra-portal/src/components/portal/journal-detail/JournalPositioning.tsxpatra-portal/src/components/portal/journal-detail/JournalRail.tsxpatra-portal/src/components/portal/journal-detail/MetricBadge.tsxpatra-portal/src/components/portal/journal-detail/RatingTable.tsxpatra-portal/src/components/portal/journal-detail/SectionEyebrow.tsxpatra-portal/src/components/portal/journal-detail/TrendChart.tsxpatra-portal/src/components/portal/status/ErrorState.tsxpatra-portal/src/components/portal/status/NotFoundState.tsxpatra-portal/src/lib/portal-api/cover-palette.tspatra-portal/src/lib/portal-api/venue-derive.tspatra-portal/src/lib/portal-api/venues.tspatra-portal/src/lib/portal-ui.tspatra-portal/src/types/portal.tspatra-portal/tests/components/portal/DisclosureSection.test.tsxpatra-portal/tests/components/portal/IdentifierChip.test.tsxpatra-portal/tests/components/portal/JournalCoverCard.test.tsxpatra-portal/tests/components/portal/JournalMasthead.test.tsxpatra-portal/tests/components/portal/status-screens.test.tsxpatra-portal/tests/e2e/journal-detail.spec.tspatra-portal/tests/lib/portal-api/venue-derive.test.tspatra-portal/tests/lib/portal-api/venues.test.ts
| export default function RootError({ | ||
| reset, | ||
| }: { | ||
| error: Error & { digest?: string }; | ||
| reset: () => void; | ||
| }) { |
There was a problem hiding this comment.
TypeScript strict 模式下类型签名不匹配。
函数签名在类型中声明了 error: Error & { digest?: string } 参数(第 9 行),但解构时只提取了 reset(第 7 行)。按设计文档意图,组件不再使用 error.message 避免信息泄漏,但这会导致 TypeScript strict 模式报错(未使用的类型参数)。
🔧 两种修复方案
方案 1(推荐):从类型签名中移除 error:
export default function RootError({
reset,
}: {
- error: Error & { digest?: string };
reset: () => void;
})方案 2:保留类型但用下划线前缀标记未使用:
export default function RootError({
reset,
+ error: _error,
}: {
error: Error & { digest?: string };
reset: () => void;
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function RootError({ | |
| reset, | |
| }: { | |
| error: Error & { digest?: string }; | |
| reset: () => void; | |
| }) { | |
| export default function RootError({ | |
| reset, | |
| }: { | |
| reset: () => void; | |
| }) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@patra-portal/src/app/error.tsx` around lines 6 - 11, The RootError
component's param type declares an unused parameter `error: Error & { digest?:
string }` while the function only destructures `reset`, causing a TypeScript
strict-mode unused-parameter error; fix by removing `error` from the parameter
type in the RootError signature (or alternatively keep it but rename to a
prefixed unused name like `_error` in the type) so the function signature and
destructuring match (update the RootError function parameter/type declaration
accordingly).
Review 反馈处理(CodeRabbit)逐条核验(依 1.
|
把新组件里的 `*-[var(--token)]` 全部改为 v4 惯用简写 `*-(--token)`(15 文件)。 两种写法功能等价(编译产物一致,border-(--x) 正确解析为 border-color,无歧义), 但 (--x) 是 Tailwind v4 推荐形式,作为项目首个使用 CSS 变量任意值的代码确立标准。 已重编译核验 + lint/typecheck/test 全绿。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
补充处理:Tailwind CSS 变量语法 → 已采纳 v4
|
portal-ci 的 e2e job 无后端(continue-on-error、不在 required-check),约定 portal e2e 须 backend-independent(参 homepage.spec.ts)。「首页点卡→详情」依赖 RSC fetchVenues 取真实期刊(服务端取数无法浏览器侧 mock),CI 无后端→首页无卡→失败。 改为无卡即 test.skip(前置=后端可达):CI skip 转绿,有后端的本地/staging 仍跑。 not-found 用例不依赖后端、始终执行。本地验证:1 skipped + 3 passed,exit 0。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
摘要
/journals/[id](RSC 服务端取数,消费 PAP-42GET /patra-catalog/portal/venues/{id}):masthead + 影响力速览 + 渐进式披露折叠区 + 双栏布局,像素级复刻 v0.5 hi-fiVenueDetail扁平贴 BE、零运行时映射;venue-derive.ts纯函数把扁平响应派生为组件视图对象(最新年评级重组 / 学科标签 / 速览卡资格 / 全程降级)NotFoundState(RSC)/ErrorState(client),重构已存在的全局not-found.tsx/error.tsx(顺带修掉原 error.tsx 直接渲染error.message的信息泄漏);非数字/404 →notFound(),5xx → 全局error.tsx/journals/[id]跳转("浏览全部期刊"保留 disabled,留 PAP-47)DisclosureSection/IdentifierChip/cover-palette/portal-ui供 PAP-45 文献详情页复用实现方式
子代理驱动开发:10 任务逐个两阶段审查(规格合规 + 代码质量)+ 最终整体审查,全部通过。详见
docs/patra/plans/2026-06-06-pap-44-journal-detail-page.html(含实施 notes)。测试计划
pnpm lint && pnpm typecheck && pnpm test全绿(26 文件 / 112 单测)PATRA_GATEWAY_BASE_URL指向可达 BE 复跑(本地执行环境后端不可达,spec 逻辑已就绪)已知 follow-up(本版有意不做)
generateMetadata(tab 标题 / SEO):因fetchVenueDetail用cache:"no-store",需 Reactcache()包装取数去重后再加,留独立改进globals.css @theme未映射--color-fg-*/--color-border-*语义 token(既有text-fg-3等为 no-op 死类);本 PR 新组件已统一用*-[var(--…)]arbitrary value 规避,根因修复(补 @theme)会改首页既有渲染,留单独评估🤖 Generated with Claude Code