Conversation
部分直式排版的中文電子書內建 CSS 設定 writing-mode: vertical-rl， 與 App 固定的橫向分頁 flow 衝突導致版面錯亂；同時這類書本的 OPF 常帶 page-progression-direction="rtl"，epub.js 會據此自動把翻頁 方向設成右到左。分別在內容文件層級強制覆寫為橫排，並在 rendition 啟動後重建 direction/layout 為 ltr，讓文字排版與翻頁順序一致。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
比照 pwa-next 的修法，同步套用到 Electron 版閱讀器：直式排版書本 CSS 強制覆寫為橫排，並在 rendition 啟動後重建 direction/layout 為 ltr，讓翻頁順序與橫排文字一致。 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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe reader now resets unsaved book settings and forces horizontal, left-to-right EPUB layout. Web and mobile readers apply CSS and inline overrides to loaded content documents. Initial display waits for layout correction. The package version changes to 1.0.8. ChangesReader writing direction
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReaderEngine
participant epub.js
participant ContentDocument
ReaderEngine->>epub.js: Set layout direction to ltr
ReaderEngine->>epub.js: Rebuild rendition layout
ReaderEngine->>epub.js: Wait before initial display
epub.js->>ContentDocument: Load EPUB content
ReaderEngine->>ContentDocument: Apply writing-mode override
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 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 `@pwa-next/src/hooks/reader/useReaderEngine.ts`:
- Around line 313-321: The rendition direction is hardcoded to ltr while
readingDirection remains configurable, causing RTL navigation to disagree with
the rendition. In pwa-next/src/hooks/reader/useReaderEngine.ts lines 313-321 and
renderer/src/hooks/reader/useReaderEngine.ts lines 256-264, synchronize
globalLayoutProperties.direction, rendition.direction(), and rendition.layout()
with the current readingDirection, preserving consistent navigation reversal
behavior for RTL.
🪄 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: 9a0d3999-1213-41db-9215-1645c6ab116c
📒 Files selected for processing (5)
package.jsonpwa-next/src/components/Reader/readerStyles.tspwa-next/src/hooks/reader/useReaderEngine.tsrenderer/src/components/Reader/readerStyles.tsrenderer/src/hooks/reader/useReaderEngine.ts
pwa-next/renderer/mobile 三端在 rendition 啟動時都會把 epub.js 翻頁方向寫死 為 ltr（用來抵銷書本 OPF 宣告的直排 RTL），但使用者可在設定面板切換的 readingDirection（左→右／右→左）只用來決定鍵盤/手勢要呼叫 next 還是 prev， 沒有同步套用到 rendition，導致切成 RTL 後翻頁動畫方向與操作方向相反。 改為讀取當下的 readingDirection 設定值。 mobile 端另外補上直式書本內文 wrapper 也可能帶 writing-mode: vertical-rl !important 的逐元素 inline 覆寫（沿用 pwa-next/renderer 已有的做法），並重新 產生 readerHtml.generated.ts。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
強制橫排的直排書內文原本會跟著使用者設定的「翻頁方向」（ltr/rtl）一起套用到 epub.js 的 rendition.direction()/layout()，導致切成 rtl 時內文 CSS 排版 （多欄順序、text-align/bidi）跟著鏡射變成靠右、行內文字順序顛倒。但「翻頁方向」 在使用者認知裡只是按鈕/手勢要呼叫 next 還是 prev 的操作偏好，不該連動影響內文 排版本身。改為 rendition 內部 direction 永遠固定 ltr，翻頁方向偏好只透過 next()/prev() 呼叫端的按鈕/手勢對應實現，pwa-next/renderer/mobile 三端一致。 同時修正兩個相關問題： - rendition.direction()/layout() 與第一次 rendition.display() 之間沒有先後 保證，可能在開書當下觸發一次非預期的重新導頁（表現為強制翻頁、跳到看似章節 末頁的畫面），改為讓 display() 明確等待方向設定完成後才呼叫。 - pwa-next/renderer 開一本從未儲存過排版設定的書時，沒有把 readingDirection 等設定重設回預設值，會沿用同一個 session 裡前一本書留下的設定；新增 useReaderStore 的 resetToDefaults()，mobile 原本就有正確重設不受影響。 mobile 端重新產生 readerHtml.generated.ts。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pwa-next/src/store/useReaderStore.ts (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the reset script value from
DEFAULT_SETTINGS.
resetScripthardcodes'tc'.DEFAULT_SETTINGS.scriptholds the same value. Reference the constant so a default change stays in one place.♻️ Proposed refactor
- resetScript: () => set({ script: 'tc' }), + resetScript: () => set({ script: DEFAULT_SETTINGS.script }),🤖 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 `@pwa-next/src/store/useReaderStore.ts` at line 47, Update resetScript in the reader store to use DEFAULT_SETTINGS.script instead of the hardcoded 'tc' value, preserving the existing state reset behavior and centralizing the default.mobile/reader-web/readerStyles.ts (1)
131-143: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider limiting the inline traversal cost.
setInlineWritingModevisits every element underbodyand runs threesetPropertycalls on each.applyWritingModeOverrideruns the traversal twice per rendered document. The document already receives similar traversals fromsetInlineFontSizeand the other typography helpers. On low-end Android WebViews, long chapters make this cost visible during page rendering.Two options reduce the work:
- Restrict the traversal to elements whose computed
writing-modeis vertical, since only those conflict with the horizontal flow.- Share a single traversal between the writing-mode and typography helpers.
🤖 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/readerStyles.ts` around lines 131 - 143, Reduce the per-document traversal cost in setInlineWritingMode, which is invoked twice by applyWritingModeOverride, by avoiding unconditional updates to every body descendant. Prefer filtering elements using their computed writing-mode and only applying the three inline properties to elements currently using vertical writing; preserve the existing direction and horizontal-flow behavior for affected elements.pwa-next/src/hooks/reader/useReaderEngine.ts (1)
331-341: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe direction-forcing promise now blocks the initial display in all three readers. Each reader builds
forceReadingDirectionfromrendition.startedand awaits it before the firstdisplay(). None of the three isolates failures. Ifstartedrejects, or ifdirection()orlayout()throws, the rejection skips the display step and reaches the outer error handler, so the reader shows no content instead of the book with an uncorrected direction.
pwa-next/src/hooks/reader/useReaderEngine.ts#L331-L341: wrap thedirection()/layout()body intry/catchand attach.catchto the promise, matching the guards at lines 718 and 767.renderer/src/hooks/reader/useReaderEngine.ts#L274-L284: apply the sametry/catchand.catch, matching the guards at lines 628 and 644.mobile/reader-web/index.ts#L505-L515: apply the sametry/catchand.catchso theawaitat line 650 cannot divert control to thecatchat line 669.🤖 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 `@pwa-next/src/hooks/reader/useReaderEngine.ts` around lines 331 - 341, The forceReadingDirection promise must never prevent the initial display when direction setup fails. In pwa-next/src/hooks/reader/useReaderEngine.ts lines 331-341, renderer/src/hooks/reader/useReaderEngine.ts lines 274-284, and mobile/reader-web/index.ts lines 505-515, wrap the direction()/layout() operations in try/catch and attach a catch handler to rendition.started, matching the existing guards in each reader so failures are isolated and display proceeds with the uncorrected direction.
🤖 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/reader-web/index.ts`:
- Around line 648-652: Re-check the existing generation guard after await
forceReadingDirection and before rendition.display() in the book-loading flow.
If the generation is stale because a later loadBook call started, return without
displaying or applying initialAnnotations; preserve the existing guard behavior
used near lines 506 and 661.
In `@pwa-next/src/hooks/reader/useReaderEngine.ts`:
- Around line 604-608: The forceReadingDirection callbacks in
pwa-next/src/hooks/reader/useReaderEngine.ts:604-608 and
renderer/src/hooks/reader/useReaderEngine.ts:486-490 must first check destroyed
and return before calling loadProgress or display; add the same guard at both
sites while preserving the existing flow when the book remains active.
- Around line 256-261: In the no-saved-layout branch around
useReaderStore.getState().resetToDefaults(), immediately reset fontSizeRef,
lineHeightRef, and letterSpacingRef to their default values before
resetScanState() runs, so the first rendered document uses the new book’s
defaults rather than the previous book’s numeric layout settings.
---
Nitpick comments:
In `@mobile/reader-web/readerStyles.ts`:
- Around line 131-143: Reduce the per-document traversal cost in
setInlineWritingMode, which is invoked twice by applyWritingModeOverride, by
avoiding unconditional updates to every body descendant. Prefer filtering
elements using their computed writing-mode and only applying the three inline
properties to elements currently using vertical writing; preserve the existing
direction and horizontal-flow behavior for affected elements.
In `@pwa-next/src/hooks/reader/useReaderEngine.ts`:
- Around line 331-341: The forceReadingDirection promise must never prevent the
initial display when direction setup fails. In
pwa-next/src/hooks/reader/useReaderEngine.ts lines 331-341,
renderer/src/hooks/reader/useReaderEngine.ts lines 274-284, and
mobile/reader-web/index.ts lines 505-515, wrap the direction()/layout()
operations in try/catch and attach a catch handler to rendition.started,
matching the existing guards in each reader so failures are isolated and display
proceeds with the uncorrected direction.
In `@pwa-next/src/store/useReaderStore.ts`:
- Line 47: Update resetScript in the reader store to use DEFAULT_SETTINGS.script
instead of the hardcoded 'tc' value, preserving the existing state reset
behavior and centralizing the default.
🪄 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: cf634b63-f841-46ab-b340-881fc90a2c0c
⛔ Files ignored due to path filters (1)
mobile/lib/readerHtml.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (8)
mobile/reader-web/index.tsmobile/reader-web/readerStyles.tspwa-next/src/components/Reader/readerStyles.tspwa-next/src/hooks/reader/useReaderEngine.tspwa-next/src/store/useReaderStore.tsrenderer/src/components/Reader/readerStyles.tsrenderer/src/hooks/reader/useReaderEngine.tsrenderer/src/store/useReaderStore.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- renderer/src/components/Reader/readerStyles.ts
- pwa-next/src/components/Reader/readerStyles.ts
- 換書流程在 forceReadingDirection 之後補上 generation/destroyed 檢查， 避免對過期的 rendition 呼叫 display 或套用已卸載元件的狀態 - 開新書若無已存排版設定，補齊 fontSize/lineHeight/letterSpacing 三個 ref 的即時重設，避免第一次渲染沿用前一本書的數值 - forceReadingDirection 內部的 direction()/layout() 包上 try/catch， 失敗時仍讓書正常顯示，不再整段卡住 - resetScript 改用 DEFAULT_SETTINGS.script 避免與 resetToDefaults 分歧 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
Chores