feat(cbx-viewer): add (back) infinite and long-strip modes, fixed.#390
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🧰 Additional context used🔀 Multi-repo context grimmory-tools/grimmory-docsFindings for grimmory-tools/grimmory-docs [::grimmory-tools/grimmory-docs::]
No files in this documentation repo reference the new backend DTO field name stripMaxWidthPercent or the exact new strings; I found no direct code-level consumers or API/schema references to the new DTO in this repo. Conclusion: documentation updates are relevant but there are no discovered code consumers in this repo that will break. 📝 WalkthroughWalkthroughAdds a configurable strip max-width preference for CBX reader (infinite/long-strip modes) with per-book/global persistence; introduces a webtoon suggestion banner, continuation hint UI, storage helpers, quick settings UI/service, preferences wiring, reader rendering/scroll refinements, tests, backend user setting, and i18n strings. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as QuickSettings UI
participant QS as QuickSettingsService
participant Reader as CBX Reader Component
participant Storage as Local/Session Storage
participant PrefSvc as ReaderPreferencesService
User->>UI: adjust strip-width slider
UI->>QS: setStripMaxWidthPercent(value)
QS->>QS: clampStripMaxWidthPercent(value)
QS->>QS: emit stripMaxWidthChange$
QS->>Reader: stripMaxWidthChange$ (subscribed)
Reader->>Reader: apply width constraint to strip container
Reader->>Storage: writeStripWidthPercentPerBook(bookId, value)
Reader->>PrefSvc: updatePreference(['cbxReaderSetting','stripMaxWidthPercent'], value, { silent: true })
PrefSvc->>Storage: persist global preference via user settings
Storage-->>Reader: persistence result (ok / error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scss (1)
51-53: Redundantalign-items: centerdeclaration.The parent
.setting-label-rowalready setsalign-items: centerat line 40, so this modifier doesn't add any styling effect.♻️ Suggested simplification
- &.strip-width-row { - align-items: center; - }If you intend to override a different alignment in the future, consider keeping it; otherwise, this can be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scss` around lines 51 - 53, The nested modifier selector &.strip-width-row inside cbx-reader-preferences-component.scss redundantly declares align-items: center which is already set on the parent .setting-label-row; remove the align-items: center line from the &.strip-width-row rule (or if you actually intend to override a different alignment later, leave a comment explaining the intent) — locate the .setting-label-row rule and the &.strip-width-row block in the file and delete the redundant property.frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss (2)
541-576: Consider extracting shared continuation hint/spacer styles.The
.cbx-continuation-spacerand.cbx-continuation-hintrules are duplicated between&.infinite-scroll(lines 421-444) and&.long-strip(lines 552-576). This could be refactored into a shared rule set to reduce maintenance burden, but it's not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss` around lines 541 - 576, The .cbx-continuation-spacer and .cbx-continuation-hint rules are duplicated under the &.infinite-scroll and &.long-strip blocks; extract their shared declarations into a single shared rule (e.g., move common properties for .cbx-continuation-spacer and .cbx-continuation-hint out of those blocks into a top-level or parent selector used by both) and keep only any variant-specific overrides inside &.infinite-scroll and &.long-strip so both contexts reuse the same base styles (.cbx-continuation-spacer and .cbx-continuation-hint) while preserving the .visible modifier behavior.
477-482: Add empty line before comment per Stylelint rules.Static analysis flagged a missing empty line before the inline comment.
🔧 Proposed fix
.infinite-scroll-wrapper { align-items: center; + // Stable width so strip max-width % resolves against the reader scrollport, not max-content. width: 100%; min-width: 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss` around lines 477 - 482, The Stylelint error is caused by a missing blank line before the inline comment in the .infinite-scroll-wrapper block; open cbx-reader.component.scss, locate the .infinite-scroll-wrapper rule and insert an empty line immediately before the comment line ("// Stable width so strip max-width % resolves against the reader scrollport, not max-content.") so the comment is preceded by a blank line (or convert the inline comment to its own block comment with a preceding blank line) to satisfy the Stylelint rule.frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts (1)
603-611: Two subscriptions to the same observable could be consolidated.There are two separate subscriptions to
stripMaxWidthChange$: one for immediate state update (lines 603-607) and one debounced for persistence (lines 609-611). While this works correctly, it could be consolidated usingtap+debounceTimein a single pipe chain. However, the current approach is clear and functionally correct.💡 Optional consolidation
- this.quickSettingsService.stripMaxWidthChange$ - .pipe(takeUntil(this.destroy$)) - .subscribe((value) => { - this.stripMaxWidthPercent = value; - }); - - this.quickSettingsService.stripMaxWidthChange$ - .pipe(debounceTime(CbxReaderComponent.STRIP_WIDTH_PERSIST_DEBOUNCE_MS), takeUntil(this.destroy$)) - .subscribe((value) => this.persistStripMaxWidth(value)); + this.quickSettingsService.stripMaxWidthChange$ + .pipe( + tap((value) => this.stripMaxWidthPercent = value), + debounceTime(CbxReaderComponent.STRIP_WIDTH_PERSIST_DEBOUNCE_MS), + takeUntil(this.destroy$) + ) + .subscribe((value) => this.persistStripMaxWidth(value));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts` around lines 603 - 611, There are two subscriptions to stripMaxWidthChange$ updating state and persisting separately; consolidate them by piping stripMaxWidthChange$ through tap to set this.stripMaxWidthPercent and then debounceTime(CbxReaderComponent.STRIP_WIDTH_PERSIST_DEBOUNCE_MS) before calling persistStripMaxWidth, keeping takeUntil(this.destroy$) at the end to unsubscribe; update the subscription to use a single subscribe that handles both immediate state update (via tap) and debounced persistence (via the debounced operator) so you can remove the duplicate subscribe block.frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html (1)
111-112: Consider extracting complex template conditions to component getters.The conditions
longStripImages[longStripImages.length - 1].page >= pages.length - 1andinfiniteScrollPages[infiniteScrollPages.length - 1] >= pages.length - 1are repeated and could be extracted to component getters likeisAtEndOfLongStripandisAtEndOfInfiniteScrollfor improved readability. This is optional.Also applies to: 143-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html` around lines 111 - 112, Extract the repeated end-of-pages checks into boolean getters on the component (e.g., add isAtEndOfLongStrip and isAtEndOfInfiniteScroll in cbx-reader.component.ts) that encapsulate the logic using longStripImages, infiniteScrollPages, and pages (for example check lengths > 0 then compare last item's .page or value to pages.length - 1); then update the template conditions to use these getters (replace the complex expressions at the locations referencing longStripImages[...] and infiniteScrollPages[...] with isAtEndOfLongStrip and isAtEndOfInfiniteScroll respectively) to improve readability and avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.html`:
- Around line 39-43: The range input lacks a programmatic label; add an
accessible name by giving the visible label span (the one showing
'readerCbx.quickSettings.stripMaxWidth' via transloco) a unique id and reference
it from the range control using aria-labelledby (or alternatively add an
aria-label using the same translation key). Update the template around the input
and the span so the input (the element bound to state().stripMaxWidthPercent and
onStripMaxWidthInput) has aria-labelledby="yourUniqueId" (or aria-label set to
the translated string) to ensure the control is programmatically labeled.
In
`@frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.html`:
- Around line 99-110: The range slider lacks a programmatic label; add an
accessible name by either giving the existing span.setting-label (the span that
renders t('stripMaxWidth')) a unique id and set the input's aria-labelledby to
that id, or add an explicit aria-label on the input (e.g., "Strip max width") so
the input associated with selectedCbxStripMaxWidthPercent and
onStripMaxWidthInput is properly announced by assistive tech.
---
Nitpick comments:
In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html`:
- Around line 111-112: Extract the repeated end-of-pages checks into boolean
getters on the component (e.g., add isAtEndOfLongStrip and
isAtEndOfInfiniteScroll in cbx-reader.component.ts) that encapsulate the logic
using longStripImages, infiniteScrollPages, and pages (for example check lengths
> 0 then compare last item's .page or value to pages.length - 1); then update
the template conditions to use these getters (replace the complex expressions at
the locations referencing longStripImages[...] and infiniteScrollPages[...] with
isAtEndOfLongStrip and isAtEndOfInfiniteScroll respectively) to improve
readability and avoid duplication.
In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss`:
- Around line 541-576: The .cbx-continuation-spacer and .cbx-continuation-hint
rules are duplicated under the &.infinite-scroll and &.long-strip blocks;
extract their shared declarations into a single shared rule (e.g., move common
properties for .cbx-continuation-spacer and .cbx-continuation-hint out of those
blocks into a top-level or parent selector used by both) and keep only any
variant-specific overrides inside &.infinite-scroll and &.long-strip so both
contexts reuse the same base styles (.cbx-continuation-spacer and
.cbx-continuation-hint) while preserving the .visible modifier behavior.
- Around line 477-482: The Stylelint error is caused by a missing blank line
before the inline comment in the .infinite-scroll-wrapper block; open
cbx-reader.component.scss, locate the .infinite-scroll-wrapper rule and insert
an empty line immediately before the comment line ("// Stable width so strip
max-width % resolves against the reader scrollport, not max-content.") so the
comment is preceded by a blank line (or convert the inline comment to its own
block comment with a preceding blank line) to satisfy the Stylelint rule.
In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts`:
- Around line 603-611: There are two subscriptions to stripMaxWidthChange$
updating state and persisting separately; consolidate them by piping
stripMaxWidthChange$ through tap to set this.stripMaxWidthPercent and then
debounceTime(CbxReaderComponent.STRIP_WIDTH_PERSIST_DEBOUNCE_MS) before calling
persistStripMaxWidth, keeping takeUntil(this.destroy$) at the end to
unsubscribe; update the subscription to use a single subscribe that handles both
immediate state update (via tap) and debounced persistence (via the debounced
operator) so you can remove the duplicate subscribe block.
In
`@frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scss`:
- Around line 51-53: The nested modifier selector &.strip-width-row inside
cbx-reader-preferences-component.scss redundantly declares align-items: center
which is already set on the parent .setting-label-row; remove the align-items:
center line from the &.strip-width-row rule (or if you actually intend to
override a different alignment later, leave a comment explaining the intent) —
locate the .setting-label-row rule and the &.strip-width-row block in the file
and delete the redundant property.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c53f4ea-5c21-4c62-aeb7-e5ecd5ae9088
📒 Files selected for processing (20)
booklore-api/src/main/java/org/booklore/model/dto/BookLoreUser.javabooklore-api/src/main/java/org/booklore/service/user/DefaultUserSettingsProvider.javafrontend/src/app/features/readers/cbx-reader/cbx-reader.component.htmlfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.scssfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.htmlfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.scssfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.spec.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.htmlfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scssfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.spec.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.tsfrontend/src/app/features/settings/reader-preferences/reader-preferences.service.tsfrontend/src/app/features/settings/user-management/user.service.tsfrontend/src/i18n/en/reader-cbx.jsonfrontend/src/i18n/en/settings-reader.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (6)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/service/user/DefaultUserSettingsProvider.javabooklore-api/src/main/java/org/booklore/model/dto/BookLoreUser.java
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.scssfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.spec.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.spec.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scssfrontend/src/app/features/settings/user-management/user.service.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.htmlfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.htmlfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.tsfrontend/src/app/features/settings/reader-preferences/reader-preferences.service.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.scssfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.htmlfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.spec.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.spec.tsfrontend/src/app/features/settings/user-management/user.service.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.tsfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.tsfrontend/src/app/features/settings/reader-preferences/reader-preferences.service.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
frontend/src/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for tests in the frontend
Files:
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.spec.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.spec.tsfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.ts
frontend/src/app/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep Angular code on standalone components. Do not add NgModules
Files:
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
frontend/src/i18n/**
📄 CodeRabbit inference engine (AGENTS.md)
Put user-facing strings in Transloco files under
frontend/src/i18n/
Files:
frontend/src/i18n/en/reader-cbx.jsonfrontend/src/i18n/en/settings-reader.json
🧠 Learnings (6)
📓 Common learnings
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/settings/user-management/user.service.ts:64-75
Timestamp: 2026-03-31T19:52:51.386Z
Learning: In `grimmory-tools/grimmory`, CBX reader settings (including `CbxPageViewMode`, `CbxPageSplitOption`, `CbxScrollMode`, etc.) are stored as a raw JSON string blob in the `user_settings` table under a setting key (e.g., `CBX_READER_SETTING`). The backend does not deserialize these values into typed Java enum instances, so adding new frontend enum values (e.g., `TWO_PAGE_REVERSED`, `CbxPageSplitOption` members) does NOT cause Jackson `InvalidFormatException` errors. Do not flag frontend-only enum additions as backend serialization issues for these settings.
📚 Learning: 2026-03-31T19:52:51.386Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/settings/user-management/user.service.ts:64-75
Timestamp: 2026-03-31T19:52:51.386Z
Learning: In `grimmory-tools/grimmory`, CBX reader settings (including `CbxPageViewMode`, `CbxPageSplitOption`, `CbxScrollMode`, etc.) are stored as a raw JSON string blob in the `user_settings` table under a setting key (e.g., `CBX_READER_SETTING`). The backend does not deserialize these values into typed Java enum instances, so adding new frontend enum values (e.g., `TWO_PAGE_REVERSED`, `CbxPageSplitOption` members) does NOT cause Jackson `InvalidFormatException` errors. Do not flag frontend-only enum additions as backend serialization issues for these settings.
Applied to files:
booklore-api/src/main/java/org/booklore/service/user/DefaultUserSettingsProvider.javabooklore-api/src/main/java/org/booklore/model/dto/BookLoreUser.javafrontend/src/i18n/en/reader-cbx.jsonfrontend/src/i18n/en/settings-reader.jsonfrontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/**/*.{test,spec}.ts : Use Vitest for tests in the frontend
Applied to files:
frontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.ts
📚 Learning: 2026-03-26T01:46:48.863Z
Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/app/**/*.component.ts : Keep Angular code on standalone components. Do not add NgModules
Applied to files:
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.ts
📚 Learning: 2026-03-31T14:12:39.521Z
Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 307
File: frontend/src/i18n/hr/layout.json:3-3
Timestamp: 2026-03-31T14:12:39.521Z
Learning: In `frontend/src/i18n/hr/layout.json` (Croatian locale), newly added i18n keys may intentionally use English placeholder values (e.g., `toggleNavigation`, `adjustSidebarWidth`, `closeNavigation`). Do not flag these as missing Croatian translations — this is a deliberate placeholder approach by the team.
Applied to files:
frontend/src/i18n/en/reader-cbx.jsonfrontend/src/i18n/en/settings-reader.json
📚 Learning: 2026-03-31T19:52:06.262Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts:787-810
Timestamp: 2026-03-31T19:52:06.262Z
Learning: In `frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts`, the `doublePairs` field (type `DoublePairs = Record<number, number>`) maps right-page indices to their paired left-page indices. By design, `currentPage` always points to the left (lower-indexed) page of any spread pair — backward navigation enforces this via `Math.min(targetPage, pairedWith)` and `goToPage`/`alignCurrentPageToParity` also preserve this invariant. Therefore, looking up `doublePairs[currentPage]` in forward navigation will only ever encounter left-page keys, and there is no scenario where `currentPage` would be a right-page key.
Applied to files:
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
🪛 HTMLHint (1.9.2)
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 111-111: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 111-111: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 143-143: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 143-143: Special characters must be escaped : [ > ].
(spec-char-escape)
🪛 Stylelint (17.6.0)
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss
[error] 479-479: Expected empty line before comment (scss/double-slash-comment-empty-line-before)
(scss/double-slash-comment-empty-line-before)
🔇 Additional comments (37)
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.scss (1)
97-101: Looks good for the new strip-width row layout.The added flex constraints are appropriate for preventing overflow and right-aligning the slider controls.
booklore-api/src/main/java/org/booklore/model/dto/BookLoreUser.java (1)
205-206: DTO extension is clean and backward-compatible.Adding
stripMaxWidthPercentas nullable here is a safe schema evolution for user settings payloads.booklore-api/src/main/java/org/booklore/service/user/DefaultUserSettingsProvider.java (1)
104-104: Default value wiring is correct.Setting the CBX default strip max width to
100here is consistent with the new setting contract.frontend/src/app/features/settings/user-management/user.service.ts (1)
213-214: Type contract update looks good.The optional
stripMaxWidthPercentfield is a clean extension of the CBX user settings model.frontend/src/app/features/settings/reader-preferences/reader-preferences.service.ts (1)
24-24: Silent update option is implemented correctly.The toast gating preserves existing behavior by default while allowing no-notification updates when needed.
Also applies to: 41-48
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.spec.ts (1)
60-60: Good coverage for strip-width preference behavior.These assertions correctly validate both default initialization and silent persistence semantics.
Also applies to: 79-89
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.spec.ts (1)
119-129: LGTM!Good test coverage for the new
stripMaxWidthChange$observable andsetStripMaxWidthPercentmethod. The test properly verifies both the emission and state update.frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.ts (1)
102-111: LGTM!The
showStripWidthControlgetter correctly hides the slider on phone portrait and only shows it for applicable scroll modes. The input handler properly delegates to the service which handles clamping.frontend/src/i18n/en/settings-reader.json (1)
83-84: LGTM!Clear and descriptive i18n strings for the new strip width setting and long strip mode.
Also applies to: 98-98
frontend/src/i18n/en/reader-cbx.json (1)
117-129: LGTM!Well-structured i18n additions for the quick settings, webtoon suggestion banner, and scroll continuation hint. The
regionLabelkey supports good accessibility practices.frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.service.ts (1)
142-146: LGTM!The
setStripMaxWidthPercentmethod correctly clamps the input value, updates the internal state, and emits the change for subscribers. The pattern is consistent with the existing service architecture.frontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.spec.ts (1)
1-70: LGTM!Comprehensive test coverage for the storage utility functions. The tests properly cover:
- Clamping edge cases (out-of-range, rounding, NaN/undefined)
- Global vs. per-book settings
- Namespaced vs. legacy key formats with fallback behavior
- Webtoon hint suppression via session and persistent flags
frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.ts (1)
107-125: LGTM!The strip max width preference implementation follows the established pattern for other CBX preferences. The use of
{ silent: true }for the slider updates is appropriate to avoid notification spam during continuous adjustments.frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss (3)
19-23: LGTM!Fullscreen styles correctly override dimensions and constrain max-height for proper full-screen display.
25-89: LGTM!The webtoon suggestion banner styles are well-structured with proper flex layout, responsive gap handling, and clear button variants (primary, default, subtle). The styling follows the existing component patterns.
408-445: LGTM!The
.strip-width-constraincontainer and continuation hint/spacer styles are well-implemented with appropriate flex behavior, centering viamargin-inline: auto, and smooth opacity/transform transitions for the visibility animation.frontend/src/app/features/readers/cbx-reader/core/cbx-reader-storage.ts (3)
1-16: LGTM!The storage key constants and
clampStripMaxWidthPercentfunction are well-implemented with proper null/NaN handling and sensible defaults.
18-45: LGTM!The
readStripWidthPercentfunction correctly handles:
- Global settings path with early return
- User-namespaced keys with legacy fallback for migration
- Try/catch for storage unavailability with sensible default (100%)
47-84: LGTM!The write and webtoon hint functions are well-implemented:
writeStripWidthPercentPerBookapplies clamping and handles quota/private mode errorsshouldOfferWebtoonHintcorrectly checks both permanent (localStorage) and session (sessionStorage) dismissal- Dismiss functions properly separate session vs. permanent dismissal with appropriate storage targets
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html (4)
1-3: LGTM!The
#readerRoottemplate reference is correctly added for fullscreen targeting and the existing class bindings are preserved.
9-24: LGTM!The webtoon suggestion banner has proper accessibility attributes (
role="region",aria-label), localized content via transloco, and clear action buttons wired to component handlers.
100-121: LGTM!The long-strip section correctly wraps content in
.strip-width-constrainwith dynamic width binding, adds continuation spacer/hint with visibility control andaria-live="polite", and maintains the end-of-comic action logic.
125-153: LGTM!The infinite scroll section mirrors the long-strip structure appropriately with strip-width constraint, continuation hints, and proper end-of-comic rendering.
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts (14)
70-77: LGTM!The new constants for timing and thresholds are well-organized with clear naming and appropriate values for debouncing and UI behavior control.
157-158: LGTM!The new
ViewChildrefs forreaderRootandimageScrollHostcorrectly use{ read: ElementRef }to ensure proper element access.
242-254: LGTM!The helper methods are well-implemented:
bumpReaderLayoutGenerationinvalidates stale layout workgetImageScrollContainerprovides null-safe access to the scroll containerafterNextPaintuses the standard double-rAF pattern for post-layout callbacks
351-359: LGTM!The strip width initialization correctly reads from storage with proper fallback handling using the new
readStripWidthPercentutility.
649-663: LGTM!The
persistStripMaxWidthmethod correctly:
- Clamps the value before persisting
- Routes to global preferences or per-book storage based on
cbxSettingsUsesGlobal- Uses
{ silent: true }to avoid toast spam during slider adjustment
665-698: LGTM!The
updateContinuationHintmethod implements proper hysteresis logic to prevent hint flicker:
- Early returns for invalid states
- Grace period check during layout transitions
- Latched state with separate show/hide thresholds
700-713: LGTM!The webtoon suggestion handlers are clean and correctly delegate to the storage functions for session/permanent dismissal.
1056-1088: LGTM!The
onScrollModeChangemethod properly:
- Bumps layout generation to invalidate stale work
- Tears down previous mode state
- Clears debounce timers when leaving infinite scroll mode
- Hides webtoon suggestion when switching to long-strip
1125-1149: LGTM!The
onScrollhandler correctly:
- Early-exits for non-infinite scroll modes
- Triggers load more/previous based on scroll position thresholds
- Debounces page detection to reduce computation
- Updates continuation hint on each scroll event
1169-1199: Well-implemented scroll anchor preservation for upward prefetching.The
loadPreviousPagesmethod uses a clever anchor-based approach to preserve scroll position when prepending pages:
- Captures the anchor element's position before mutation
- Prepends pages to the array
- After paint, adjusts scrollTop by the position delta
This prevents the jarring scroll jump that would otherwise occur.
1201-1235: LGTM!The
updateCurrentPageFromScrollmethod correctly finds the closest visible image to the viewport center using distance calculation, avoiding the edge-case issues that a simple "first visible" approach would have.
1315-1357: LGTM!The
longStripWaitForImagesAndScrollmethod properly:
- Guards against stale layout generations
- Uses retry loop with attempt limit for DOM availability
- Handles both immediate completion and load/error events
- Uses
afterNextPaintfor proper timing
1362-1394: LGTM!The
snapScrollContainerToElementhelper provides precise scroll positioning without relying onscrollIntoViewfor instant scrolls, which can cause double-teleport issues. ThelongStripDoScrollmethod correctly selects between snap and smooth approaches.
2209-2223: LGTM!The
ngOnDestroycleanup properly clears theinfiniteScrollPageDebounceTimerto prevent memory leaks and stale callbacks after component destruction.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts`:
- Around line 295-298: A pending infinite-scroll debounce timer can fire after a
book/mode transition and call updateCurrentPageFromScroll, overwriting
currentPage for the new context; introduce and use a dedicated timer property
(e.g., this.scrollDebounceTimer) and always clear it (clearTimeout and set to
null) when switching books/modes, in bumpReaderLayoutGeneration, and in
ngOnDestroy, and also clear any existing timer before scheduling a new
setTimeout for updateCurrentPageFromScroll so the callback only runs for the
active context.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8667fc8b-63ba-43a9-b011-95e730dac2b7
📒 Files selected for processing (6)
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.htmlfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.scssfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.tsfrontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.htmlfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.htmlfrontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scss
✅ Files skipped from review due to trivial changes (2)
- frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.html
- frontend/src/app/features/settings/reader-preferences/cbx-reader-preferences/cbx-reader-preferences-component.scss
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/features/readers/cbx-reader/cbx-reader.component.scss
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (3)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS files
Files:
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.htmlfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.tsfrontend/src/app/features/readers/cbx-reader/cbx-reader.component.html
frontend/src/app/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep Angular code on standalone components. Do not add NgModules
Files:
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/settings/user-management/user.service.ts:64-75
Timestamp: 2026-03-31T19:52:51.386Z
Learning: In `grimmory-tools/grimmory`, CBX reader settings (including `CbxPageViewMode`, `CbxPageSplitOption`, `CbxScrollMode`, etc.) are stored as a raw JSON string blob in the `user_settings` table under a setting key (e.g., `CBX_READER_SETTING`). The backend does not deserialize these values into typed Java enum instances, so adding new frontend enum values (e.g., `TWO_PAGE_REVERSED`, `CbxPageSplitOption` members) does NOT cause Jackson `InvalidFormatException` errors. Do not flag frontend-only enum additions as backend serialization issues for these settings.
📚 Learning: 2026-03-31T19:52:06.262Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts:787-810
Timestamp: 2026-03-31T19:52:06.262Z
Learning: In `frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts`, the `doublePairs` field (type `DoublePairs = Record<number, number>`) maps right-page indices to their paired left-page indices. By design, `currentPage` always points to the left (lower-indexed) page of any spread pair — backward navigation enforces this via `Math.min(targetPage, pairedWith)` and `goToPage`/`alignCurrentPageToParity` also preserve this invariant. Therefore, looking up `doublePairs[currentPage]` in forward navigation will only ever encounter left-page keys, and there is no scenario where `currentPage` would be a right-page key.
Applied to files:
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
📚 Learning: 2026-03-31T19:52:51.386Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 306
File: frontend/src/app/features/settings/user-management/user.service.ts:64-75
Timestamp: 2026-03-31T19:52:51.386Z
Learning: In `grimmory-tools/grimmory`, CBX reader settings (including `CbxPageViewMode`, `CbxPageSplitOption`, `CbxScrollMode`, etc.) are stored as a raw JSON string blob in the `user_settings` table under a setting key (e.g., `CBX_READER_SETTING`). The backend does not deserialize these values into typed Java enum instances, so adding new frontend enum values (e.g., `TWO_PAGE_REVERSED`, `CbxPageSplitOption` members) does NOT cause Jackson `InvalidFormatException` errors. Do not flag frontend-only enum additions as backend serialization issues for these settings.
Applied to files:
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts
🪛 HTMLHint (1.9.2)
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (4)
frontend/src/app/features/readers/cbx-reader/layout/quick-settings/cbx-quick-settings.component.html (1)
37-47: Nice accessibility follow-through on the strip-width slider.The conditional control, range bounds, and programmatic label wiring look correct here.
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.html (2)
9-24: Webtoon suggestion banner structure is solid.Good use of localized text/actions and a labeled region wrapper for assistive tech context.
100-151: Strip-width constraint + continuation/end CTA composition looks clean.The long-strip/infinite wrappers and conditional continuation blocks are well integrated and readable.
frontend/src/app/features/readers/cbx-reader/cbx-reader.component.ts (1)
1260-1267: Good use of generation-aware delayed layout work.The guarded delayed scrolling/positioning flow is a strong reliability improvement for mode and book switches.
Also applies to: 1362-1394, 1614-1626
…rimmory-tools#390) * feat(cbx-viewer): add (back) infinite and long-strip modes, fixed. * feat(reader): improve strip width controls and improve continuation affordance * fix(cbx-reader): clear debounce timer on layout generation changes
…rimmory-tools#390) * feat(cbx-viewer): add (back) infinite and long-strip modes, fixed. * feat(reader): improve strip width controls and improve continuation affordance * fix(cbx-reader): clear debounce timer on layout generation changes
…rimmory-tools#390) * feat(cbx-viewer): add (back) infinite and long-strip modes, fixed. * feat(reader): improve strip width controls and improve continuation affordance * fix(cbx-reader): clear debounce timer on layout generation changes
Description
Linked Issue: Fixes #
Changes
This pull request introduces a new feature to the CBX comic reader: users can now control the maximum width (as a percentage) for infinite and long-strip (webtoon) reading modes. It also adds a user-friendly suggestion banner for switching to webtoon mode, improves accessibility, and enhances the reading experience with continuation hints and better state persistence. The changes span backend DTOs, frontend UI, styles, and local storage handling.
The most important changes are:
User Preference for Strip Max Width:
stripMaxWidthPercentfield to the backend DTO (CbxReaderSetting) and set its default to 100% in user settings, allowing the server to store and provide this preference.UI/UX Improvements for Webtoon/Long-Strip Modes:
Continuation and Accessibility Enhancements:
Frontend Service and Storage Logic:
Quick Settings Panel Updates:
Summary by CodeRabbit
New Features
Improvements
Tests