Conversation
Co-Authored-By: Claude Sonnet 5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 24 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 (6)
📝 WalkthroughWalkthroughIntroduces a theme system (light/dark palettes, persisted preference, context/provider) applied across RN screens and components, extends reader message contracts, adds a WebView dark-mode/typography/script-conversion pipeline, a SettingsPanel UI, and a useTTS hook, wired into the reader screen, plus progress documentation updates. ChangesTheming, reader settings, and TTS
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReaderScreen
participant WebView
participant ReaderWebIndex
participant useTTS
ReaderScreen->>WebView: postMessage setDarkMode/setTypography
WebView->>ReaderWebIndex: handle inbound message
ReaderWebIndex->>ReaderWebIndex: apply overrides to iframe docs
ReaderWebIndex->>ReaderScreen: bookLanguageDetected(baseScript)
ReaderScreen->>WebView: postMessage getChapterText
ReaderWebIndex->>ReaderScreen: chapterText(text)
ReaderScreen->>useTTS: speak(text)
useTTS-->>ReaderScreen: onAllDone callback
ReaderScreen->>WebView: postMessage getChapterText (next)
🚥 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 |
新增齒輪按鈕開啟排版與語音設定面板,比照網頁版支援字體/字級/行距/字距/繁簡切換/ 左右閱讀方向,並用 expo-speech 實作簡化版 TTS(播放/暫停/重置/語速/語音選擇/睡眠計時)。 一併修正簡繁轉換寫死假設書本原文為繁體的 bug,改為偵測 epub metadata 的書本原始語言。 Co-Authored-By: Claude Sonnet 5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
mobile/components/ThemeProvider.tsx (1)
13-15: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd error handling; avoid initial light-theme flash for dark-mode users.
loadDarkModePreference()has no.catch, so a rejectedAsyncStorage.getItembecomes an unhandled promise rejection. Also, sincedarkModestarts asfalseand only updates once this async load resolves, users with a persisted dark preference will briefly see the light theme flash on every app launch.💡 Proposed fix
- const [darkMode, setDarkMode] = useState(false); + const [darkMode, setDarkMode] = useState(false); + const [themeLoaded, setThemeLoaded] = useState(false); useEffect(() => { - loadDarkModePreference().then(setDarkMode); + loadDarkModePreference() + .then(setDarkMode) + .catch((err) => console.warn('[theme] failed to load dark mode preference', err)) + .finally(() => setThemeLoaded(true)); }, []);Then gate rendering (or at least the initial reveal) on
themeLoadedwhere the provider is mounted.🤖 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/ThemeProvider.tsx` around lines 13 - 15, The ThemeProvider initialization currently loads dark mode without handling rejected promises and renders with a default light theme before the saved preference is known. Update the loadDarkModePreference flow in ThemeProvider to add error handling for the async storage read, and introduce a loaded state so rendering can be gated until the preference has finished loading. Use the existing useEffect, loadDarkModePreference, and setDarkMode logic to apply the persisted theme only after the initial async lookup completes.mobile/components/SettingsPanel.tsx (1)
87-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd accessibility labels/roles to remaining interactive controls.
Close/reset/play buttons have
accessibilityRole/accessibilityLabel, but the font-family list items, script/direction segmented toggles, voice chips, and sleep-timer presets don't. Worth aligning for screen-reader users.Also applies to: 106-123, 151-169, 221-227
🤖 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/SettingsPanel.tsx` around lines 87 - 104, The remaining interactive controls in SettingsPanel are missing screen-reader metadata, so add accessibilityRole and accessibilityLabel to the FONT_OPTIONS Pressable items as well as the script/direction segmented toggles, voice chips, and sleep-timer presets. Use the existing patterns from the close/reset/play buttons as the model, and place the labels on the Pressable components or equivalent interactive wrappers identified by FONT_OPTIONS and the related toggle/chip sections.
🤖 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/reader/`[id].tsx:
- Around line 167-178: The auto-continue flow in readNextAndContinue uses a
fixed 400ms delay after posting the next-page message, which can race with
slower page transitions and make requestChapterText() return stale or empty
text. Replace the timeout with an awaitable signal from the existing
handleMessage relocation flow, following the promise-based pattern already used
for getChapterText, so readNextAndContinue only calls requestChapterText() after
the relocated event confirms navigation is complete.
- Around line 158-165: requestChapterText currently leaves callers hanging
forever if the WebView never responds and it overwrites
chapterTextResolverRef.current on overlapping calls. Update requestChapterText
(and the chapterTextResolverRef handling used by
readNextAndContinue/handleTTSPlay) to track an in-flight request, reject or
ignore a second request while one is pending, and add a timeout that clears the
resolver and rejects the Promise if no chapterText arrives. Make sure the
response handler also clears any timeout and resolver state once the Promise
settles.
- Around line 105-111: The initial WebView sync in the `msg.type === 'ready'`
handler can send `setTypography` before `loadBookSettings` has finished, causing
a brief default-typography flash. Update the `ready` path in `reader/[id].tsx`
to gate the first `postMessage` for typography on `settingsLoadedRef.current`
(alongside `webviewReadyRef`), so `setTypography` is only sent once the async
settings load has completed. Keep the later effect that re-sends typography
state as the fallback, but make the initial ready handshake in the `ready`
branch consistent with the loaded settings state.
In `@mobile/lib/tts.ts`:
- Around line 165-174: The sleep timer state is updated in `handleSleepChange`,
but `speak()` in `useCallback` never starts the timer when playback begins, so a
preselected sleep duration is ignored if chosen before play. Update the `speak`
flow in `tts.ts` to initialize the sleep timer whenever `sleepMinutes` is
already set, using `startSleepTimer` from the playback start path rather than
only from `handleSleepChange`. Make sure `startSleepTimer` is defined before it
is referenced by `speak` to avoid a TDZ issue, and keep the existing
`playing`/`paused` behavior intact.
In `@mobile/reader-web/index.ts`:
- Around line 15-22: The contentDocs Set is only ever populated by
rendition.hooks.content.register and never cleaned up, so stale chapter
Documents keep accumulating. Update the reader flow in index.ts by adding a
rendition.hooks.unloaded handler that removes the unloaded Document from
contentDocs, using the same Document reference added in the content hook so
later theme/typography updates only iterate active iframe documents.
---
Nitpick comments:
In `@mobile/components/SettingsPanel.tsx`:
- Around line 87-104: The remaining interactive controls in SettingsPanel are
missing screen-reader metadata, so add accessibilityRole and accessibilityLabel
to the FONT_OPTIONS Pressable items as well as the script/direction segmented
toggles, voice chips, and sleep-timer presets. Use the existing patterns from
the close/reset/play buttons as the model, and place the labels on the Pressable
components or equivalent interactive wrappers identified by FONT_OPTIONS and the
related toggle/chip sections.
In `@mobile/components/ThemeProvider.tsx`:
- Around line 13-15: The ThemeProvider initialization currently loads dark mode
without handling rejected promises and renders with a default light theme before
the saved preference is known. Update the loadDarkModePreference flow in
ThemeProvider to add error handling for the async storage read, and introduce a
loaded state so rendering can be gated until the preference has finished
loading. Use the existing useEffect, loadDarkModePreference, and setDarkMode
logic to apply the persisted theme only after the initial async lookup
completes.
🪄 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: 2ac3ae52-b0e5-4a98-9d31-016c6316329e
⛔ Files ignored due to path filters (1)
mobile/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
RN_SETUP_GUIDE.mdmobile/app/(tabs)/_layout.tsxmobile/app/(tabs)/index.tsxmobile/app/(tabs)/settings.tsxmobile/app/_layout.tsxmobile/app/reader/[id].tsxmobile/components/BookCard.tsxmobile/components/SettingsPanel.tsxmobile/components/SortControl.tsxmobile/components/ThemeProvider.tsxmobile/lib/coverStyles.tsmobile/lib/readerHtml.generated.tsmobile/lib/readerMessages.tsmobile/lib/readerSettings.tsmobile/lib/theme.tsmobile/lib/tts.tsmobile/package.jsonmobile/reader-web/index.ts
💤 Files with no reviewable changes (1)
- mobile/lib/coverStyles.ts
| let darkMode = false; | ||
| let typography: TypographySettings = DEFAULT_TYPOGRAPHY; | ||
| // 書本本身原始使用的文字(依 epub metadata 的 language 判斷,比照網頁版 Reader.tsx 的 | ||
| // baseScriptRef)。轉換/還原都要拿這個當基準,不能寫死假設書本原始文字一定是繁體—— | ||
| // 上一版沒有偵測這個值,導致「書本原本就是簡體」時,切成「繁體」只會呼叫 restoreDoc() | ||
| // 還原成書本原本的簡體文字,並不會真的轉換成繁體。 | ||
| let baseScript: TypographySettings['script'] = 'tc'; | ||
| const contentDocs = new Set<Document>(); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does epub.js Rendition type definition include a hooks.unloaded Hook, and what arguments does it pass to registered callbacks?
💡 Result:
Yes, the epub.js Rendition object includes an unloaded hook, which is defined in the Rendition type definition [1][2][3]. This hook is intended for cleanup operations when content is being removed from the display [4]. When the unloaded hook is triggered, it passes the following arguments to registered callbacks: view: The view object associated with the content being unloaded [4]. rendition: The rendition instance [4]. The hook is commonly used to perform cleanup tasks, such as removing event listeners or clearing resources that were previously established during the content loading or rendering processes [4].
Citations:
- 1: https://github.com/futurepress/epub.js/blob/master/README.md
- 2: https://github.com/futurepress/epub.js/blob/master/types/rendition.d.ts
- 3: https://github.com/futurepress/epub.js/blob/f09089cf/README.md
- 4: https://deepwiki.com/futurepress/epub.js/5.3-hook-system
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the nearby implementation.
wc -l mobile/reader-web/index.ts
sed -n '1,220p' mobile/reader-web/index.ts
printf '\n---\n'
sed -n '220,420p' mobile/reader-web/index.ts
# Find all uses of contentDocs and unloaded hooks in the repo.
printf '\n=== contentDocs usages ===\n'
rg -n "contentDocs|hooks\.unloaded|hooks\.content|applyDarkOverride|setInlineFontSize|originalTexts" mobile/reader-web/index.ts
# If epub.js type definitions are vendored or referenced in the repo, inspect them.
printf '\n=== epub.js hook definitions ===\n'
rg -n "unloaded|content.register|hooks" -g '!*node_modules*' -g '!*dist*' -g '!*build*' .Repository: Retsomm/TravelInTime
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the package version that mobile/reader-web/index.ts is using.
printf '=== package files ===\n'
fd -a 'package.json|package-lock.json|pnpm-lock.yaml|yarn.lock' .
printf '\n=== epubjs references in package files ===\n'
rg -n '"epubjs"|epubjs@|futurepress/epub.js' package.json package-lock.json pnpm-lock.yaml yarn.lock
# Check for any in-repo use of unloaded hooks or cleanup patterns around epub content docs.
printf '\n=== unloaded hook / cleanup usages ===\n'
rg -n "hooks\.unloaded|contentDocs\.delete|unload|Section contents are being unloaded" mobile renderer pwaRepository: Retsomm/TravelInTime
Length of output: 621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the mobile package manifest and look for epubjs / hook cleanup references.
cat -n mobile/package.json
printf '\n=== epubjs / cleanup references in mobile ===\n'
rg -n "epubjs|hooks\.unloaded|contentDocs\.delete|Section contents are being unloaded" mobile
printf '\n=== nearby reader code in other packages for comparison ===\n'
rg -n "hooks\.unloaded|contentDocs\.delete|Section contents are being unloaded" renderer pwaRepository: Retsomm/TravelInTime
Length of output: 50378
🌐 Web query:
epubjs 0.3.93 hooks.unloaded callback arguments view document rendition types/rendition.d.ts
💡 Result:
In Epub.js version 0.3.93, the rendition.hooks.unloaded hook is triggered when a section's contents are being removed from the viewer [1][2]. Callback functions registered to this hook typically receive two arguments: view and rendition [3]. The view argument represents the specific view instance being unloaded, and rendition provides access to the main Rendition instance [3]. This hook is intended for performing cleanup operations, such as removing event listeners or disposing of resources created during the rendering process [3]. In the TypeScript type definitions (types/rendition.d.ts), the hooks are defined as part of the Rendition class structure [4][5]. While standard documentation for hook argument signatures is limited, the community-identified signature for unloaded in practical usage is (view, rendition) [6][3]. Developers are encouraged to return a Promise from the callback if the cleanup operation is asynchronous, ensuring the rendering pipeline waits for completion [1][2][3].
Citations:
- 1: https://npmx.dev/package/epubjs/v/0.3.93
- 2: https://github.com/futurepress/epub.js/
- 3: https://deepwiki.com/futurepress/epub.js/5.3-hook-system
- 4: https://github.com/futurepress/epub.js/blob/master/types/rendition.d.ts
- 5: https://github.com/futurepress/epub.js/blob/f09089cf/types/rendition.d.ts
- 6: https://gist.github.com/tuki0918/3a01033d06bb3b3935b5a8c10a17a650
Prune contentDocs on section unload. rendition.hooks.content.register(...) adds each chapter Document to the Set, but nothing removes it. Hook rendition.hooks.unloaded and delete the document there so theme/typography updates don’t keep traversing stale iframe documents from the whole reading session.
🤖 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 15 - 22, The contentDocs Set is only
ever populated by rendition.hooks.content.register and never cleaned up, so
stale chapter Documents keep accumulating. Update the reader flow in index.ts by
adding a rendition.hooks.unloaded handler that removes the unloaded Document
from contentDocs, using the same Document reference added in the content hook so
later theme/typography updates only iterate active iframe documents.
- reader/[id].tsx: requestChapterText 加逾時與防重疊保護,readNextAndContinue 改用等待 relocated 事件取代固定延遲,ready 訊息延後送出未載入完成的排版設定 - tts.ts: speak() 補上啟動預先選好的睡眠計時器 - reader-web/index.ts: 修正 contentDocs 從不清除的洩漏,改用 defaultView 判斷卸載 - SettingsPanel.tsx: 補齊字體/簡繁/方向/語音/睡眠計時選項的無障礙標籤 - ThemeProvider.tsx: 補上讀取深色模式偏好失敗時的錯誤處理 Co-Authored-By: Claude Sonnet 5 <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes