Replace calendar day arrows with edge taps on video - #48
Open
LukeNeedham wants to merge 6 commits into
Open
Conversation
Removes the previous/next chevron buttons from the calendar date selector. Tapping the left or right edge of the video now navigates to the previous/next day instead, while swiping through days and press-and-hold to pause playback keep working as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
|
A long press (holding to pause playback) with no movement was still short enough in distance to pass the tap-detection check, so releasing it after a long hold incorrectly navigated to the previous/next day. Edge taps now also require the press to be released within the platform's long-press timeout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
onPrevious/onNext computed the target page from pagerState.currentPage, which only updates once an in-flight animateScrollToPage animation settles. A tap landing while the previous page-flip was still animating therefore resolved to the same page already being animated to - a no-op - making rapid edge taps feel unresponsive for as long as the prior animation was still running. Use pagerState.targetPage instead, which reflects the page the pager is currently animating towards (or currentPage when idle), so each tap advances immediately regardless of any animation in progress. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
The targetPage fix wasn't enough - edge taps could still go dead for a second or two right after a page change. The actual cause: each page in the pager mounted its own VideoPlayerExo when it became current, and that composable creates a brand new native TextureView and rebinds the shared ExoPlayer's video surface to it every time. Recreating that native view on every day navigation left the video area unresponsive to touch for a beat, regardless of how the target page was computed. Now the live player is hosted once, persistently, as an overlay in CalendarScroller, only ever swapping which video it points at rather than tearing down and recreating its surface. Each page in the pager only ever shows its static thumbnail (VideoPlayer gets a new showPlayer = false flag for this). The overlay hides while the pager is mid-swipe, since it doesn't track page-drag offset itself - pages show their thumbnail during the drag, and the live video reappears once a swipe settles, matching how incoming pages already behaved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
The previous fix hoisted the player to a single overlay, but still gated its composition on !pagerState.isScrollInProgress. Since that flips true then false on every single tap/swipe, the overlay was still being unmounted and remounted on each navigation - recreating the native TextureView just as before, only one level up. That's why the touch dead-zone persisted after that change. Now VideoPlayerExo stays composed continuously (gated only on there being a video at all, not on scroll state), so its native view is created once and never torn down during ordinary day-to-day navigation. It's hidden during an active swipe via alpha instead of being removed from composition, preserving the previous show/hide behaviour without the teardown/recreate cost. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
The manual PointerEventPass.Initial loop (awaitFirstDown + waitForUpOrCancellation in a hand-written while(true), with our own touch-slop/long-press-duration math) had some subtle bug causing a one-to-two second window after the first tap where further taps weren't registered at all - rapid tapping on an edge should page through days one at a time, not stall after the first. Switch to detectTapGestures, Compose's standard primitive for this exact tap/long-press/drag arbitration: onPress still pauses/resumes playback for the whole gesture regardless of how it ends, onLongPress (a no-op) is enough to make onTap only fire for genuine taps, and onTap fires the edge navigation. Letting the pager's own scrollable (a descendant, so it sees drag motion first on the default Main pass) naturally consume real swipes - rather than us intercepting on the Initial pass and re-deriving swipe-vs-tap ourselves - removes the custom logic that was likely behind the stall. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbtRXD54bQgJSoNBeLVMHP
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
2 participants
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
CalendarDaySelector), leaving just the tappable date label.Test plan
./gradlew :app:compileDebugKotlin— compiles cleanly./gradlew :app:testDebugUnitTest :app:assembleDebug— unit tests pass, debug APK buildsGenerated by Claude Code