feat: add open library page#80
Open
jordansilly77-stack wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough新增 Library 页面（ Changes图书馆页面新增
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Filter as 筛选输入
participant State as 状态管理
participant Render as 页面渲染
User->>Filter: 输入关键字/分类/状态
Filter->>State: 更新 keyword/category/status
State->>State: 计算 filteredBooks 与 activeBook
State->>Render: 触发 observer 重新渲染
Render->>User: 更新卡片列表与详情面板
User->>Render: 点击卡片或选择下拉项
Render->>State: 更新 selectedCode
State->>Render: activeBook 转换，重新渲染详情
Render->>User: 展示新的 activeBook 详情与按钮文案
预估代码审查难度🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@pages/library/index.tsx`:
- Line 13: The category field uses hardcoded English text that bypasses i18n,
violating the requirement that all user-facing text must use the t() function
for internationalization. Change the category property type from string to
I18nKey, update the hardcoded English category values at the data definition
points to appropriate i18n keys, and modify all rendering locations where
category is displayed to wrap the value with the t() translation function. This
applies to the category property declaration, the hardcoded values in the data
objects, and the rendering logic in the template where categoryKey is output
directly.
- Line 116: The component violates list semantics and styling conventions in two
places. First, the ul element with className={styles.heroMeta} at line 116 is
missing the list-unstyled class to properly style the first-level list. Add the
list-unstyled class to this ul element's className. Second, the steps section
around lines 249-255 is incorrectly implemented using div elements with manual
numbering instead of semantic HTML. Replace this div-based structure with proper
ordered list elements (ol wrapping li items) to represent the countable steps
correctly, removing any manual numbering from the content.
- Around line 104-106: Replace all native HTML tags in the
pages/library/index.tsx file with React Bootstrap components to comply with the
coding guidelines. Starting with the diff shown (the h1, p, and flex div
elements around lines 104-106), replace heading tags (h1, h2, h3) with React
Bootstrap typography utilities or Card.Header, replace p tags with Card.Text or
typography utilities, replace container divs with Stack or Container components,
replace ul/li lists with ListGroup/ListGroup.Item, and replace
section/article/aside tags with Card or appropriate layout components. Apply
these replacements consistently across all 40+ native HTML tag instances
throughout the file, including the sections around lines 116-129 and 191-195 and
any other native elements found.
🪄 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
Run ID: ccbfe341-040a-4109-8b7e-ea37ce02e5c5
⛔ Files ignored due to path filters (3)
translation/en-US.tsis excluded by none and included by nonetranslation/zh-CN.tsis excluded by none and included by nonetranslation/zh-TW.tsis excluded by none and included by none
📒 Files selected for processing (3)
components/Navigator/MainNavigator.tsxpages/library/index.module.lesspages/library/index.tsx
Comment on lines
+104
to
+106
| <h1 className="display-5 fw-bold mb-3">{t('library_hero_title')}</h1> | ||
| <p className="fs-5 text-muted mb-4">{t('library_hero_intro')}</p> | ||
| <div className="d-flex flex-wrap gap-3"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 只读核查：快速定位 library 页面中的常见原生 UI 标签，评估是否需要批量替换为 RB 组件
rg -nP '<(h1|h2|h3|p|ul|li|strong|small|article|aside|section|header)\b' pages/library/index.tsxRepository: Open-Source-Bazaar/Open-Source-Bazaar.github.io
Length of output: 2800
🏁 Script executed:
# 验证是否有更多原生标签需要关注
rg -c '<(h1|h2|h3|p|ul|li|strong|small|article|aside|section|header|nav)\b' pages/library/index.tsx | tail -5Repository: Open-Source-Bazaar/Open-Source-Bazaar.github.io
Length of output: 94
统一使用 React Bootstrap 组件替代原生 HTML 标签。
编码指南明确规定该文件路径（pages/**/*.tsx）必须"ALWAYS use React Bootstrap components instead of custom HTML elements"。当前代码中存在 40 多处原生标签（<h1>、<h2>、<h3>、<p>、<ul>、<li>、<strong>、<small>、<header>、<section>、<article>、<aside> 等），分布在多个区块（第 104–106 行、116–129 行、191–195 行等），需要统一重构。
建议迁移方案：
- 标题元素 → React Bootstrap 的
<Container>+ 排版 utility，或<Card.Header> - 列表 →
ListGroup/ListGroup.Item - 分段容器 →
Card/Stack等组件 - 文本元素 →
Card.Text或组件内原生标签（在 RB 组件上下文中）
这是强制性重构，需要在本页落地，以避免风格漂移。
🤖 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 `@pages/library/index.tsx` around lines 104 - 106, Replace all native HTML tags
in the pages/library/index.tsx file with React Bootstrap components to comply
with the coding guidelines. Starting with the diff shown (the h1, p, and flex
div elements around lines 104-106), replace heading tags (h1, h2, h3) with React
Bootstrap typography utilities or Card.Header, replace p tags with Card.Text or
typography utilities, replace container divs with Stack or Container components,
replace ul/li lists with ListGroup/ListGroup.Item, and replace
section/article/aside tags with Card or appropriate layout components. Apply
these replacements consistently across all 40+ native HTML tag instances
throughout the file, including the sections around lines 116-129 and 191-195 and
any other native elements found.
Source: Coding guidelines
jordansilly77-stack
force-pushed
the
feat/open-library-page
branch
from
2fe18ef to
23b764a
Compare
jordansilly77-stack
force-pushed
the
feat/open-library-page
branch
from
23b764a to
a6ae3e2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pages/library/index.module.less (2)
118-124: 💤 Low value可选优化:考虑使用更灵活的标签宽度。
第 119 行将标签宽度固定为
flex: 0 0 5rem。虽然对于当前的中英文标签应该足够,但考虑到国际化场景中某些语言的标签可能更长,建议考虑使用min-width替代固定宽度,或增加到6rem:&::before { flex: 0 0 auto; min-width: 5rem; // 或者 // flex: 0 0 6rem; content: attr(data-label); color: `#6c757d`; font-weight: 600; text-align: left; }这样可以在保持布局整齐的同时,为更长的标签文本提供更好的容错性。
🤖 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 `@pages/library/index.module.less` around lines 118 - 124, The `&::before` pseudo-element has a fixed flex width of 5rem which may not accommodate longer labels in internationalization scenarios. Replace the fixed flex width constraint with a more flexible approach by either changing to `flex: 0 0 auto` and adding `min-width: 5rem`, or increasing the flex width to 6rem. This allows longer label text to display properly while maintaining layout consistency.
70-78: ⚡ Quick win考虑移除
!important声明。这两个状态类都使用了
!important来覆盖样式。虽然在与 Bootstrap Badge 组件配合时可能需要提高优先级,但使用!important通常是代码异味,会降低样式的可维护性。建议尝试通过 CSS 模块的
:global组合或更具体的选择器来达到相同效果,例如:.statusAvailable { background: rgba(25, 135, 84, 0.1); color: `#198754`; border-color: transparent; }如果 Bootstrap 的优先级确实需要覆盖,可以在组件层面使用
className组合或style属性作为替代方案。🤖 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 `@pages/library/index.module.less` around lines 70 - 78, The statusAvailable and statusBorrowed classes in the LESS file currently use !important declarations to override Bootstrap Badge styles, which reduces maintainability. Remove the !important flags from both the background and color properties in these two classes. If the styles don't apply correctly without !important, instead handle the style overrides at the component level by using className combinations to increase CSS specificity, or apply inline styles directly to the component elements that use these status classes, rather than relying on !important in the stylesheet.
🤖 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.
Nitpick comments:
In `@pages/library/index.module.less`:
- Around line 118-124: The `&::before` pseudo-element has a fixed flex width of
5rem which may not accommodate longer labels in internationalization scenarios.
Replace the fixed flex width constraint with a more flexible approach by either
changing to `flex: 0 0 auto` and adding `min-width: 5rem`, or increasing the
flex width to 6rem. This allows longer label text to display properly while
maintaining layout consistency.
- Around line 70-78: The statusAvailable and statusBorrowed classes in the LESS
file currently use !important declarations to override Bootstrap Badge styles,
which reduces maintainability. Remove the !important flags from both the
background and color properties in these two classes. If the styles don't apply
correctly without !important, instead handle the style overrides at the
component level by using className combinations to increase CSS specificity, or
apply inline styles directly to the component elements that use these status
classes, rather than relying on !important in the stylesheet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a6d120d-9936-4f54-8da5-42664722bf4d
⛔ Files ignored due to path filters (3)
translation/en-US.tsis excluded by none and included by nonetranslation/zh-CN.tsis excluded by none and included by nonetranslation/zh-TW.tsis excluded by none and included by none
📒 Files selected for processing (3)
components/Navigator/MainNavigator.tsxpages/library/index.module.lesspages/library/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/Navigator/MainNavigator.tsx
- pages/library/index.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/libraryOpen Library page with book browsing, search, category/status filters, catalog table, detail panel, and borrowing guideChecks
pnpm exec tsc --noEmitpnpm exec eslint components/Navigator/MainNavigator.tsx pages/library/index.tsx translation/zh-CN.ts translation/zh-TW.ts translation/en-US.tsgit diff --checkhttp://localhost:3000/librarylocallyNote:
pnpm buildcurrently stops while collecting existing Lark-backed pages because local Lark app id/secret env vars are unavailable.Closes #15
Summary by CodeRabbit
发布说明
/library