Conversation
…alculation/data - 抽出 4 個純計算檔案與 4 個 hooks（useBookmarks/useChapterPageScan/useAnnotationPopups/useReaderEngine），Reader.tsx 從 1850 行降至 314 行 - 修正章節掃描背景渲染共用主 book 導致偶爾清空 annotation 的問題 - 修正朗讀跟隨高亮閃爍（移除搶跑的估算進度計時器，比照 pwa 已驗證修法） - 修正註記跳轉後不顯示（relocated 後補救重新注入遺漏的 SVG 底線） - 修正書庫排序按鈕缺少 no-drag class 導致點擊被視窗拖曳吃掉 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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: 30 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)
📝 WalkthroughWalkthroughThe PR extracts renderer and mobile reader behavior into dedicated hooks and shared helper modules for bookmarks, TOC lookup, page/progress calculations, TTS follow logic, annotations, typography, and WebView messaging. It also updates the annotation store API, removes TTS estimate-progress ticking, and includes minor manifest, UI, and progress-document changes. ChangesRenderer reader hooks refactor
Mobile reader refactor
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ReaderPage as Reader.tsx
participant ReaderEngine as useReaderEngine
participant ChapterScan as useChapterPageScan
participant AnnotationStore as useAnnotationStore
participant TTSFollow as ttsFollowCalculations
ReaderPage->>ReaderEngine: initialize book, refs, settings
ReaderEngine->>AnnotationStore: addAnnotation(annotation)
AnnotationStore-->>ReaderEngine: annotation id
ReaderEngine->>ChapterScan: triggerScan()
ChapterScan-->>ReaderEngine: setPageInfo(page, total)
ReaderEngine->>TTSFollow: shouldAdvanceTTSPage(...)
TTSFollow-->>ReaderEngine: decision + reason
ReaderEngine-->>ReaderPage: state and handlers
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (6)
renderer/src/components/Reader/BookmarkPanel.tsx (1)
35-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffInline
styleusage in touched JSX.The updated bookmark list markup still relies on inline
style={{...}}objects rather thanclassName. As per coding guidelines,**/*.tsx:React JSX 頁面/元件中應使用 className 寫法，並避免使用 inline style. This is a pre-existing pattern spanning the whole component, so a full conversion is out of scope here, but flagging since these lines were touched by the refactor.🤖 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 `@renderer/src/components/Reader/BookmarkPanel.tsx` around lines 35 - 41, The touched JSX in BookmarkPanel still uses inline style objects instead of className-based styling. Update the bookmark item markup in BookmarkPanel to use existing or new CSS/className hooks for layout, colors, spacing, hover state, and typography, and keep the inline style usage removed from the mapped bookmark row elements and their children.Source: Coding guidelines
renderer/src/hooks/reader/useBookmarks.ts (1)
15-25: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse functional state updates to avoid stale-closure races.
toggle/removereadbookmarksfrom the closure and then callsetBookmarks(next)directly. If either is invoked twice before a re-render (e.g., rapid double-click), the second call operates on stalebookmarks, silently dropping the first update. Using the functional updater form removes this class of bug.♻️ Proposed fix using functional updates
- const toggle = (cfi: string, label: string) => { - const next = toggleBookmark(bookmarks, cfi, label, crypto.randomUUID(), Date.now()) - saveBookmarks(bookId, next) - setBookmarks(next) - } - - const remove = (id: string) => { - const next = removeBookmarkById(bookmarks, id) - saveBookmarks(bookId, next) - setBookmarks(next) - } + const toggle = (cfi: string, label: string) => { + setBookmarks((prev) => { + const next = toggleBookmark(prev, cfi, label, crypto.randomUUID(), Date.now()) + saveBookmarks(bookId, next) + return next + }) + } + + const remove = (id: string) => { + setBookmarks((prev) => { + const next = removeBookmarkById(prev, id) + saveBookmarks(bookId, next) + return next + }) + }🤖 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 `@renderer/src/hooks/reader/useBookmarks.ts` around lines 15 - 25, The toggle and remove handlers in useBookmarks are reading bookmarks from a stale closure, which can drop rapid successive updates. Update the state changes to use the functional form of setBookmarks in both toggle and remove so each update is derived from the latest bookmarks value, and keep saveBookmarks in sync with the newly computed next state.renderer/src/components/Reader/bookmarkUtils.ts (1)
1-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for these pure helpers.
These are simple, side-effect-free functions (sort/remove/toggle/format) — ideal candidates for unit tests, and valuable given this PR is a behavior-preserving extraction from
Reader.tsx.🤖 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 `@renderer/src/components/Reader/bookmarkUtils.ts` around lines 1 - 17, Add unit tests for the pure helpers in bookmarkUtils.ts: sortBookmarksByAddedAt, removeBookmarkById, toggleBookmark, and formatBookmarkDate. Cover the key behaviors in these functions from Reader.tsx extraction, including sorting by addedAt, filtering by id, toggling by cfi (remove existing or append new bookmark), and formatting the date output.renderer/src/hooks/reader/useAnnotationPopups.ts (1)
19-19: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSelecting the whole store causes needless re-renders.
useAnnotationStore()with no selector subscribes to the entire store, so anyset()call anywhere in the app (add/update/remove annotation) re-renders this hook's consumer (Reader.tsx), even though only stable action functions are used here.⚡ Proposed fix using per-field selectors
- const { addAnnotation, updateColor, removeAnnotation } = useAnnotationStore() + const addAnnotation = useAnnotationStore((s) => s.addAnnotation) + const updateColor = useAnnotationStore((s) => s.updateColor) + const removeAnnotation = useAnnotationStore((s) => s.removeAnnotation)🤖 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 `@renderer/src/hooks/reader/useAnnotationPopups.ts` at line 19, The useAnnotationPopups hook is subscribing to the full annotation store, which causes Reader.tsx to re-render on unrelated store changes. Update the useAnnotationStore call to use per-field selectors for only the needed action functions, and keep the symbols addAnnotation, updateColor, and removeAnnotation as the selected values so the hook no longer depends on the entire store.renderer/src/hooks/useTTS.ts (1)
94-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm
'estimate'progress source is fully retired.
startProgressTimerno longer starts an interval and is now just astopProgressTimer()call plus a_textLengthunused parameter — consistent with removing the periodic estimate-progress emission. This makesemitProgress(..., 'estimate')(and theTTSProgressSourceunion's'estimate'member consumed downstream inttsFollowCalculations.ts'sshouldAdvanceTTSPage) effectively unreachable if no other code path still emits it.Worth confirming no other caller (e.g., a mobile/PWA-shared path) still relies on
'estimate', then either drop the type member or simplify the now-single-branch consumer logic.🤖 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 `@renderer/src/hooks/useTTS.ts` around lines 94 - 105, Confirm that the estimate-based progress path is fully removed: `startProgressTimer` in `useTTS` now only stops the timer, so verify no other code still calls `emitProgress(..., 'estimate')` or depends on the `'estimate'` member of `TTSProgressSource`. If it is truly unreachable, remove the unused `_textLength` parameter and clean up downstream consumers like `shouldAdvanceTTSPage` in `ttsFollowCalculations.ts` to drop the dead branch and simplify the source union.renderer/src/components/Reader/ttsFollowCalculations.ts (1)
40-47: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the dead
'estimate'path
useTTS.tsonly emits'boundary', so this special-case can be folded into the generic threshold andTTSProgressSourcecan likely be narrowed to'boundary'only.🤖 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 `@renderer/src/components/Reader/ttsFollowCalculations.ts` around lines 40 - 47, The TTS progress calculation still carries an unused special-case for the 'estimate' source, but useTTS only emits 'boundary'. Update ttsFollowCalculations’s progressShouldAdvance logic to remove the dead source-specific branch and rely on the generic threshold path, and narrow TTSProgressSource to 'boundary' only wherever it is defined or consumed so the types match the actual emitted values.
🤖 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 `@renderer/src/components/Reader/progressCalculations.ts`:
- Around line 1-4: The average calculation in computeChapterAverage can divide
by zero when the known map is empty, producing NaN that later contaminates
computeAccurateTotal and computeGlobalPage. Update computeChapterAverage to
handle an empty knownValues array explicitly and return a safe fallback value
instead of dividing by length, so pageInfo never receives NaN.
In `@renderer/src/hooks/reader/useAnnotationPopups.ts`:
- Around line 55-65: The delayed fallback in useAnnotationPopups is unguarded,
so the setTimeout callback can throw if the rendition is torn down before it
runs. Wrap the body of the timer callback in try/catch (similar to the
annotations.add handling) and safely skip when rendition/views are no longer
available, especially around rendition.views(), annApi.clear, and annApi.inject.
- Around line 68-77: The pending underline flow in addPendingAnnotation is
currently dead because it is never used anywhere in renderer. Either wire
addPendingAnnotation into useReaderEngine so pending annotations are actually
added and cleared through pendingAnnotationCfiRef, or remove the
pending-annotation state and related underline handling from useAnnotationPopups
if this behavior is no longer needed. Keep the fix scoped to the
addPendingAnnotation hook and its consumers in renderer.
In `@renderer/src/hooks/reader/useReaderEngine.ts`:
- Around line 501-546: The cleanup returned by useReaderEngine is missing the
sleep timer teardown, so the interval created by startSleepTimer can keep
running after leaving a book or unmounting. Update the cleanup block to
explicitly clear the active sleep timer using the existing sleep timer handling
(for example the clearSleepTimer path or direct sleepIntervalRef.current
cleanup) alongside the other TTS reset refs. Make sure this happens in the same
cleanup section that already resets ttsAutoFollowUnlockTimerRef,
cancelScheduledTTSHighlight, and clearAllTTSHighlights so stale
setSleepRemaining updates and unexpected stop() calls cannot leak into the next
book session.
---
Nitpick comments:
In `@renderer/src/components/Reader/BookmarkPanel.tsx`:
- Around line 35-41: The touched JSX in BookmarkPanel still uses inline style
objects instead of className-based styling. Update the bookmark item markup in
BookmarkPanel to use existing or new CSS/className hooks for layout, colors,
spacing, hover state, and typography, and keep the inline style usage removed
from the mapped bookmark row elements and their children.
In `@renderer/src/components/Reader/bookmarkUtils.ts`:
- Around line 1-17: Add unit tests for the pure helpers in bookmarkUtils.ts:
sortBookmarksByAddedAt, removeBookmarkById, toggleBookmark, and
formatBookmarkDate. Cover the key behaviors in these functions from Reader.tsx
extraction, including sorting by addedAt, filtering by id, toggling by cfi
(remove existing or append new bookmark), and formatting the date output.
In `@renderer/src/components/Reader/ttsFollowCalculations.ts`:
- Around line 40-47: The TTS progress calculation still carries an unused
special-case for the 'estimate' source, but useTTS only emits 'boundary'. Update
ttsFollowCalculations’s progressShouldAdvance logic to remove the dead
source-specific branch and rely on the generic threshold path, and narrow
TTSProgressSource to 'boundary' only wherever it is defined or consumed so the
types match the actual emitted values.
In `@renderer/src/hooks/reader/useAnnotationPopups.ts`:
- Line 19: The useAnnotationPopups hook is subscribing to the full annotation
store, which causes Reader.tsx to re-render on unrelated store changes. Update
the useAnnotationStore call to use per-field selectors for only the needed
action functions, and keep the symbols addAnnotation, updateColor, and
removeAnnotation as the selected values so the hook no longer depends on the
entire store.
In `@renderer/src/hooks/reader/useBookmarks.ts`:
- Around line 15-25: The toggle and remove handlers in useBookmarks are reading
bookmarks from a stale closure, which can drop rapid successive updates. Update
the state changes to use the functional form of setBookmarks in both toggle and
remove so each update is derived from the latest bookmarks value, and keep
saveBookmarks in sync with the newly computed next state.
In `@renderer/src/hooks/useTTS.ts`:
- Around line 94-105: Confirm that the estimate-based progress path is fully
removed: `startProgressTimer` in `useTTS` now only stops the timer, so verify no
other code still calls `emitProgress(..., 'estimate')` or depends on the
`'estimate'` member of `TTSProgressSource`. If it is truly unreachable, remove
the unused `_textLength` parameter and clean up downstream consumers like
`shouldAdvanceTTSPage` in `ttsFollowCalculations.ts` to drop the dead branch and
simplify the source union.
🪄 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: c903795f-abd8-4777-9cba-fb6db74bdc7b
📒 Files selected for processing (15)
FP_REFACTOR_PROGRESS.mdpackage.jsonrenderer/src/components/Library/SortControl.tsxrenderer/src/components/Reader/BookmarkPanel.tsxrenderer/src/components/Reader/bookmarkUtils.tsrenderer/src/components/Reader/progressCalculations.tsrenderer/src/components/Reader/tocLookup.tsrenderer/src/components/Reader/ttsFollowCalculations.tsrenderer/src/hooks/reader/useAnnotationPopups.tsrenderer/src/hooks/reader/useBookmarks.tsrenderer/src/hooks/reader/useChapterPageScan.tsrenderer/src/hooks/reader/useReaderEngine.tsrenderer/src/hooks/useTTS.tsrenderer/src/page/Reader.tsxrenderer/src/store/useAnnotationStore.ts
- 修正 computeChapterAverage 空 map 除以零導致 NaN 汙染 pageInfo - addEpubAnnotation 延遲補渲染的 setTimeout 加上 try/catch，避免 rendition 已銷毀時拋錯 - 移除從未被呼叫的 addPendingAnnotation 死程式碼 - useReaderEngine 離開書本時補上清除睡眠計時器 interval - useAnnotationPopups 改用 per-field selector 訂閱 store，減少不必要重render - useBookmarks toggle/remove 改用函式型 setState 避免連續呼叫遺失更新 - 清除 TTS 'estimate' 死程式碼分支（實際只會 emit 'boundary'）
- RN 端 app/reader/[id].tsx（788→383 行）拆出 lib/reader/calculations.ts 純函數與 hooks/reader/（useReaderEngine/useBookmarks/useAnnotations/useTTSReading） - WebView 端 reader-web/index.ts（1132→692+ 行）拆出 tocLookup/progressCalculations/ ttsFollowCalculations/scriptConversion/readerStyles/annotationUtils - 修正註記標記從書籤/目錄/註記清單跳轉後不顯示的問題：epub.js manager 在目標為目前 已顯示章節時會跳過重繪 hook，改成 gotoTarget 後無條件強制 reinject 標記 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (4)
renderer/src/hooks/useTTS.ts (1)
48-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
progressTimerRef/stopProgressTimerare now dead code.
startProgressTimerno longer setsprogressTimerRef.currentto anything (line 101-105), sostopProgressTimer(called fromfinishPlayback,onend,onerror,stop,pause) will always see anullref and no-op. Consider removing the now-unusedprogressTimerRefand thestopProgressTimercalls/definition, or leave a clearer comment if this scaffolding is intentionally kept for a future re-introduction of timer-based fallback progress.Also applies to: 94-105
🤖 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 `@renderer/src/hooks/useTTS.ts` around lines 48 - 49, `progressTimerRef` and `stopProgressTimer` in `useTTS` are now unused because `startProgressTimer` no longer assigns an interval to the ref, so the cleanup path in `finishPlayback`, `onend`, `onerror`, `stop`, and `pause` is dead code. Remove the `progressTimerRef` state, delete the `stopProgressTimer` helper and its calls, and simplify `startProgressTimer` accordingly; if you intend to keep the scaffold for future fallback progress, add a clear comment near `startProgressTimer`/`stopProgressTimer` explaining that it is intentionally unused for now.renderer/src/hooks/reader/useAnnotationPopups.ts (1)
18-18: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the dead pending-underline cleanup path.
pendingAnnotationCfiRefis never assigned, soremovePendingAnnotationalways returns early and the exported ref is unused.🤖 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 `@renderer/src/hooks/reader/useAnnotationPopups.ts` at line 18, Remove the dead pending-underline cleanup path in useAnnotationPopups: pendingAnnotationCfiRef is never assigned, so removePendingAnnotation always exits early and the ref is unused. Update the hook by deleting the unused pendingAnnotationCfiRef state and removing or simplifying removePendingAnnotation so it no longer relies on that ref, while keeping any actual annotation popup cleanup logic intact.mobile/hooks/reader/useBookmarks.ts (1)
27-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove the
saveBookmarksside effect out of the state updater and handle its rejection.The updater passed to
setBookmarksperforms a side effect (saveBookmarks). State updaters should be pure — React (esp. Strict Mode in React 19) may invoke them more than once, causing duplicate writes. Also, unlikeuseAnnotationswheresaveAnnotations(...).catch(...)is used,saveBookmarkshere has no rejection handler, risking an unhandled promise rejection.♻️ Proposed refactor
const handleToggleBookmark = () => { if (!id || !currentCfi) return; const newBookmark: Bookmark = { id: generateId(), cfi: currentCfi, label: currentChapterTitle || '書籤', addedAt: Date.now(), }; - setBookmarks((prev) => { - const next = toggleBookmarkList(prev, currentCfi, newBookmark); - saveBookmarks(id, next); - return next; - }); + const next = toggleBookmarkList(bookmarks, currentCfi, newBookmark); + setBookmarks(next); + saveBookmarks(id, next).catch((err) => console.error('[reader] saveBookmarks 失敗', err)); }; const handleDeleteBookmark = (bookmarkId: string) => { if (!id) return; - setBookmarks((prev) => { - const next = removeBookmarkList(prev, bookmarkId); - saveBookmarks(id, next); - return next; - }); + const next = removeBookmarkList(bookmarks, bookmarkId); + setBookmarks(next); + saveBookmarks(id, next).catch((err) => console.error('[reader] saveBookmarks 失敗', 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/hooks/reader/useBookmarks.ts` around lines 27 - 41, The `saveBookmarks` call inside the `setBookmarks` updater in `handleAddBookmark` and `handleDeleteBookmark` is a side effect and should be moved out of the state updater so the updater remains pure. Compute the next bookmark list inside `setBookmarks`, then call `saveBookmarks(id, next)` after state derivation, following the pattern used in `useAnnotations`. Also add rejection handling to `saveBookmarks(...).catch(...)` to avoid unhandled promise rejections and to keep bookmark persistence errors handled consistently.mobile/hooks/reader/useReaderEngine.ts (1)
191-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the remaining
console.logcalls with__DEV__.Line 191 is gated by
__DEV__, but theselectionCleared,annotationTapped, anddebuglogs (Lines 197, 202, 208) are not. Thedebughandler in particular fires for every WebView debug message and will spam the production console. Gate them consistently for parity with thetextSelectedbranch.♻️ Proposed change
if (msg.type === 'selectionCleared') { - console.log('[reader] selectionCleared 收到'); + if (__DEV__) console.log('[reader] selectionCleared 收到'); setSelection(null); return; } if (msg.type === 'annotationTapped') { - console.log('[reader] annotationTapped 收到', msg.id); + if (__DEV__) console.log('[reader] annotationTapped 收到', msg.id); setSelection(null); setEditingAnnotationId(msg.id); return; } if (msg.type === 'debug') { - console.log('[reader-web debug]', msg.message); + if (__DEV__) console.log('[reader-web debug]', msg.message); return; }🤖 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/hooks/reader/useReaderEngine.ts` around lines 191 - 208, The remaining console logging in useReaderEngine’s message handler is inconsistent with the __DEV__-guarded textSelected branch and will leak logs in production. Update the selectionCleared, annotationTapped, and debug cases in the same message dispatch logic to only call console.log when __DEV__ is true, keeping logging behavior consistent across all branches and preventing noisy WebView debug output from reaching production.
🤖 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/hooks/reader/useBookmarks.ts`:
- Around line 27-41: The `saveBookmarks` call inside the `setBookmarks` updater
in `handleAddBookmark` and `handleDeleteBookmark` is a side effect and should be
moved out of the state updater so the updater remains pure. Compute the next
bookmark list inside `setBookmarks`, then call `saveBookmarks(id, next)` after
state derivation, following the pattern used in `useAnnotations`. Also add
rejection handling to `saveBookmarks(...).catch(...)` to avoid unhandled promise
rejections and to keep bookmark persistence errors handled consistently.
In `@mobile/hooks/reader/useReaderEngine.ts`:
- Around line 191-208: The remaining console logging in useReaderEngine’s
message handler is inconsistent with the __DEV__-guarded textSelected branch and
will leak logs in production. Update the selectionCleared, annotationTapped, and
debug cases in the same message dispatch logic to only call console.log when
__DEV__ is true, keeping logging behavior consistent across all branches and
preventing noisy WebView debug output from reaching production.
In `@renderer/src/hooks/reader/useAnnotationPopups.ts`:
- Line 18: Remove the dead pending-underline cleanup path in
useAnnotationPopups: pendingAnnotationCfiRef is never assigned, so
removePendingAnnotation always exits early and the ref is unused. Update the
hook by deleting the unused pendingAnnotationCfiRef state and removing or
simplifying removePendingAnnotation so it no longer relies on that ref, while
keeping any actual annotation popup cleanup logic intact.
In `@renderer/src/hooks/useTTS.ts`:
- Around line 48-49: `progressTimerRef` and `stopProgressTimer` in `useTTS` are
now unused because `startProgressTimer` no longer assigns an interval to the
ref, so the cleanup path in `finishPlayback`, `onend`, `onerror`, `stop`, and
`pause` is dead code. Remove the `progressTimerRef` state, delete the
`stopProgressTimer` helper and its calls, and simplify `startProgressTimer`
accordingly; if you intend to keep the scaffold for future fallback progress,
add a clear comment near `startProgressTimer`/`stopProgressTimer` explaining
that it is intentionally unused for now.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 626db183-cd5f-4963-b381-10d7a26deca1
📒 Files selected for processing (22)
FP_REFACTOR_PROGRESS.mdmobile/app/reader/[id].tsxmobile/hooks/reader/useAnnotations.tsmobile/hooks/reader/useBookmarks.tsmobile/hooks/reader/useReaderEngine.tsmobile/hooks/reader/useTTSReading.tsmobile/lib/reader/calculations.tsmobile/lib/readerHtml.generated.tsmobile/lib/tts.tsmobile/reader-web/annotationUtils.tsmobile/reader-web/index.tsmobile/reader-web/progressCalculations.tsmobile/reader-web/readerStyles.tsmobile/reader-web/scriptConversion.tsmobile/reader-web/tocLookup.tsmobile/reader-web/ttsFollowCalculations.tsrenderer/src/components/Reader/progressCalculations.tsrenderer/src/components/Reader/ttsFollowCalculations.tsrenderer/src/hooks/reader/useAnnotationPopups.tsrenderer/src/hooks/reader/useBookmarks.tsrenderer/src/hooks/reader/useReaderEngine.tsrenderer/src/hooks/useTTS.ts
✅ Files skipped from review due to trivial changes (1)
- FP_REFACTOR_PROGRESS.md
🚧 Files skipped from review as they are similar to previous changes (4)
- renderer/src/components/Reader/progressCalculations.ts
- renderer/src/components/Reader/ttsFollowCalculations.ts
- renderer/src/hooks/reader/useReaderEngine.ts
- renderer/src/hooks/reader/useBookmarks.ts
- useBookmarks: saveBookmarks 移出 setState updater 並補上 rejection handling - useReaderEngine (mobile): selectionCleared/annotationTapped/debug log 補 __DEV__ 判斷 - useAnnotationPopups: 移除從未被賦值的 pendingAnnotationCfiRef 與永遠是 no-op 的 removePendingAnnotation - useTTS: 移除已無作用的 progressTimerRef/stopProgressTimer 殘留程式碼 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 by CodeRabbit
New Features
Bug Fixes
Chores