iOS: run the zoom cover transition for modal present/dismiss - #5
Open
artemlitch wants to merge 3 commits into
Open
iOS: run the zoom cover transition for modal present/dismiss#5artemlitch wants to merge 3 commits into
artemlitch wants to merge 3 commits into
Conversation
The Bookwise reader can now be presented OVER the bookstore pageSheet (as a transparentModal) with the same cover-zoom flight the stack push/pop uses: - RNSScreenStackView becomes the transitioningDelegate for modal screens whose stackAnimation is zoom, reusing RNSScreenStackAnimator through the modal present/dismiss path (animator.modalTransition skips the pop-side presenting-view reparenting, which would orphan a still-live sheet). - Zoom rects from JS are re-anchored by the card-hosting RNSScreenView's window origin: Fabric measureInWindow sums shadow-tree layout, which misses the native pageSheet offset (~62pt), so every rect measured inside a presented sheet was short by it (truncated stand-in composite, displaced flight). Exact no-op for stack screens at (0,0). - Modal zoom drags never run as UIKit interactive transitions: percent-driven modal dismissals freeze the container layer clock until completion, leaving the commit flight invisible (everything snapped at the end). The drag is a manual pose on the live presented view (mask + own dim, model writes), a committed release starts a plain non-interactive dismissal whose animator flies home from the release pose, and a cancelled drag springs back with no transition at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…its presence breaks Gradle autolinking when the fork is consumed via a link:/file: dependency (treated as a separate included build - 'No variants exist'). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… as the stack pop The modal zoom dismissal had its own drag implementation: a hand-driven pose, a hand-installed dim, and a plain non-interactive dismissal on commit. It ran with no UIKit transition until release, which meant the screen never reported a transition to JS. Consumers that suppress UI during a dismissal (a session recap sheet, a loading-cover reveal) learn about it from transitionStart/transitionEnd, so they stopped firing and that UI surfaced over the drag. The reason for the parallel path was that scrubbing a percent-driven transition parks the container's layer clock, making the commit flight invisible until it snapped at the end. That is real, but it is not a reason to avoid the transition. A nav pop restarts the clock at finishInteractiveTransition, before the flight's animations commit. An over-full-screen modal keeps it parked until completeTransition:, which the flight itself only calls once it lands - so the flight waits for the clock and the clock waits for the flight. Restart the clock at release instead (RNSZoomRestartLayerClock), and let the modal use RNSPercentDrivenInteractiveTransition exactly as the stack pop does. The drag pose is still applied by hand via model writes, which render fine on a parked layer - that part was always shared. handleZoomSwipe: is now identical for both paths; only the call that starts the dismissal differs. Deleted: beginManualZoomDragOnView:, cancelManualZoomDrag, _zoomManualDim, _modalZoomDragAnimator, the dim handover, and the modal commit branch in animateZoomPopWithContext:. Net 201 lines removed. Also restores android/settings.gradle. It was dropped to work around Gradle autolinking when the fork was consumed via a link:/file: dependency; consumed as a tarball it is harmless, and :app:assembleDebug is green with it present. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
The Bookwise reader could not be presented over the bookstore pageSheet with the cover-zoom flight. The sheet had to be dismissed first, so the zoom had nothing to fly out of and nothing to land back on. This branch lets the reader present over a live sheet using the same
RNSScreenStackAnimatorthe stack push/pop already uses.Consumed by readwiseio/rekindled#11083.
1. The zoom animator runs for modal present/dismiss
RNSScreenStackViewbecomes thetransitioningDelegatefor modal screens whosestackAnimationiszoom, reusing the existing animator throughanimationControllerForPresentedController:/animationControllerForDismissedController:.An
animator.modalTransitionflag guards the places where the stack's pop-side behaviour is wrong for a modal. The presenting view never left the window, so reparenting it into the transition container, or writing its frame, would orphan a still-live sheet after the transition.2. Zoom rects are re-anchored by the card's hosting screen
Fabric's
measureInWindowsums shadow-tree layout, which does not account for the native pageSheet offset, so every rect measured inside a presented sheet was short by exactly that amount. Measured on device: the card's true rect wasy=114while JS reportedy=52, a 62pt gap.That produced two visible symptoms. The stand-in composite was truncated, cut off roughly 40% down, because the card render was mapped against a slot rect that did not match the wrapper. And the whole flight landed 62pt above the real card.
RNSZoomCorrectSourceRectForCardoffsets the incoming rect by the card-hostingRNSScreenView's true window origin. Stack screens sit at (0,0), so this is an exact no-op for the shipped shelf zoom. Only sheet-hosted cards are corrected.3. The modal dismissal is the same interactive transition as the stack pop
This started out as a separate implementation: a hand-driven pose, a hand-installed dim, and a plain non-interactive dismissal on commit. It ran with no UIKit transition until release. That had a consequence beyond the animation. A screen with no transition never reports one, so
transitionStartandtransitionEndnever fired, and the JS that suppresses UI during a dismissal (the session recap sheet, the loading-cover reveal) never engaged. The recap sheet presented on top of an in-progress drag.The reason for the separate path was real but the conclusion was wrong. Scrubbing a percent-driven transition parks the container's layer clock (
speed=0, scrubbedtimeOffset), and animations added while it is parked do not play. For a nav pop the clock restarts atfinishInteractiveTransition, before the commit flight's animations commit, so the flight plays. For an over-full-screen modal it stays parked untilcompleteTransition:, which the flight itself only calls once it lands. The flight waits for the clock and the clock waits for the flight, which is the ~400ms hold followed by a hard cut.So restart the clock at release (
RNSZoomRestartLayerClock) and let the modal useRNSPercentDrivenInteractiveTransitionexactly as the stack pop does. The drag pose is still applied by hand through model writes, which render fine on a parked layer; that part was always shared between the two paths.handleZoomSwipe:is now identical for both paths. Only the call that starts the dismissal differs,dismissViewControllerAnimated:instead ofpopViewControllerAnimated:. Removed:beginManualZoomDragOnView:,cancelManualZoomDrag,_zoomManualDim,_modalZoomDragAnimator, the dim handover, and the modal commit branch inanimateZoomPopWithContext:. Net 201 lines deleted.Scope
Nav-stack push/pop is untouched. Every change is gated behind
modalTransitionor a presented-modal check, so the shipped shelf zoom behaves identically. iOS only; no JS API changes.android/settings.gradleis restored. An earlier commit deleted it to work around Gradle treating the fork as a separate included build when consumed vialink:/file:. Consumed as a tarball, which is how it ships, its presence is harmless.Testing
iPhone 17 Pro, Bookwise, from a recording rather than stills. Open from the modal cover, commit dismissal by drag, commit by flick, and a cancelled drag. Mid-drag frames show the reader shrunk and translated with the sheet live underneath and no recap sheet present, which was the regression. The close flight shows three distinct intermediate poses before landing, so it plays rather than snapping. The cover lands back on the sheet at full size.
Android:
:app:assembleDebugis green withsettings.gradlerestored, installed and run on a Pixel 7 Pro (API 34). Opening a book from the bookstore modal keeps the sheet mounted, and back returns to it.