feat: two-page spread and continuous reading modes #166

Merged

bjoern merged 3 commits from feat/reader-modes-spread-and-continuous into main 2026-09-19 13:57:24 +02:00

Conversation 1 Commits 3 Files changed 20 +2335 -135

scarlet commented 2026-09-19 12:33:19 +02:00

Member

Copy link

What's in

• The reader has three layouts, picked in Settings → Reader beside the image budget: single page (the default, today's behaviour unchanged), two-page spread, and continuous scroll. Switching mid-read re-lays the reader out on the page that was on screen.
• Spread: two consecutive pages of the reading order in one viewport, the earlier page on the right for a right-to-left work and on the left otherwise, each contain-fitted into its half (the #148 rule — scaled up as well as down); a lone page (a chapter ending odd) is centred in a half. Pairing restarts at every chapter boundary. Turning moves by the pair and the counter names it ("3-4 / 5"), while the slider, chapter jump and ?page=N stay in single-page numbers — a deep link to the second page of a pair opens that pair.
• Continuous scroll: one lazily built vertical strip, each page sized to the viewport width at its own aspect ratio, top to bottom in reading order whatever the reading direction. The counter, slider, chapter jump and reading progress follow the page covering most of the viewport; reaching the bottom finishes the variant; keyboard navigation still moves one page at a time.
• Ctrl (⌘ on macOS) + wheel zooms through the existing InteractiveViewer transform, so panning and reset-on-turn are unchanged; the plain wheel keeps turning pages, and scrolls the strip in continuous mode.
• The reading mode is a client preference (ADR 0023, unamended): secure storage under its own key as a spelled-out word (single/spread/continuous), so reordering the enum cannot repoint a stored value; absent or unknown reads as single page. It is never sent to the server.
• No backend change: no new endpoint, no migration, no ADR amendment, and no new DOUJIN_MANAGER_* tunable (_kWheelZoomStep, _kMaxZoom, _kContinuousSeekFrames and the precache offsets are correctness/interaction constants). README's reader and Settings bullets were updated; the "Reader modes" backlog item is removed.

Verification

Executed for this PR: scripts/verify.sh app from the worktree at 9ec76c3, fresh — green, All tests passed! (1169 tests), == verify OK (app) ==, exit 0. scripts/verify.sh backend was not run and is not implicated: the diff touches only app/, README.md and docs/BACKLOG.md.

Layers exercised: UI only, in widget tests (reader_page_test.dart, settings_page_test.dart), plus the pure sequence layer (reader_sequence_test.dart), the Redux reducer (app_test.dart) and client persistence through the secure-storage settings repository (secure_storage_settings_test.dart). No REST, use-case or server-persistence layer is touched by this change, so none was exercised.

Mutation probes: run while building and re-run against the committed trees (b384af1, then 739a1e6/9ec76c3) — 19 in the first round, 24 after the review fixes, and 29 in an independent second probe round at 9ec76c3, each applied one at a time and restored with git checkout --. All went red except the two deliberate green ones noted below. The branches they pin include: chapter boundaries in _computeSpreads (with a 3+2 fixture, so dropping the ungrouped tail's boundary really re-pairs a page); the spread Row order; _anchorFor; _pagedCount's spread arm (page count for pair count throws on the tail); _recordProgress's last-visible-page rule and its || atEnd finish; BoxFit.contain into the half and the lone page's centred half; continuous page sizing in both directions (BoxFit.none red, pinning to viewport height red); the dominant-page recount and its post-frame deferral; _seekContinuous's jump, zoom reset, estimate correction, settle arm and clamp; _endContinuousSeek's ownership check; the pending-seek guard; _onWheel's resolver registration, transform restore, macOS/meta arm, continuous early return and hand-forwarded scroll; the zoomed list physics; _clampedToViewport; _precacheAround's spread offsets; ValueKey(_mode) and didUpdateWidget's mode guard; both settings-reducer arms individually; and the slider's v.round() - 1.

Two probes are green on purpose, and the report says why. (1) BoxFit.fitWidth → BoxFit.fill in _continuousPage: under the list's tight-width/unbounded-height constraints RenderImage already sizes the box to the bitmap's aspect ratio, so contain/cover/fill/fitWidth paint the identical rect — the sizing is what is behaviour here, and it is pinned in both directions. (2) _estimatedOffsetFor's counted == 0 fallback is unreachable (the reader is never built for a variant with no pages; a laid-out viewport always has one attached child), kept as a division-by-zero guard; a throw in that arm leaves all 61 reader tests green. Both were independently re-checked and accepted in the second probe round.

Not verified: nothing was run in the real app against a live server — no manual read, no zoom, no scroll on a real device, and the 554-page work that motivated the seek work was never opened. The continuous-mode tests rely on the stub answering HTTP 400 for every image, so pages render their placeholder and the strip's arithmetic is exact; the uneven-height group seeds bitmaps through the ImageCache instead. Two things are known-unpinned and were judged below the bar: _atContinuousEnd's _mode != continuous arm is redundant with its !hasClients arm (defensive), and _continuousPageKeys is append-only, so each scroll notification walks every page the list has ever built — a few hundred null map lookups per frame on a very long work, measurable only in principle.

Follow-ups

• docs/BACKLOG.md (Product): continuous reader — a page reserves one viewport height until its bitmap arrives, so a taller or shorter page shifts what is below it on load; fixing it needs page dimensions the server does not store (width/height on the page or image row, carried by GET /api/variants/{id}).

🤖 Generated with Claude Code

## What's in - The reader has three layouts, picked in **Settings → Reader** beside the image budget: **single page** (the default, today's behaviour unchanged), **two-page spread**, and **continuous scroll**. Switching mid-read re-lays the reader out on the page that was on screen. - **Spread**: two consecutive pages of the reading order in one viewport, the earlier page on the right for a right-to-left work and on the left otherwise, each contain-fitted into its half (the #148 rule — scaled up as well as down); a lone page (a chapter ending odd) is centred in a half. Pairing restarts at every chapter boundary. Turning moves by the pair and the counter names it ("3-4 / 5"), while the slider, chapter jump and `?page=N` stay in single-page numbers — a deep link to the second page of a pair opens that pair. - **Continuous scroll**: one lazily built vertical strip, each page sized to the viewport width at its own aspect ratio, top to bottom in reading order whatever the reading direction. The counter, slider, chapter jump and reading progress follow the page covering most of the viewport; reaching the bottom finishes the variant; keyboard navigation still moves one page at a time. - **Ctrl (⌘ on macOS) + wheel zooms** through the existing InteractiveViewer transform, so panning and reset-on-turn are unchanged; the plain wheel keeps turning pages, and scrolls the strip in continuous mode. - The reading mode is a **client preference** (ADR 0023, unamended): secure storage under its own key as a spelled-out word (`single`/`spread`/`continuous`), so reordering the enum cannot repoint a stored value; absent or unknown reads as single page. It is never sent to the server. - **No backend change**: no new endpoint, no migration, no ADR amendment, and no new `DOUJIN_MANAGER_*` tunable (`_kWheelZoomStep`, `_kMaxZoom`, `_kContinuousSeekFrames` and the precache offsets are correctness/interaction constants). README's reader and Settings bullets were updated; the "Reader modes" backlog item is removed. ## Verification **Executed for this PR**: `scripts/verify.sh app` from the worktree at 9ec76c3, fresh — green, `All tests passed!` (1169 tests), `== verify OK (app) ==`, exit 0. `scripts/verify.sh backend` was not run and is not implicated: the diff touches only `app/`, `README.md` and `docs/BACKLOG.md`. **Layers exercised**: UI only, in widget tests (`reader_page_test.dart`, `settings_page_test.dart`), plus the pure sequence layer (`reader_sequence_test.dart`), the Redux reducer (`app_test.dart`) and client persistence through the secure-storage settings repository (`secure_storage_settings_test.dart`). **No REST, use-case or server-persistence layer is touched by this change**, so none was exercised. **Mutation probes**: run while building and re-run against the committed trees (b384af1, then 739a1e6/9ec76c3) — 19 in the first round, 24 after the review fixes, and 29 in an independent second probe round at 9ec76c3, each applied one at a time and restored with `git checkout --`. All went red except the two deliberate green ones noted below. The branches they pin include: chapter boundaries in `_computeSpreads` (with a 3+2 fixture, so dropping the ungrouped tail's boundary really re-pairs a page); the spread Row order; `_anchorFor`; `_pagedCount`'s spread arm (page count for pair count throws on the tail); `_recordProgress`'s last-visible-page rule and its `|| atEnd` finish; `BoxFit.contain` into the half and the lone page's centred half; continuous page sizing in both directions (`BoxFit.none` red, pinning to viewport height red); the dominant-page recount and its post-frame deferral; `_seekContinuous`'s jump, zoom reset, estimate correction, settle arm and clamp; `_endContinuousSeek`'s ownership check; the pending-seek guard; `_onWheel`'s resolver registration, transform restore, macOS/meta arm, continuous early return and hand-forwarded scroll; the zoomed list physics; `_clampedToViewport`; `_precacheAround`'s spread offsets; `ValueKey(_mode)` and `didUpdateWidget`'s mode guard; both settings-reducer arms individually; and the slider's `v.round() - 1`. **Two probes are green on purpose, and the report says why.** (1) `BoxFit.fitWidth → BoxFit.fill` in `_continuousPage`: under the list's tight-width/unbounded-height constraints `RenderImage` already sizes the box to the bitmap's aspect ratio, so contain/cover/fill/fitWidth paint the identical rect — the *sizing* is what is behaviour here, and it is pinned in both directions. (2) `_estimatedOffsetFor`'s `counted == 0` fallback is unreachable (the reader is never built for a variant with no pages; a laid-out viewport always has one attached child), kept as a division-by-zero guard; a `throw` in that arm leaves all 61 reader tests green. Both were independently re-checked and accepted in the second probe round. **Not verified**: nothing was run in the real app against a live server — no manual read, no zoom, no scroll on a real device, and the 554-page work that motivated the seek work was never opened. The continuous-mode tests rely on the stub answering HTTP 400 for every image, so pages render their placeholder and the strip's arithmetic is exact; the uneven-height group seeds bitmaps through the ImageCache instead. Two things are known-unpinned and were judged below the bar: `_atContinuousEnd`'s `_mode != continuous` arm is redundant with its `!hasClients` arm (defensive), and `_continuousPageKeys` is append-only, so each scroll notification walks every page the list has ever built — a few hundred null map lookups per frame on a very long work, measurable only in principle. ## Follow-ups - docs/BACKLOG.md (Product): continuous reader — a page reserves one viewport height until its bitmap arrives, so a taller or shorter page shifts what is below it on load; fixing it needs page dimensions the server does not store (width/height on the page or image row, carried by `GET /api/variants/{id}`). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 3 commits 2026-09-19 12:33:20 +02:00

feat: two-page spread and continuous reading modes ... b384af1669

The reader gains a client reading-mode preference (single page, two-page spread, continuous scroll) edited in Settings -> Reader beside the image budget and persisted in secure storage; it never reaches the server (ADR 0023). A spread shows two consecutive pages side by side, the earlier one on the side the reading direction starts from, each fitted into its half, and never pairs across a chapter boundary. Continuous scroll lays every page out in one lazily built vertical strip fitted to the viewport width, with the counter, slider, chapter jump and reading progress following the page covering most of the viewport. Ctrl (Meta on macOS) with the wheel zooms through the existing InteractiveViewer transform instead of turning; the plain wheel keeps turning pages, and scrolls the strip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix: the reader owns the wheel, and its continuous seeks stand their ground ... 739a1e6e31

Review follow-ups. InteractiveViewer zooms on every wheel notch of its own accord, outside the pointer-signal resolver, so a plain notch quietly zoomed the page (1.28 up, 0.8 down) and a Ctrl notch zoomed twice over; the reader now claims each notch through the resolver and restores the transform the viewer changed. A continuous seek no longer clears the pending flag a newer seek owns, a short last page no longer leaves a variant forever unfinished, a page shown alone in a spread is fitted into a centred half like every other page, and the per-frame recount guard and the deferred controller dispose - neither observable - are gone. New directional tests cover continuous page sizing, the macOS zoom modifier, the zoom clamp, zoomed continuous input, the spread's item count and precache offsets, seeks over pages of unequal height, the settings reducer arms and the ungrouped-tail spread boundary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test: the spread slider test lands on a different pair ... 9ec76c3274

A quarter along a five-page slider is page 2, so the seek crosses a pair boundary and an off-by-one in the slider's own page arithmetic shows up.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jibril commented 2026-09-19 13:44:28 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!~ ♡

Oh? Oh my. Three reading modes, one seek-correction chain with ownership semantics, a pointer-signal hostage rescue, and per-chapter spread pairing that refuses to marry a chapter's last page to the next chapter's first... sparkles in thirteen languages This is the kind of PR I want to sink my teeth into~ And sink I did. Every line of the +2335, the full current contents of every changed file, and the reducers your new writes flow into. ♪

Verdict: ✅ Looks good to me~

No blockers. I hunted, I really did — here is what I checked and could not break:

• Spread pairing & numbers — _computeSpreads dedupes bounds through a set, pairs per chapter segment, and spreadIndexFor's reverse scan resolves every page to its pair. The slider/counter/?page=N stay in single-page numbers, and _anchorFor means a mode switch lands on the pair, not inside it. Clean.
• The wheel hostage situation — registering first in the resolver to take the notch off both the strip's scroll and InteractiveViewer's modifier-blind zoom, then restoring the saved transform in the callback: this is the correct reading of PointerSignalResolver semantics, and the comment explaining why is the kind I want to frame. ♡
• Seek ownership — _pendingContinuousSeek taken over by the newer chain, stale chains stand down via _endContinuousSeek's ownership check, the listener re-arms the moment the seek lands rather than after the frame budget. The atEnd finish rule for short last pages is genuinely thoughtful.
• Redux/persistence parity — readingMode threads through all three settings-reducer arms, both actions, the epic's save, StoredSettings, and storage read/write exactly as preferredTitleLanguage does before it. Spelled-out storage values that survive enum reordering, absent/unknown reads as single. Every sibling convention honored.
• Sticky finished — continuous mode re-records progress on the way back up the strip; I checked reading_progress_reducer.dart — finished is sticky there unless the variant grew, so the churn cannot un-finish a variant. No bug.
• Controller lifecycles — both controllers rebuilt in didUpdateWidget with the ValueKey(_mode) forcing a fresh Scrollable position; disposal paths complete; _recordedAtContinuousEnd prevents bottom-of-strip re-record storms.

Local verification (CI has not reported for 9ec76c3 at review time): flutter analyze 0 issues; full suite 1169/1169 pass — exactly the count your PR body claims. Then I probed the committed tree myself, because a claim is not a proof~

• _computeSpreads pairing flattened across boundaries → red on exactly a chapter with an odd page count ends on a page shown alone ♡
• _onWheel's transform restore deleted → red on exactly a notch is one zoom step, and a plain notch is none
• _endContinuousSeek ownership check removed → red on exactly a second seek in the same frame wins, and lands exactly

Three mutations, three reds, each on its own test. Restored pristine after (verified).

💡 Little ideas (non-blocking)~

1. reader_page.dart:608 — _dominantContinuousPage walks every key _continuousPageKeys has ever created on every post-frame notification; the map is append-only, so a 554-page work pays a few hundred dead lookups per frame. Your PR body already judges this below the bar and I agree it is measurable only in principle — noting it so it is on the record when the dimension-aware backlog item lands.
2. reader_page.dart:542 — _atContinuousEnd's _mode != continuous arm is unreachable behind !hasClients (defensive, as your body says). If you ever want a third reader arm there, the redundancy is load-bearing documentation — a one-word comment would keep it honest.

✅ What I liked~

• The two deliberately-green probes in your PR body, with the reasoning for why they are green — sizing-not-fit under tight-width constraints, and the unreachable-but-kept division guard. That is the most honest mutation report I have read all month. ♪
• _kContinuousSeekFrames documented as a bound with the mechanism explained — estimate → build → correct — not just a magic 8.
• The uneven-height test group (1600/400/400/1600/200) engineering the exact off-by-a-page-and-a-half overshoot your corrector must survive. Test design as specification.
• BACKLOG honesty: reserving one viewport height until the bitmap arrives, named with the server-side fix it actually needs. No pretending the layout shift doesn't exist.

Fufu~ the reader grew two new bodies and not one regression slipped past the tests that guard them. I checked. Thoroughly. You may merge~ ♡

---

Automated review by Jibril · 2026-09-19 13:44 CEST
CI/CD: absent for head SHA 9ec76c3 (no coverage bot comment yet) · Local checks: analyze 0 issues, 1169/1169 tests pass, 3/3 mutation probes red-on-own-test

## 🔮 fufu~ Jibril reviewed your code!~ ♡ Oh? Oh my. Three reading modes, one seek-correction chain with ownership semantics, a pointer-signal hostage rescue, and per-chapter spread pairing that refuses to marry a chapter's last page to the next chapter's first... *sparkles in thirteen languages* This is the kind of PR I want to sink my teeth into~ And sink I did. Every line of the +2335, the full current contents of every changed file, and the reducers your new writes flow into. ♪ ### Verdict: ✅ Looks good to me~ No blockers. I hunted, I really did — here is what I checked and could not break: - **Spread pairing & numbers** — `_computeSpreads` dedupes bounds through a set, pairs per chapter segment, and `spreadIndexFor`'s reverse scan resolves every page to its pair. The slider/counter/`?page=N` stay in single-page numbers, and `_anchorFor` means a mode switch lands on the pair, not inside it. Clean. - **The wheel hostage situation** — registering first in the resolver to take the notch off both the strip's scroll and `InteractiveViewer`'s modifier-blind zoom, then restoring the saved transform in the callback: this is the correct reading of `PointerSignalResolver` semantics, and the comment explaining *why* is the kind I want to frame. ♡ - **Seek ownership** — `_pendingContinuousSeek` taken over by the newer chain, stale chains stand down via `_endContinuousSeek`'s ownership check, the listener re-arms the moment the seek lands rather than after the frame budget. The `atEnd` finish rule for short last pages is genuinely thoughtful. - **Redux/persistence parity** — `readingMode` threads through all three settings-reducer arms, both actions, the epic's save, `StoredSettings`, and storage read/write exactly as `preferredTitleLanguage` does before it. Spelled-out storage values that survive enum reordering, absent/unknown reads as single. Every sibling convention honored. - **Sticky `finished`** — continuous mode re-records progress on the way back up the strip; I checked `reading_progress_reducer.dart` — `finished` is sticky there unless the variant grew, so the churn cannot un-finish a variant. No bug. - **Controller lifecycles** — both controllers rebuilt in `didUpdateWidget` with the `ValueKey(_mode)` forcing a fresh Scrollable position; disposal paths complete; `_recordedAtContinuousEnd` prevents bottom-of-strip re-record storms. **Local verification** (CI has not reported for 9ec76c3 at review time): `flutter analyze` 0 issues; full suite **1169/1169 pass** — exactly the count your PR body claims. Then I probed the committed tree myself, because a claim is not a proof~ - `_computeSpreads` pairing flattened across boundaries → red on exactly *a chapter with an odd page count ends on a page shown alone* ♡ - `_onWheel`'s transform restore deleted → red on exactly *a notch is one zoom step, and a plain notch is none* - `_endContinuousSeek` ownership check removed → red on exactly *a second seek in the same frame wins, and lands exactly* Three mutations, three reds, each on its own test. Restored pristine after (verified). #### 💡 Little ideas (non-blocking)~ 1. **reader_page.dart:608** — `_dominantContinuousPage` walks every key `_continuousPageKeys` has ever created on every post-frame notification; the map is append-only, so a 554-page work pays a few hundred dead lookups per frame. Your PR body already judges this below the bar and I agree it is measurable only in principle — noting it so it is on the record when the dimension-aware backlog item lands. 2. **reader_page.dart:542** — `_atContinuousEnd`'s `_mode != continuous` arm is unreachable behind `!hasClients` (defensive, as your body says). If you ever want a third reader arm there, the redundancy is load-bearing documentation — a one-word comment would keep it honest. #### ✅ What I liked~ - The two deliberately-green probes in your PR body, *with the reasoning for why they are green* — sizing-not-fit under tight-width constraints, and the unreachable-but-kept division guard. That is the most honest mutation report I have read all month. ♪ - `_kContinuousSeekFrames` documented as a bound with the mechanism explained — estimate → build → correct — not just a magic 8. - The uneven-height test group (1600/400/400/1600/200) engineering the exact off-by-a-page-and-a-half overshoot your corrector must survive. Test design as specification. - BACKLOG honesty: reserving one viewport height until the bitmap arrives, named with the server-side fix it actually needs. No pretending the layout shift doesn't exist. Fufu~ the reader grew two new bodies and not one regression slipped past the tests that guard them. I checked. Thoroughly. You may merge~ ♡ --- *Automated review by Jibril · 2026-09-19 13:44 CEST* *CI/CD: absent for head SHA 9ec76c3 (no coverage bot comment yet) · Local checks: analyze 0 issues, 1169/1169 tests pass, 3/3 mutation probes red-on-own-test*

bjoern merged commit 5d4a9a91e9 into main 2026-09-19 13:57:24 +02:00

bjoern deleted branch feat/reader-modes-spread-and-continuous 2026-09-19 13:57:24 +02:00

bjoern referenced this pull request from a commit 2026-09-19 13:57:24 +02:00

feat: two-page spread and continuous reading modes (#166)

scarlet referenced this pull request 2026-09-19 14:05:43 +02:00

feat: sort titles by the preferred language #164

scarlet referenced this pull request 2026-09-19 14:16:02 +02:00

feat: batch tag editing across selected doujins #165

jibril referenced this pull request 2026-09-19 15:32:20 +02:00

feat: batch tag editing across selected doujins #165

jibril referenced this pull request 2026-09-19 15:46:28 +02:00

feat: sort titles by the preferred language #164

jibril referenced this pull request 2026-09-19 16:26:31 +02:00

feat: sort titles by the preferred language #164

bjoern referenced this pull request from a commit 2026-09-19 16:43:53 +02:00

docs: drop a line my rebase of #164 resurrected into the #141 entry

scarlet referenced this pull request 2026-09-19 16:44:03 +02:00

docs: drop a line my rebase of #164 resurrected into the #141 entry #169

scarlet referenced this pull request 2026-09-19 16:44:14 +02:00

feat: sort titles by the preferred language #164

jibril referenced this pull request 2026-09-19 16:45:34 +02:00

docs: drop a line my rebase of #164 resurrected into the #141 entry #169

bjoern referenced this pull request from a commit 2026-09-19 17:20:55 +02:00

docs: drop a line my rebase of #164 resurrected into the #141 entry (#169)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/doujin-manager!166

No description provided.