chore(verify): combined device-verify build — DO NOT MERGE - #1697
Conversation
Chrome "Share → Candela" (ACTION_SEND / EXTRA_TEXT) opened the app but the URL was never added. Root cause: the URL was plumbed correctly end-to-end — manifest filter → MainActivity → DeepLinkResolver.resolve() → libraryWithShare() route → nav arg → LibraryScreen(sharedUrl=…) — and then DROPPED: `sharedUrl` was a dead parameter, never consumed into the Add-by-URL (Magic-add) flow. The clipboard magic-link path (same libraryWithShare route) was broken by the same gap. - LibraryScreen: a consume-once LaunchedEffect(sharedUrl) opens the Add-by-URL sheet via viewModel.onSharedUrl; rememberSaveable guards against re-opening on back-stack re-entry. - LibraryViewModel.onSharedUrl + pure sharePrefillState(): open the sheet PRE-FILLED (never silent-add — the #472 "Magic-add sheet pre-populated" contract). No-op for blank / non-http(s) input. - AddByUrlSheetState.Open gains a `prefill`; AddByUrlSheet seeds its field from it. - SharePrefillStateTest: JVM unit test for the pure decision. Compile-verified path only. RUNTIME behavior (cold-start share opens the sheet pre-filled; once-only across back-nav) needs on-device verification — see PR. Closes #1679 Refs #472 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Opening a fiction with progress left the chapter list at the top; the listener had to scroll to find where they were. Now, on first open, the list scrolls to the resume chapter (first-unfinished-or-first, via the existing pickChapterToPlay) in both the wide (tablet) and narrow (phone) layouts. - New pure `targetScrollIndex(chapters, resumeId)` — the chapter's index, or null (empty / id-absent → no scroll). CI-provable core; JVM-tested in TargetScrollIndexTest (pure cases + composed with pickChapterToPlay: fresh→0, partway→first-unfinished, all-finished→0, empty→no-scroll). - Per-layout once-only `LaunchedEffect` → `scrollToItem(target + headerOffset)`, reusing the jump feature's wideHeaderOffset / narrowHeaderCount. A rememberSaveable guard makes it fire once on first chapter-load so it never fights manual scrolling or re-fires on chapter-list updates (download / cache state), and survives config change. Scope note: this is the "last-played on open" half. Live auto-follow of the currently-PLAYING chapter is deferred — FictionDetail doesn't observe the playing-chapter id today (ChapterCard currentId is hardcoded null), so that needs new playback-state wiring + a manual-scroll-interaction decision. Tracked on the issue. => Progresses #1676 (not Closes). Device-verify (JP): open a book with progress → list opens scrolled to the resume chapter (wide + narrow); manual scroll afterwards isn't yanked back. Progresses #1676 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Builds on #1595 (ShakeDetector + 12 m/s² threshold, reused as-is) and #1609 (main-marshalled pause/resume). After the sleep timer fires and pauses, the accelerometer keeps listening for a ~60s grace window; a qualifying shake resumes from the paused position and re-arms a fresh timer at the SAME mode that just elapsed. After the window with no shake, the sensor is released so a stopped timer never drains battery overnight. - SleepTimer: new `timerFired: SharedFlow<SleepTimerMode>` (replay=0 event; cancel() emits nothing) carrying the fired mode — solves fire-vs-cancel AND "same duration as last" in one signal. Emitted at the single fire point (fadeAndPause). Concrete class → no fakes affected. - DefaultPlaybackController: concrete-only `timerFired` val delegating to SleepTimer — the PlaybackController interface is untouched, so its fakes need no change. - StoryvoxPlaybackService: one accelerometer owner (`refreshShakeListening`) armed for fade-tail OR grace window; `shakeGraceJob` opens/closes the grace window on `timerFired`; onShake revives (resume + re-arm mode) when in-window. - SleepTimerDecisions: pure `shouldListenInGraceWindow` / `shouldReviveOnShake` + JVM tests in SleepTimerAutoArmTest (fire→listen, boundary, expiry, no-fire). CI proves compile + the pure decisions; sensor-listening + resume + re-arm are runtime → device-verify by JP (shake is physical). green+OPEN for JP. Device-verify (JP): timer stops → shake within 60s resumes + re-arms same duration; no shake within 60s → accelerometer released (no drain); user- cancelled timer opens no window. Closes #1618 Refs #1595 #1609 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements: a post-stop shake-to-revive grace window for the sleep timer (Issue #1618), automatic scrolling to the resume chapter upon opening the chapter list (Issue #1676), and pre-filling the Add-by-URL sheet when launched from a shared URL or clipboard link (Issue #1679). The review feedback correctly identifies a bug in FictionDetailScreen.kt where the narrowHeaderCount is underestimated by 2 because it omits CompanionRow and HighlightsSection, which causes the scroll to land offset from the intended target.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Issue #1676 — bring the resume chapter into view on first open, | ||
| // once-only (see the wide-layout note above). Reuses [narrowHeaderCount]. | ||
| var narrowDidResumeScroll by androidx.compose.runtime.saveable.rememberSaveable { | ||
| mutableStateOf(false) | ||
| } | ||
| androidx.compose.runtime.LaunchedEffect(narrowFiltered.isNotEmpty()) { | ||
| if (!narrowDidResumeScroll && narrowFiltered.isNotEmpty()) { | ||
| targetScrollIndex(narrowFiltered, pickChapterToPlay(state.chapters)?.id) | ||
| ?.let { narrowListState.scrollToItem(it + narrowHeaderCount) } | ||
| narrowDidResumeScroll = true | ||
| } | ||
| } |
There was a problem hiding this comment.
The narrowHeaderCount calculation currently omits CompanionRow and HighlightsSection, both of which are unconditionally added as item blocks in the LazyColumn (occupying indices 5 and 7 respectively). Because of this, narrowHeaderCount is underestimated by exactly 2, causing the list to scroll to an item 2 positions above the intended target. Redefining a corrected header count that adds 2 resolves this offset issue.
// Issue #1676 — bring the resume chapter into view on first open,
// once-only (see the wide-layout note above). Reuses [narrowHeaderCount]
// adjusted for CompanionRow and HighlightsSection items.
val correctedHeaderCount = narrowHeaderCount + 2
var narrowDidResumeScroll by androidx.compose.runtime.saveable.rememberSaveable {
mutableStateOf(false)
}
androidx.compose.runtime.LaunchedEffect(narrowFiltered.isNotEmpty()) {
if (!narrowDidResumeScroll && narrowFiltered.isNotEmpty()) {
targetScrollIndex(narrowFiltered, pickChapterToPlay(state.chapters)?.id)
?.let { narrowListState.scrollToItem(it + correctedHeaderCount) }
narrowDidResumeScroll = true
}
}
Throwaway CI build combining PRs #1692 + #1695 + #1696 so their debug APK can be device-verified on R5CRB0W66MK in one install. Will be closed after verification; the three real PRs merge individually.
🤖 Generated with Claude Code