Conversation
- 借用 WebView 內的 epub.js 擷取書名/作者/封面，書櫃改格狀卡片版面（比照網頁版）， 新增右上角刪除按鈕與最近閱讀/書名/進度排序控制項 - 修正閱讀進度用章節內頁碼算全書百分比、翻到任一章結尾就誤判成讀畢的問題， 改用 book.locations 產生全書位置索引算精確進度
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 44 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 EPUB metadata/cover extraction via a hidden WebView, persists cover images to sandbox storage, redesigns the library screen into a sortable grid with new BookCard and SortControl components, fixes reading-progress calculation to use epub.js locations-based percentage, and updates progress documentation. ChangesMetadata extraction, cover storage, and library grid/sort UI
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LibraryScreen
participant HiddenWebView
participant LibraryLib
User->>LibraryScreen: add book
LibraryScreen->>LibraryLib: addBook()
LibraryScreen->>LibraryScreen: refresh()
LibraryScreen->>HiddenWebView: extractMetaFor(record)
HiddenWebView->>HiddenWebView: load epub, read metadata/cover
HiddenWebView-->>LibraryScreen: metaExtracted / metaError
LibraryScreen->>LibraryLib: updateBookMeta(id, patch)
LibraryLib->>LibraryLib: saveCoverImage(base64)
LibraryScreen->>LibraryScreen: refresh()
sequenceDiagram
participant ReaderScreen
participant HiddenWebView
participant EpubJs
ReaderScreen->>HiddenWebView: display(cfi)
HiddenWebView->>EpubJs: rendition.display
HiddenWebView->>EpubJs: book.locations.generate(1024)
EpubJs-->>HiddenWebView: relocated event
HiddenWebView-->>ReaderScreen: relocated(percentage)
ReaderScreen->>ReaderScreen: updateProgress(id, percentage)
🚥 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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
mobile/components/BookCard.tsx (1)
22-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd accessibility role/label to the cover Pressable.
The card's primary tap target (open book) has no
accessibilityRole/accessibilityLabel, unlike the delete button. Screen-reader users won't get an announced "button" affordance for opening the book.♻️ Suggested fix
- <Pressable onPress={onPress} onLongPress={onDelete}> + <Pressable + onPress={onPress} + onLongPress={onDelete} + accessibilityRole="button" + accessibilityLabel={`打開《${record.title}》`} + >🤖 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/BookCard.tsx` around lines 22 - 53, The main book-open tap target in BookCard’s Pressable is missing accessibility semantics, so add an appropriate accessibilityRole and an accessibilityLabel to the Pressable that wraps the cover content. Use the existing book data in BookCard (such as record.title and possibly record.author) to describe the action clearly, and keep the delete action’s accessibility handling unchanged.mobile/components/SortControl.tsx (1)
11-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate color tokens across files.
PAPER_BG,INK_COLOR(#2a2420), andINK3_COLOR(#9a8f80) values are duplicated here, inmobile/app/(tabs)/index.tsx, and inline inmobile/components/BookCard.tsx. Consider extracting a sharedtheme.tswith these tokens to avoid drift as the design evolves.🤖 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/SortControl.tsx` around lines 11 - 15, The color token constants in SortControl are duplicated across multiple components, so centralize them in a shared theme module to prevent drift. Move the shared values like PAPER_BG, INK_COLOR, and INK3_COLOR into a common theme.ts and update SortControl, mobile/app/(tabs)/index.tsx, and BookCard to import the same tokens instead of defining inline copies. Keep the existing token names or consistent equivalents so the relevant style usage in these components remains easy to locate.mobile/app/(tabs)/index.tsx (1)
128-130: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnhandled rejection if
updateBookMetafails.
updateBookMeta(pending.id, patch).then(refresh)has no.catch(). If the underlyingAsyncStoragewrite throws, this becomes an unhandled promise rejection andrefresh()is silently skipped, leaving the UI unaware the update didn't persist.♻️ Suggested fix
- if (Object.keys(patch).length > 0) { - updateBookMeta(pending.id, patch).then(refresh); - } + if (Object.keys(patch).length > 0) { + updateBookMeta(pending.id, patch) + .then(refresh) + .catch((err) => __DEV__ && console.warn('[library] updateBookMeta failed', err)); + }🤖 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/app/`(tabs)/index.tsx around lines 128 - 130, The `updateBookMeta(pending.id, patch).then(refresh)` call in `index.tsx` can reject without being handled, which skips the UI refresh and leaves failures silent. Update this flow to handle the promise returned by `updateBookMeta` in the same block, adding a `.catch()` (or equivalent async/await try/catch) around the `updateBookMeta` call so `refresh()` only runs after a successful write and errors are surfaced through the existing update logic.mobile/lib/library.ts (1)
37-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHand-rolled base64 decode — consider a well-known library instead.
base64ToBytesreimplements base64 decoding with manual char-lookup and padding-tolerant indices. I traced the padding math (0/1/2 trailing=) and it's correct for well-formed input, but it silently assumesbase64is a pure base64 payload with nodata:image/...;base64,prefix — any such prefix would pass the character filter'sA-Za-z0-9+/characters from "image", "data", etc. and corrupt the decode without throwing. Sinceexpo-file-system's newFile.write()doesn't accept a base64 encoding option (confirmed via docs/changelog search), the custom decoder is filling a real gap, but a maintained library likejs-base64orbase64-jswould remove the risk of subtle edge-case bugs (URL-safe variants, whitespace) for near-zero cost.♻️ Example using js-base64
-const base64ToBytes = (base64: string): Uint8Array => { - const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'; - const clean = base64.replace(/[^A-Za-z0-9+/]/g, ''); - const bytes = new Uint8Array(Math.floor((clean.length * 3) / 4)); - let byteIndex = 0; - for (let i = 0; i < clean.length; i += 4) { - ... - } - return bytes.slice(0, byteIndex); -}; +import { Base64 } from 'js-base64'; +const base64ToBytes = (base64: string): Uint8Array => Base64.toUint8Array(base64);Also worth confirming: does
reader-web'sblobToBase64(a different layer in this PR stack) strip anydata:URL prefix before postingcoverBase64? If not,saveCoverImagewould write corrupted cover files.Also applies to: 139-158
🤖 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 37 - 64, The custom base64 decoder in `base64ToBytes` can corrupt inputs that include a `data:` URL prefix, so update the `saveCoverImage` flow to ensure `coverBase64` is stripped to raw base64 before decoding/writing. Either switch `base64ToBytes` to a maintained base64 library or harden it to reject/handle prefixed payloads, and confirm the upstream `blobToBase64` caller does not pass prefixed data through unchanged.
🤖 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 `@mobile/app/`(tabs)/index.tsx:
- Around line 33-38: `pendingExtractionRef` in `extractMetaFor` is a single
shared slot, so overlapping add/extract requests can overwrite each other and
misapply `metaExtracted` results. Update the flow to either prevent re-entrant
book adds while extraction is in flight or carry a unique request id through
`extractMeta` and `metaExtracted` so each WebView response is matched to the
correct book before resolving and writing metadata.
In `@mobile/reader-web/index.ts`:
- Around line 156-167: The relocated progress logic in index.ts is trusting
l.start.percentage too early, which can persist 0% before locations are ready.
Update the relocated handler to use the chapter-based fallback until
book.locations.generate() has completed or another explicit locations-ready
signal is available, and only then read l.start.percentage. Keep the fix
localized around the relocated/post flow and the updateProgress path so saved
progress is not overwritten by early events.
---
Nitpick comments:
In `@mobile/app/`(tabs)/index.tsx:
- Around line 128-130: The `updateBookMeta(pending.id, patch).then(refresh)`
call in `index.tsx` can reject without being handled, which skips the UI refresh
and leaves failures silent. Update this flow to handle the promise returned by
`updateBookMeta` in the same block, adding a `.catch()` (or equivalent
async/await try/catch) around the `updateBookMeta` call so `refresh()` only runs
after a successful write and errors are surfaced through the existing update
logic.
In `@mobile/components/BookCard.tsx`:
- Around line 22-53: The main book-open tap target in BookCard’s Pressable is
missing accessibility semantics, so add an appropriate accessibilityRole and an
accessibilityLabel to the Pressable that wraps the cover content. Use the
existing book data in BookCard (such as record.title and possibly record.author)
to describe the action clearly, and keep the delete action’s accessibility
handling unchanged.
In `@mobile/components/SortControl.tsx`:
- Around line 11-15: The color token constants in SortControl are duplicated
across multiple components, so centralize them in a shared theme module to
prevent drift. Move the shared values like PAPER_BG, INK_COLOR, and INK3_COLOR
into a common theme.ts and update SortControl, mobile/app/(tabs)/index.tsx, and
BookCard to import the same tokens instead of defining inline copies. Keep the
existing token names or consistent equivalents so the relevant style usage in
these components remains easy to locate.
In `@mobile/lib/library.ts`:
- Around line 37-64: The custom base64 decoder in `base64ToBytes` can corrupt
inputs that include a `data:` URL prefix, so update the `saveCoverImage` flow to
ensure `coverBase64` is stripped to raw base64 before decoding/writing. Either
switch `base64ToBytes` to a maintained base64 library or harden it to
reject/handle prefixed payloads, and confirm the upstream `blobToBase64` caller
does not pass prefixed data through unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6158326-7500-4d4e-a548-1c365da3e93d
📒 Files selected for processing (10)
RN_SETUP_GUIDE.mdmobile/app/(tabs)/index.tsxmobile/app/reader/[id].tsxmobile/components/BookCard.tsxmobile/components/SortControl.tsxmobile/lib/coverStyles.tsmobile/lib/library.tsmobile/lib/readerHtml.generated.tsmobile/lib/readerMessages.tsmobile/reader-web/index.ts
| // epubjs 的 Spine 型別定義沒有列出 length（實際上 unpack() 時有設定這個欄位），只能用 any 存取。 | ||
| const spineLength = (book.spine as any).length as number; | ||
| const total = typeof l.start.percentage === 'number' | ||
| ? l.start.percentage | ||
| : (l.start.index + (displayed.page - 1) / Math.max(displayed.total, 1)) / Math.max(spineLength, 1); | ||
|
|
||
| post({ | ||
| type: 'relocated', | ||
| cfi: l.start.cfi, | ||
| page: displayed.page, | ||
| total: displayed.total, | ||
| percentage: Math.max(0, Math.min(1, total)), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
epub.js relocated location start percentage undefined or 0 before locations.generate
💡 Result:
In epub.js, receiving an undefined or 0 value for location percentage or start location is a standard result of attempting to calculate progress before the book's locations have been indexed [1][2]. The library does not generate these locations automatically because the process is resource-intensive, as it requires scanning the entire book [1][3]. To resolve this, you must explicitly call book.locations.generate and wait for the returned promise to resolve before attempting to access location data [4][5]. Recommended Implementation: 1. Initialize the book and wait for the ready event [5]. 2. Check for previously saved locations (e.g., in localStorage) to improve load times [5]. 3. If no stored locations exist, call book.locations.generate [5]. 4. Only after the generation promise resolves, proceed with rendering the book or calculating location-based data like percentages [4][5]. Example pattern: book.ready.then(function{ var key = book.key + '-locations'; var stored = localStorage.getItem(key); if (stored) { return book.locations.load(stored); } else { // Generate locations (e.g., every 1600 characters) return book.locations.generate(1600); } }).then(function { // Now it is safe to use book.locations.percentageFromCfi // or other location-dependent methods }); Once locations are generated, methods like book.locations.percentageFromCfi(cfi) will return the correct progress [1][6][3]. If you call these methods before the generation process completes, the library cannot map the current CFI to the generated location index, resulting in 0 or undefined [1][2].
Citations:
- 1: percentageFromCfi always return 0 futurepress/epub.js#278
- 2: Total pages and redirect to a specific page futurepress/epub.js#1252
- 3: Getting current page and total number of pages futurepress/epub.js#744
- 4: How do I get the current location and the total locations? futurepress/epub.js#1224
- 5: https://context7.com/futurepress/epub.js/llms.txt
- 6: https://github.com/futurepress/epub.js/blob/f09089cf/examples/locations.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- mobile/reader-web/index.ts (around lines 140-180) ---'
sed -n '140,180p' mobile/reader-web/index.ts
echo
echo '--- mobile/app/reader/[id].tsx (around lines 50-90) ---'
sed -n '50,90p' mobile/app/reader/[id].tsx
echo
echo '--- epub.js / locations usage in repo ---'
rg -n "book\.locations|locations\.generate|percentageFromCfi|relocated|updateProgress" mobile -SRepository: Retsomm/TravelInTime
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- epub.js version references ---'
rg -n '"epubjs"|epub\.js|0\.3\.93' package.json package-lock.json yarn.lock pnpm-lock.yaml mobile -S
echo
echo '--- mobile/lib/library.ts ---'
sed -n '140,190p' mobile/lib/library.tsRepository: Retsomm/TravelInTime
Length of output: 50377
Use the chapter-based estimate until locations are ready
typeof l.start.percentage === 'number' also matches 0, so early relocated events can skip the fallback and persist 0% while book.locations.generate() is still running. Gate on a real locations-ready signal before trusting l.start.percentage; otherwise updateProgress() can overwrite the saved position with 0.
🤖 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/reader-web/index.ts` around lines 156 - 167, The relocated progress
logic in index.ts is trusting l.start.percentage too early, which can persist 0%
before locations are ready. Update the relocated handler to use the
chapter-based fallback until book.locations.generate() has completed or another
explicit locations-ready signal is available, and only then read
l.start.percentage. Keep the fix localized around the relocated/post flow and
the updateProgress path so saved progress is not overwritten by early events.
- 加書時用 addingRef 擋掉重疊觸發，避免共用的 pendingExtractionRef 被覆蓋導致 metadata 錯配 - updateBookMeta 補上 .catch()，避免寫入失敗時靜默跳過畫面刷新 - BookCard 封面 Pressable 補上 accessibilityRole/accessibilityLabel - 新增 lib/theme.ts 集中管理 SortControl 與 index.tsx 重複的色票常數 Co-Authored-By: Claude Sonnet 5 <[email protected]>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes