Feature/extraction error log misc#155
Merged
Merged
Conversation
When Readeck reports extraction errors for a bookmark, the details panel now shows a collapsible error box (collapsed by default) above the thumbnail. Tapping it reveals the error list, a help note, a Learn more link to the Readeck extension docs, and a View log button that fetches and displays the raw extraction log in a modal dialog with copy-to-clipboard and a scroll progress indicator. Fixes detection by mapping hasServerErrors through to the domain model so bookmarks flagged via replaceServerErrorFlags (but with an empty errors list) correctly trigger the error box. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace fixed 200dp crop with aspect-ratio-aware sizing. Uses rememberAsyncImagePainter to reactively compute the natural height at full card width, then applies one of three zones: crop up to 180dp minimum for wide/panoramic images, show full image at natural height for normal images, or crop down to 50% screen height for tall portrait images. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The reader HTML clearance was derived from TopAppBarExpandedHeight + statusBars.top, which produced a spacer exactly equal to the bar height. On Pixel 9 the title's line-box half-leading was enough to clear the bar visually, but on Pixel Tablet sub-pixel rounding pushed the cap height behind the bar. Measure the bar's actual rendered height via onSizeChanged and add a 4 dp cushion so the title always clears. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Doc cleanup: add the mini-specs for the reader top-bar clearance fix and the extraction error box + log viewer feature that shipped without their accompanying spec files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Slice 4 drawer-gate change set gesturesEnabled = false whenever swipe actions were enabled on the bookmark list, which Material 3's ModalNavigationDrawer also uses to gate scrim-tap-to-close and swipe-to-close. The first attempt to keep gestures live with || !isOnBookmarkList overcorrected and re-enabled drawer-swipe-to-open on every other screen, including the reader. Restore the original bookmark-list-only swipe-to-open scope while keeping gestures live whenever the drawer is open, so scrim-tap and swipe-close work regardless of which screen the user is on. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The reader top bar uses enterAlwaysScrollBehavior, which collapses and expands the bar as the user scrolls. The onSizeChanged callback added in 9392327 fires on every collapse step, updating measuredTopBarHeightPx each frame. That cascades into a recomputed CSS spacer for the WebView and recomputed Compose padding, causing content reflow during scroll. Read-progress restoration then yanks the scroll position around — most visibly, a 100%-read article jumps back to the bottom on any scroll-down attempt, and any article scrolls erratically. Gate the measurement update so it only captures the bar's maximum observed height, locking the spacer to the fully-expanded bar height. The bar's collapse animation no longer feeds back into the WebView's content height. Preserves the original tablet sub-pixel rounding fix (measurement still wins over the static fallback) and the 4dp cushion. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The implementation pushed in 6db1839 could not be validated locally — we lack a highlights dataset large enough (thousands across years) to iterate on the fast-scroll behavior, and the one tester with that data couldn't confirm it worked. Rather than ship unverified code, back out the feature. The original commit is preserved at the tag wip/highlights-fast-scroll for future revival. The spec doc is kept (with a shelved-status note prepended) so the design intent and reference material remain discoverable in-tree. The plan: build the same fast-scroll pattern for the main bookmark list first, where we have test data, then port the proven pattern back to highlights. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The max-seen guard added previously closed the continuous-thrash bug, but one residual issue remained: between first composition (clearance derived from the static fallback) and the post-layout onSizeChanged callback (clearance derived from the measured height), readerTopClearance CssPx changes value once. Because it keys the WebView's content remember and LaunchedEffect, that one-step change regenerates the article HTML and reloads the WebView. A user who starts scrolling immediately after the article opens hits exactly the window where the reload yanks them back to the top. The original 9392327 commit introduced both a measurement and a 4dp cushion to handle Pixel Tablet sub-pixel rounding. The 4dp cushion is multiple orders of magnitude larger than any sub-pixel error — the measurement is redundant. Drop the measurement, keep the cushion. The clearance value is now stable from first composition forward, so the WebView's article HTML is generated exactly once and never reloaded. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Compose's default touch-slop arbitration races the inner horizontal draggable against the outer LazyColumn's vertical scrollable for the first axis to cross slop. That works for clean motion but lets a "scroll then sideways within the same touch" gesture commit a swipe because the late horizontal motion crosses slop first. Add a per-pointer intent fence that decides exactly once, at the moment one axis first crosses platform touch slop, whether the gesture is horizontal or vertical-dominant. If vertical-dominant (dy >= dx), set verticalIntentVeto for the pointer's life so the draggable's enabled flag stays false even if the user later moves cleanly sideways. Locking once (rather than re-checking each event) is critical: if the veto could flip mid-drag, Modifier.draggable would observe enabled=false and call onDragStopped abnormally with whatever offset and velocity it had, sometimes committing a delete past the midpoint without going through the snackbar handshake. The 45° boundary (dy >= dx) rather than a 1.5x ratio is also deliberate: at exactly equal motion neither the inner draggable nor the outer scrollable can resolve axis dominance on its own and the gesture produces no motion at all. Biasing the equality case toward scroll resolves the deadlock and matches user intent — as much vertical as horizontal is too much vertical for a swipe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The static derivation TopAppBarExpandedHeight + statusBars + 4dp was sufficient on Pixel 9 but left the very top edge of article titles clipped under the bar on the Pixel Tablet emulator. The M3 TopAppBar apparently adds a small amount of internal padding on tablet form factors beyond what TopAppBarExpandedHeight reports. 12dp comfortably covers the gap. The extra ~8dp at the top of articles on phones is negligible visually, and the trade is worth it: keeping the value statically derived means the WebView's HTML clearance spacer stays constant across the screen's lifetime, so we don't reintroduce the mid-scroll content reload that the measurement-based approach caused. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The three coEvery/coAnswers blocks that mock performTransaction's three generic instantiations cast invocation.args[0] to the matching suspend lambda type. The casts are safe by construction (mockk wires the right type into each generic call site) but Kotlin can't prove it through the reflective args lookup, so the compiler emits Unchecked cast warnings on each. Suppress locally with @Suppress("UNCHECKED_CAST") and a one-line comment explaining the reasoning. Co-Authored-By: Claude Opus 4.7 <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
1 participant
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
Pile of related work that grew from a single "extraction error log" focus into a broader set of bookmark-details, reader, and swipe-gesture improvements:
wip/highlights-fast-scroll.12 commits, ~2,240 line additions, ~26 deletions across code, tests, locale strings, user-guide markdown, and design specs.
Bookmark details
Commits:
b536b52,b5b3350BookmarkRepository/Bookmarkmodel /ReadeckApito carry the new fields.BookmarkRepositoryImplTestandBookmarkDetailViewModelTestcovering the new fields and dialog state.Specs:
docs/specs/extraction-error-log-detail-panel-mini-spec.md(initial design)docs/archive/extraction-error-log-detail-panel-mini-spec.md(post-ship archive copy)Reader top-bar clearance
Commits:
9392327→0a1228b→686c29d→3543a0aThis area went through several iterations as edge cases surfaced during testing:
9392327introducedonSizeChangedmeasurement of the rendered top bar to fix sub-pixel title clipping on Pixel Tablet.0a1228badded a max-seen guard after testing showed the measurement was firing repeatedly duringenterAlwaysScrollBehaviorcollapses, thrashing the WebView's CSS clearance spacer and causing scroll jumps (100%-read articles snapping to the bottom; fresh articles scrolling erratically).686c29ddropped the measurement entirely once it became clear that the one-shot transition from "static fallback" to "measured value" on first composition was still causing a one-time WebView reload that would yank the user back to the top of the article if they were already scrolling. A static derivation with a 4dp safety cushion replaced it.3543a0abumped the cushion to 12dp after the Pixel Tablet emulator showed that 4dp wasn't enough to clear the M3 TopAppBar's internal padding on tablet form factors.End state:
topBarClearanceis statically derived fromTopAppBarExpandedHeight + WindowInsets.statusBars + 12.dp, stable across the screen's lifetime, so the WebView's article HTML is generated exactly once and never reloaded mid-scroll.Spec:
docs/archive/reader-top-bar-clearance-mini-spec.md.Swipe-action gesture refinements
Commit:
5139d17The original swipe-actions feature shipped on
mainwith Compose's default touch-slop arbitration: whichever axis (horizontal or vertical) crosses platform touch slop first wins the pointer. That works for clean motion, but it lets a "scroll a bit, then move sideways within the same touch" gesture commit a swipe — because the late horizontal motion crosses slop before the vertical motion did.This branch adds a per-pointer vertical-intent fence:
dy >= dx, setverticalIntentVeto = truefor the rest of that pointer's life.enabledflag, not by consuming events, so the parentLazyColumnstill receives the vertical motion normally.Modifier.draggablewould observeenabled = falseand callonDragStoppedabnormally, sometimes committing a delete past the midpoint without going through the snackbar handshake.dy >= dxrather than a 1.5× ratio) is also intentional: at exactly equal motion neither the inner draggable nor the outer scrollable can resolve dominance, and the gesture produces no motion at all. Biasing the equality case toward scroll resolves the deadlock and matches user intent — as much vertical as horizontal is too much vertical for a swipe.Spec updates in
docs/specs/swipe-actions-for-bookmark-cards-spec.mdandarch-spec.mddocument the new behaviour.Drawer regression fix
Commit:
2fa23dcThe Slice 4 drawer-gate change (already on
main) setgesturesEnabledonModalNavigationDrawerto a value that, when card swipe was enabled, correctly suppressed drawer-swipe-to-open — but also disabled M3's scrim-tap-to-close and swipe-to-close, leaving the drawer effectively unclosable except via menu item selection.Fix: keep gestures live whenever the drawer is already open, by OR-ing
drawerState.isOpeninto the gate.Highlights date fast-scroll — shelved
Commits:
6db1839(implementation),e915446(revert + spec preservation)A Google-Photos-style date fast-scroll overlay for the highlights list was implemented and pushed to a snapshot build for a tester with thousands of highlights spanning years. The tester couldn't validate the feature, and there's no local dataset large enough to iterate on it.
Rather than ship unverified code, the feature was reverted. The plan is to first build the equivalent fast-scroll pattern for the main bookmark-list views (where there is test data), validate it there, then port back to highlights.
Preservation:
wip/highlights-fast-scroll(pushed to origin).docs/specs/highlights-date-fast-scroll-spec.mdwas kept in-tree, with a "Shelved" banner at the top pointing at the tag.Documentation
Commits:
b46f9cd,d46e6acdocs/archive/.docs/specs/bookmark-compact-date-fast-scroll-prototype-spec.md— proposed first home for the fast-scroll pattern, where data is available.docs/specs/list-multi-select-actions-spec.md— multi-select mode for the bookmark list (tablet/landscape batch UX)..gitignoreupdates.Test plan
./gradlew :app:assembleDebugAll :app:testDebugUnitTestAll :app:lintDebugAllall pass.Notes for reviewers
docs/specs/swipe-actions-for-bookmark-cards-arch-spec.md§3.1, including why the decision is locked once per gesture and why the boundary is at 45° rather than the more permissive 1.5× ratio.git show wip/highlights-fast-scrollor by cherry-picking the tag. The spec doc explains both the original design and the plan to port the pattern from a future main-list fast-scroll prototype.