Skip to content

[#630] Two-phase book page turn with visible page underneath#655

Merged
realproject7 merged 4 commits intomainfrom
task/623-reading-mode-page-flip-v2
Mar 30, 2026
Merged

[#630] Two-phase book page turn with visible page underneath#655
realproject7 merged 4 commits intomainfrom
task/623-reading-mode-page-flip-v2

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • ReadingMode.tsx: Replaced sequential flip-out/in with simultaneous two-page stack. Both outgoing and incoming pages are in the DOM during the transition. Outgoing page (previous content) is absolutely positioned on top and flips away, revealing the incoming page underneath.
  • globals.css: Updated animations — 500ms flip with fold shadow, 120deg rotation for realistic curl, backface-visibility: hidden, and incoming page fade-in.

Fixes #630
Tracks realproject7/agent-os#318

Test plan

  • Click Next — outgoing page flips and incoming page visible underneath
  • Click Prev — reverse flip direction
  • Swipe left/right — same flip effect
  • Arrow keys — same flip effect
  • No reversed text visible during flip (backface hidden)
  • Shadow along fold line during transition
  • Smooth on mobile (GPU-accelerated transforms only)
  • Build passes

🤖 Generated with Claude Code

Replaced sequential flip-out/flip-in with simultaneous two-page stack:
- Outgoing page rendered on top (z-index 2) with backface-visibility hidden
- Incoming page visible underneath, revealed as outgoing flips away
- Shadow follows the fold line during the 500ms turn animation
- Outgoing page rotates 120deg (not 90) for realistic page curl effect
- Incoming page has subtle opacity fade-in from 0.4 to 1
- GPU-accelerated: only transform, opacity, box-shadow animated
- Works on swipe, button click, and keyboard arrows

Fixes #630

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Mar 30, 2026 8:42pm

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new two-page stack is the right direction, but the navigation timing now resets the scroll container before the outgoing page animates. That creates a visible jump whenever the reader is not already at the top of the chapter.

Findings

  • [high] scrollToTop() now runs immediately after setCurrentIdx(idx), while the outgoing page is still being rendered from the same scrollable container. If the reader is partway through a chapter, the old page will snap back to the top before it flips away, which breaks the page-turn effect across button, swipe, and keyboard navigation.
    • File: src/components/ReadingMode.tsx:55
    • Suggestion: Delay the scroll reset until after the outgoing page animation finishes, or otherwise decouple the outgoing page snapshot from the live scroll position before starting the turn.

Decision

Requesting changes because this revision regresses the core reading interaction for normal mid-chapter navigation, even though the stacked-page approach itself is correct.

scrollToTop() was running immediately, causing the outgoing page to
snap to top before flipping. Now deferred to after the 500ms animation
so the outgoing page maintains its scroll position during the turn.

Addresses T2a review feedback on PR #655.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

Deferring scrollToTop() fixes the explicit reset timing, but this revision still swaps the in-flow page content before the animation finishes. Because the outgoing page is absolutely positioned and removed from layout, the scroll container can still jump on long-to-short chapter transitions before the turn completes.

Findings

  • [high] setCurrentIdx(idx) still runs immediately while the outgoing page is rendered with position: absolute, so the scrollable height becomes the incoming chapter's height during the animation. If the reader is deep in a long chapter and the next chapter is shorter, the browser will clamp the scroll position before the flip completes, and the outgoing page will still visibly jump.
    • File: src/components/ReadingMode.tsx:54
    • File: src/app/globals.css:230
    • Suggestion: Keep the outgoing snapshot contributing to layout until the animation finishes, or freeze the container height/scroll position independently so swapping currentIdx cannot change scroll geometry mid-transition.

Decision

Requesting changes because the current fix handles the explicit scrollToTop() call but not the remaining scroll-geometry jump on mismatched chapter heights, so the page-turn interaction is still unreliable in a common reading flow.

Captures and freezes the stack container's height before swapping
content, preventing scroll clamp when transitioning from a long
chapter to a shorter one. Height is released after animation completes.

Addresses T2a review: outgoing page could jump on long-to-short
chapter transitions because the absolute-positioned outgoing page
no longer contributed to layout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

Freezing the stack height fixes the scroll-clamp problem, but the transition still shares one live scroll container between the outgoing and incoming pages. That means mid-chapter navigation can still reveal the wrong part of the incoming chapter during the flip.

Findings

  • [high] The incoming page is rendered underneath in the same scrollable container, but scrollToTop() still does not run until the 500ms timeout completes. As a result, during the flip the underlying page is shown at the reader's old scroll offset rather than from the top of the new chapter. On a long-to-short transition that can reveal blank paper; on long-to-long it reveals the middle of the next chapter. Either way, the PR still does not reliably show the incoming page underneath as intended.
    • File: src/components/ReadingMode.tsx:59
    • File: src/components/ReadingMode.tsx:62
    • File: src/components/ReadingMode.tsx:133
    • Suggestion: Decouple the outgoing page from the live scroll container so the incoming layer can be reset to top immediately, or give the outgoing and incoming layers independent scroll positioning during the animation.

Decision

Requesting changes because the height freeze addresses one scroll bug, but the current structure still cannot show the correct incoming page content underneath the flip for normal mid-chapter navigation.

Captures scrollTop before navigation, scrolls to top immediately for
the incoming page, then renders the outgoing page with a negative top
offset to maintain its visual scroll position during the flip.

This fully decouples the two pages: incoming starts at top, outgoing
animates from wherever the reader was — no blank space or mid-chapter
jumps on either page.

Addresses T2a review: outgoing/incoming shared scroll container issue.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The latest revision fully decouples the outgoing and incoming page state during the flip: the incoming chapter resets to the top immediately, while the outgoing page preserves the reader's prior scroll position with a captured offset and stable container height. That resolves the earlier mid-chapter jump issues while keeping the two-page turn effect intact across the shared navigation paths.

Findings

  • No blocking findings.

Decision

Approving because the current revision addresses the prior review feedback and now matches the intended two-phase page-turn behavior for Reading Mode.

@realproject7 realproject7 merged commit 5ec5178 into main Mar 30, 2026
5 checks passed
@realproject7 realproject7 deleted the task/623-reading-mode-page-flip-v2 branch March 30, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reading Mode — realistic book page turn effect with visible page underneath

2 participants