Conversation
- 頂部工具列新增書籤按鈕與清單面板（書籤/目錄/資訊/註記四分頁），皆比照網頁版對應功能 - 全面改用 react-native-svg 照抄網頁版線條圖示，取代原本的 Unicode 符號/emoji - 修正封面圖用絕對路徑持久化、App 重新安裝後失效的問題，改存檔名現算路徑 - 設定/清單面板改成點擊圖示開關的方式，移除多餘的關閉鈕與麵包屑列 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds bookmark and table-of-contents navigation to the mobile reader, including a new ChangesBookmarks, TOC, and Icons Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ReaderScreen as Reader Screen
participant WebView as reader-web (WebView)
participant ListPanel
User->>ReaderScreen: Tap list panel icon
ReaderScreen->>ListPanel: Render with bookmarks, toc, currentHref
User->>ListPanel: Select chapter/bookmark
ListPanel->>ReaderScreen: onNavigateChapter/onNavigateBookmark
ReaderScreen->>WebView: postMessage goto(target)
WebView->>WebView: rendition.display(target)
WebView->>ReaderScreen: relocated(href, chapterTitle, cfi)
ReaderScreen->>ReaderScreen: update currentCfi/currentHref/currentChapterTitle
ReaderScreen->>ReaderScreen: persist CFI/progress
sequenceDiagram
participant WebView as reader-web (WebView)
participant Book as epub.js Book
participant ReaderScreen as Reader Screen
WebView->>Book: load book.navigation.toc, book.spine.items
WebView->>WebView: build tocCache, spineHrefs
WebView->>ReaderScreen: postMessage tocLoaded(toc)
ReaderScreen->>ReaderScreen: setToc(toc)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (4)
mobile/components/icons.tsx (1)
11-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared SVG wrapper to reduce duplication.
Every icon repeats the same
<Svg width={size} height={size} viewBox="0 0 24 24" fill=... stroke=... strokeWidth=... strokeLinecap=... strokeLinejoin=...>boilerplate. A small shared wrapper component takingchildrenand the common props would remove this repetition across all ~13 icons and centralize future style tweaks.♻️ Example wrapper extraction
+const IconBase = ({ size = 18, color = '`#000`', filled = false, strokeWidth = 1.8, children }: IconProps & { filled?: boolean; strokeWidth?: number; children: React.ReactNode }) => ( + <Svg width={size} height={size} viewBox="0 0 24 24" fill={filled ? color : 'none'} stroke={color} strokeWidth={strokeWidth} strokeLinecap="round" strokeLinejoin="round"> + {children} + </Svg> +); + -export const IconBack = ({ size = 18, color = '`#000`' }: IconProps) => ( - <Svg width={size} height={size} viewBox="0 0 24 24" fill="none" stroke={color} strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round"> - <Polyline points="15 18 9 12 15 6" /> - </Svg> -); +export const IconBack = (props: IconProps) => ( + <IconBase {...props}> + <Polyline points="15 18 9 12 15 6" /> + </IconBase> +);🤖 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 `@mobile/components/icons.tsx` around lines 11 - 105, The icon components in IconBack, IconClose, IconSettings, and the other exports all repeat the same Svg boilerplate, so extract a shared wrapper component for the common width, height, viewBox, fill, stroke, strokeWidth, strokeLinecap, and strokeLinejoin props. Update the individual icon functions to render only their shape children through that wrapper, keeping size and color as inputs so future style changes are centralized and the repeated JSX is removed.mobile/lib/library.ts (3)
154-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrphaned cover file if extension changes across saves for the same book id.
saveCoverImageonly deletes/overwrites the file at the newly computed${id}.${coverExtension(mediaType)}path. If a later save for the sameidresolves to a different extension than a prior save, the previous cover file under the old extension is never deleted, leaking storage over time.🤖 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 `@mobile/lib/library.ts` around lines 154 - 162, The saveCoverImage helper leaves behind the previous cover file when the computed extension changes for the same book id. Update saveCoverImage to first clean up any existing cover files for that id in coversDir(), not just the newly named File, then recreate/write the current one so only one cover remains per id regardless of coverExtension(mediaType).
132-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate cover-filename derivation logic.
The
record.coverFilename ?? record.coverUri?.split('/').pop()fallback is duplicated inremoveBookandgetCoverUri. If this derivation logic changes in one place but not the other,removeBookcould fail to delete the correct cover file (or delete the wrong one) whilegetCoverUristill resolves it differently, silently leaking files or breaking display consistency.♻️ Extract a shared helper
+const resolveCoverFilename = (record: Pick<BookRecord, 'coverFilename' | 'coverUri'>): string | undefined => + record.coverFilename ?? record.coverUri?.split('/').pop(); + export const removeBook = (id: string) => updateMeta((records) => { const record = records.find((r) => r.id === id); if (record) { const file = new File(booksDir(), record.filename); if (file.exists) file.delete(); - const coverFilename = record.coverFilename ?? record.coverUri?.split('/').pop(); + const coverFilename = resolveCoverFilename(record); if (coverFilename) { const cover = new File(coversDir(), coverFilename); if (cover.exists) cover.delete(); } } ... }); export const getCoverUri = (record: BookRecord): string | null => { - const filename = record.coverFilename ?? record.coverUri?.split('/').pop(); + const filename = resolveCoverFilename(record); if (!filename) return null; const file = new File(coversDir(), filename); return file.exists ? file.uri : null; };Also applies to: 167-172
🤖 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 `@mobile/lib/library.ts` around lines 132 - 134, The cover filename fallback is duplicated between removeBook and getCoverUri in library.ts, so extract the shared derivation into a single helper and use it from both places. Create a helper near the existing record handling logic that returns the resolved cover filename from record.coverFilename or record.coverUri, then update removeBook and getCoverUri to call that helper so both paths stay consistent if the logic changes.
167-172: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
getCoverUriperforms a synchronous FS existence check on every call, including in render paths.
File.existstriggers a synchronous native call per invocation.getCoverUri(record)is invoked directly inBookCard's andListPanel's render bodies (see relevant snippets), meaning every re-render of every visible book card/bookinfo tab does a blocking disk stat, instead of the previous zero-costrecord.coverUrifield read. This is a regression in render-path cost, most noticeable in the library gridFlatList.Consider memoizing the result in the consuming components (e.g.,
useMemo(() => getCoverUri(record), [record.coverFilename, record.coverUri])) so the stat only re-runs when the cover reference actually changes.🤖 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 `@mobile/lib/library.ts` around lines 167 - 172, The issue is that getCoverUri does a synchronous File.exists check on every render-path call, which is too expensive in BookCard and ListPanel. Update the consuming components to memoize getCoverUri(record) with useMemo keyed by record.coverFilename and record.coverUri so the native stat only runs when the cover reference changes. Keep getCoverUri as the lookup helper, but avoid calling it directly inside render bodies.
🤖 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 `@mobile/components/icons.tsx`:
- Around line 11-105: The icon components in IconBack, IconClose, IconSettings,
and the other exports all repeat the same Svg boilerplate, so extract a shared
wrapper component for the common width, height, viewBox, fill, stroke,
strokeWidth, strokeLinecap, and strokeLinejoin props. Update the individual icon
functions to render only their shape children through that wrapper, keeping size
and color as inputs so future style changes are centralized and the repeated JSX
is removed.
In `@mobile/lib/library.ts`:
- Around line 154-162: The saveCoverImage helper leaves behind the previous
cover file when the computed extension changes for the same book id. Update
saveCoverImage to first clean up any existing cover files for that id in
coversDir(), not just the newly named File, then recreate/write the current one
so only one cover remains per id regardless of coverExtension(mediaType).
- Around line 132-134: The cover filename fallback is duplicated between
removeBook and getCoverUri in library.ts, so extract the shared derivation into
a single helper and use it from both places. Create a helper near the existing
record handling logic that returns the resolved cover filename from
record.coverFilename or record.coverUri, then update removeBook and getCoverUri
to call that helper so both paths stay consistent if the logic changes.
- Around line 167-172: The issue is that getCoverUri does a synchronous
File.exists check on every render-path call, which is too expensive in BookCard
and ListPanel. Update the consuming components to memoize getCoverUri(record)
with useMemo keyed by record.coverFilename and record.coverUri so the native
stat only runs when the cover reference changes. Keep getCoverUri as the lookup
helper, but avoid calling it directly inside render bodies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 893c7bf0-bc77-416b-8fd7-6518bd58a275
⛔ Files ignored due to path filters (1)
mobile/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
RN_SETUP_GUIDE.mdmobile/app/(tabs)/_layout.tsxmobile/app/(tabs)/index.tsxmobile/app/reader/[id].tsxmobile/components/BookCard.tsxmobile/components/ListPanel.tsxmobile/components/SettingsPanel.tsxmobile/components/icons.tsxmobile/lib/library.tsmobile/lib/readerHtml.generated.tsmobile/lib/readerMessages.tsmobile/package.jsonmobile/reader-web/index.ts
- saveCoverImage 換副檔名時清除舊封面檔，避免留下孤兒檔案 - 抽出共用的封面檔名回退邏輯，供 removeBook / getCoverUri 共用 - BookCard / ListPanel 用 useMemo 快取 getCoverUri，避免每次 render 都做檔案系統檢查 - icons.tsx 抽出共用 Svg 外殼元件，減少重複屬性 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes