diff options
| -rw-r--r-- | BACKLOG.md | 56 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt | 3 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt | 169 |
3 files changed, 152 insertions, 76 deletions
@@ -48,36 +48,36 @@ extracted or feed-provided HTML and swap it for a tap-to-play thumbnail tap, rather than trying to make the embed actually play inline (which would mean turning JS back on for untrusted third-party content). -## Bug report button +## Bottom action strip (partially done) -**Motivation:** every crash/bug so far has been diagnosed by manually pulling -logcat and querying the on-device DB over adb -- fine on tungsten's emulator, -not available from the user's actual phone in the moment something breaks. +**Done:** swipe-to-next-article dropped entirely (two different attempts at +keeping it from interfering with WebView scrolling both failed to actually +fix the reported jerkiness, and the second broke swipe outright). Replaced +with explicit Previous/Next buttons in a new bottom bar -- no gesture to +disambiguate with scrolling at all now. -**Rough shape (not yet built):** a "Report bug" action (article overflow -menu, or a dedicated crash-handler hook) that gathers what's needed to -reproduce in the emulator -- the current article's URL/id, a short -user-typed description, and whatever we've already instrumented internally -(feed id, reader-mode state, isReaderMode/extraction state, recent logcat -lines if reachable) -- and copies it to the clipboard as one block of text, -ready to paste back into a message. +**Still open:** the top bar is still crowded (reader-toggle, open-in-browser, +bookmark, Stash, overflow) and top-heavy for one-handed use. Move those +action icons + the overflow menu down into the bottom bar alongside +Previous/Next, and auto-hide both the top and bottom bars together on +scroll (hide on scroll-down, reveal on scroll-up or tap), matching the +common "immersive reading" pattern other reader apps use. -## Smaller title text +## Scroll jerkiness (still open, root cause unknown) -Article titles in the reader's top bar (and the list rows) read too large -relative to everything else. Reduce the type scale -- exact size TBD by eye -once tried. +Reported as "jerky, loses track of my motion." Two theories tried and +ruled out live, not just in code review: +- An orientation-aware Compose gesture detector wrapping the WebView (for + swipe-to-next-article) -- kept working correctly for paging, but didn't + fix the reported jerkiness either, so it likely wasn't (only) about + gesture-arbitration overhead from that detector. +- A passive native OnTouchListener replacing the above -- broke swipe + outright, reverted, and jerkiness was never re-tested in isolation since + swipe was still present in that build. -## Bottom action strip - -**Motivation:** the reader's top bar is getting crowded (reader-toggle, -open-in-browser, bookmark, Stash, overflow) and it's a top-heavy layout for -a one-handed phone -- reachability, not just tidiness. - -**Rough shape (not yet built):** move the action icons + overflow menu into -a bottom strip (title/back stay in the top bar). Auto-hide both the top and -bottom strips together on scroll (hide on scroll-down, reveal on scroll-up -or tap), matching the common "immersive reading" pattern other reader apps -use. Needs to coexist with the existing scroll-vs-swipe gesture handling in -ReaderScreen without reintroducing the jank/interception problems already -fought through there. +Swipe is now gone entirely (see "Bottom action strip" above), which removes +one whole category of possible interference. Worth a fresh look at whether +scrolling still feels jerky now that nothing above the WebView is doing any +gesture detection at all -- if it still does, the cause is something else +(WebView rendering/compositing itself, the emulator's software rendering +not reflecting real-device feel, etc.), not gesture interception. diff --git a/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt index 4a67a3a..c3a398b 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt @@ -185,7 +185,8 @@ private fun ArticleRow( headlineContent = { Text( article.title, - textDecoration = if (article.isRead) TextDecoration.LineThrough else null + textDecoration = if (article.isRead) TextDecoration.LineThrough else null, + style = MaterialTheme.typography.bodyMedium ) }, supportingContent = { if (metaText != null) Text(metaText) }, diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt index 82cb792..a46d1aa 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -1,12 +1,18 @@ package org.terst.feedreader.ui +import android.content.ClipData +import android.content.ClipboardManager +import android.content.Context import android.content.Intent import android.net.Uri import androidx.compose.foundation.ExperimentalFoundationApi -import androidx.compose.foundation.gestures.detectHorizontalDragGestures import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.pager.HorizontalPager import androidx.compose.foundation.pager.rememberPagerState @@ -18,8 +24,12 @@ import androidx.compose.material.icons.filled.BookmarkBorder import androidx.compose.material.icons.filled.CloudDone import androidx.compose.material.icons.filled.CloudUpload import androidx.compose.material.icons.filled.MoreVert +import androidx.compose.material.icons.filled.NavigateBefore +import androidx.compose.material.icons.filled.NavigateNext import androidx.compose.material.icons.filled.OpenInBrowser import androidx.compose.material.icons.outlined.Article +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.BottomAppBar import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem @@ -27,10 +37,12 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -41,9 +53,7 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView @@ -103,49 +113,34 @@ fun ReaderScreen( val initialIndex = safeIds.indexOf(initialArticleId).coerceAtLeast(0) val pagerState = rememberPagerState(initialPage = initialIndex) { safeIds.size } val scope = rememberCoroutineScope() - val swipeThresholdPx = with(LocalDensity.current) { 80.dp.toPx() } - // Tried moving swipe detection to a passive native OnTouchListener on the WebView - // (always returns false, never consumes) to get Compose's gesture arbitration - // completely out of the touch path, on the theory that it was the source of reported - // scroll jerkiness. Reverted: swipe-to-next-article stopped working at all with that - // approach (confirmed live), which is a worse regression than unproven jerkiness. - // Back to the known-working Compose detector -- orientation-aware touch-slop - // detection, so it doesn't grab vertical-dominant drags, confirmed live repeatedly. - Box( - modifier = Modifier - .fillMaxSize() - .pointerInput(safeIds.size) { - var totalDrag = 0f - detectHorizontalDragGestures( - onDragStart = { totalDrag = 0f }, - onDragCancel = { totalDrag = 0f }, - onDragEnd = { - val direction = when { - totalDrag <= -swipeThresholdPx -> 1 - totalDrag >= swipeThresholdPx -> -1 - else -> 0 - } - if (direction != 0) { - val target = (pagerState.currentPage + direction).coerceIn(0, safeIds.size - 1) - scope.launch { pagerState.animateScrollToPage(target) } - } - }, - onHorizontalDrag = { change, dragAmount -> - change.consume() - totalDrag += dragAmount - } - ) - } - ) { - HorizontalPager(state = pagerState, userScrollEnabled = false, modifier = Modifier.fillMaxSize()) { page -> - ReaderContent( - articleId = safeIds[page], - onBack = onBack, - onOpenLink = onOpenLink, - onOpenBrowser = onOpenBrowser - ) - } + // Swipe-to-next-article is gone -- two different approaches to keeping it from + // interfering with WebView scrolling (an orientation-aware Compose gesture detector, + // then a passive native touch listener) were tried and neither actually resolved the + // reported jerkiness, and the second one broke swipe outright. Explicit Previous/Next + // buttons in the bottom bar replace it: no ambiguity with scrolling at all, since + // there's no gesture to disambiguate in the first place. + val currentPage = pagerState.currentPage + val hasPrevious = currentPage > 0 + val hasNext = currentPage < safeIds.size - 1 + fun goToPreviousArticle() { + scope.launch { pagerState.animateScrollToPage((pagerState.currentPage - 1).coerceAtLeast(0)) } + } + fun goToNextArticle() { + scope.launch { pagerState.animateScrollToPage((pagerState.currentPage + 1).coerceAtMost(safeIds.size - 1)) } + } + + HorizontalPager(state = pagerState, userScrollEnabled = false, modifier = Modifier.fillMaxSize()) { page -> + ReaderContent( + articleId = safeIds[page], + onBack = onBack, + onOpenLink = onOpenLink, + onOpenBrowser = onOpenBrowser, + hasPrevious = hasPrevious, + hasNext = hasNext, + onPrevious = ::goToPreviousArticle, + onNext = ::goToNextArticle + ) } } @@ -155,7 +150,11 @@ private fun ReaderContent( articleId: String, onBack: () -> Unit, onOpenLink: (String) -> Unit, - onOpenBrowser: (String) -> Unit + onOpenBrowser: (String) -> Unit, + hasPrevious: Boolean, + hasNext: Boolean, + onPrevious: () -> Unit, + onNext: () -> Unit ) { val context = LocalContext.current val db = remember { AppDatabase.get(context) } @@ -170,6 +169,8 @@ private fun ReaderContent( var isExtracting by remember(articleId) { mutableStateOf(false) } var isSaving by remember(articleId) { mutableStateOf(false) } var moreMenuExpanded by remember(articleId) { mutableStateOf(false) } + var showBugReportDialog by remember(articleId) { mutableStateOf(false) } + var bugReportDescription by remember(articleId) { mutableStateOf("") } LaunchedEffect(articleId) { val loaded = db.articleDao().getById(articleId) ?: return@LaunchedEffect @@ -236,7 +237,12 @@ private fun ReaderContent( TopAppBar( title = { Column { - Text(article?.title.orEmpty(), maxLines = 1, overflow = TextOverflow.Ellipsis) + Text( + article?.title.orEmpty(), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + style = MaterialTheme.typography.titleMedium + ) feed?.title?.let { Text( it, @@ -343,12 +349,81 @@ private fun ReaderContent( onOpenBrowser(a.link) } ) + DropdownMenuItem( + text = { Text("Report bug") }, + onClick = { + moreMenuExpanded = false + showBugReportDialog = true + } + ) } } ) }, + bottomBar = { + BottomAppBar { + TextButton( + onClick = onPrevious, + enabled = hasPrevious, + modifier = Modifier.weight(1f) + ) { + Icon(Icons.Default.NavigateBefore, contentDescription = null) + Text("Previous") + } + TextButton( + onClick = onNext, + enabled = hasNext, + modifier = Modifier.weight(1f) + ) { + Text("Next") + Icon(Icons.Default.NavigateNext, contentDescription = null) + } + } + }, snackbarHost = { SnackbarHost(snackbarHostState) } ) { padding -> + if (showBugReportDialog) { + val a = article + AlertDialog( + onDismissRequest = { showBugReportDialog = false }, + title = { Text("Report bug") }, + text = { + Column { + Text("Describe what happened. Copies article/feed state along with it so it can be reproduced.") + Spacer(modifier = Modifier.height(12.dp)) + OutlinedTextField( + value = bugReportDescription, + onValueChange = { bugReportDescription = it }, + label = { Text("Description") }, + modifier = Modifier.fillMaxWidth() + ) + } + }, + confirmButton = { + TextButton(onClick = { + val report = buildString { + appendLine("Feeds bug report") + appendLine("Description: ${bugReportDescription.ifBlank { "(none provided)" }}") + appendLine("Article title: ${a?.title}") + appendLine("Article URL: ${a?.link}") + appendLine("Article id: ${a?.id}") + appendLine("Feed: ${feed?.title} (${feed?.id})") + appendLine("Feed readerModeEnabled: ${feed?.readerModeEnabled}") + appendLine("Session isReaderMode: $isReaderMode") + appendLine("isSaved: ${a?.isSaved}, stashArtifactId: ${a?.stashArtifactId}") + appendLine("extractedContent cached: ${readerContent != null}") + } + val clipboard = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager + clipboard.setPrimaryClip(ClipData.newPlainText("Bug report", report)) + showBugReportDialog = false + scope.launch { snackbarHostState.showSnackbar("Copied to clipboard") } + }) { Text("Copy") } + }, + dismissButton = { + TextButton(onClick = { showBugReportDialog = false }) { Text("Cancel") } + } + ) + } Box(modifier = Modifier.fillMaxSize().padding(padding)) { val content = if (isReaderMode) readerContent else (feedContent ?: readerContent) val baseUrl = article?.link |