From bedf8a47a8b0601f4e3c253e4d2a7325c6ccbfc6 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Wed, 22 Jul 2026 18:56:47 +0000 Subject: Fix accidental page-swipe while scrolling; bring linked-article reader to parity - ReaderScreen: HorizontalPager's full-surface drag detection was treating ordinary vertical scrolling inside the embedded WebView (a native View, which doesn't arbitrate with Compose's gesture system the way a pure-Compose scrollable child would) as an attempted horizontal page-turn on anything less than a perfectly vertical swipe -- i.e. most real thumb scrolling. Tried gating it to edge-zone-only drags first, but that collides with Android's own edge-swipe-back gesture on gesture-nav devices (reproduced live: an edge swipe triggered system back, not the pager). Landed on disabling the Pager's built-in gesture and driving it from a custom detector that works anywhere on screen but only commits to a page turn once a drag is clearly horizontal-dominant (>2x the vertical movement) -- ordinary scrolling, even sloppy/diagonal, is left completely unconsumed for the WebView. Verified live: a deliberately diagonal mid-screen scroll now stays on the same article; a decisive horizontal swipe still changes pages. - LinkReaderScreen: was missing the bookmark/Stash/reader-toggle/menu actions entirely -- reported after tapping a link left "all my icons... gone." Brought to parity with the main article reader: save locally and save to Stash now lazily create a standalone ArticleEntity (feedId=null, id = hash of the URL, same scheme ShareReceiverActivity uses) on first use; reader-mode toggle falls back to the live page (JS on) instead of always forcing extraction; same login/find-another-source menu. Verified live: all 5 icons present once extraction finishes, bookmarking a linked article correctly creates a standalone DB row. --- .../org/terst/feedreader/ui/LinkReaderScreen.kt | 209 ++++++++++++++++++--- .../java/org/terst/feedreader/ui/MainActivity.kt | 3 +- .../java/org/terst/feedreader/ui/ReaderScreen.kt | 77 +++++++- 3 files changed, 250 insertions(+), 39 deletions(-) (limited to 'app/src/main/java/org/terst/feedreader') diff --git a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt index ff764c4..1a715b0 100644 --- a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt @@ -1,19 +1,32 @@ package org.terst.feedreader.ui +import android.annotation.SuppressLint import android.content.Intent import android.net.Uri +import android.webkit.WebView import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Article import androidx.compose.material.icons.filled.ArrowBack +import androidx.compose.material.icons.filled.Bookmark +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.OpenInBrowser +import androidx.compose.material.icons.outlined.Article import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable @@ -21,47 +34,97 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.style.TextOverflow -import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView +import kotlinx.coroutines.launch +import org.terst.feedreader.data.AppDatabase +import org.terst.feedreader.data.ArticleEntity +import org.terst.feedreader.data.Net +import org.terst.feedreader.data.Settings import org.terst.feedreader.readability.ReaderExtractor +import org.terst.feedreader.stash.StashClient +import org.terst.feedreader.stash.StashSaveResult +import java.net.URLEncoder +import java.security.MessageDigest + +private fun urlToArticleId(url: String): String = + MessageDigest.getInstance("SHA-256").digest(url.toByteArray()).joinToString("") { "%02x".format(it) } /** - * A lightweight, non-persisted reader view for links tapped inside an article's - * WebView -- extracts the linked page with the same Readability pipeline as the - * main reader, but doesn't touch the DB (it's not a subscribed article), and - * recursively re-opens itself for any further links tapped inside, so link-hopping - * stays inside the app's reader instead of dropping into a raw WebView or the - * system browser. + * A reader view for links tapped inside an article's WebView -- extracts the linked + * page with the same Readability pipeline as the main reader, and recursively re-opens + * itself for further links, so link-hopping stays inside the app's reader instead of + * dropping into the system browser. Brought to parity with the main article reader: + * save locally / save to Stash (lazily creates a standalone ArticleEntity, feedId=null, + * keyed by a hash of the URL -- same scheme ShareReceiverActivity uses, so sharing the + * same link later lands on the same row instead of duplicating it), a reader-mode + * toggle (falls back to the live page, JS on, when the user wants the original instead + * of the extracted view or extraction failed), and the same login/search-alternate menu. */ +@SuppressLint("SetJavaScriptEnabled") @OptIn(ExperimentalMaterial3Api::class) @Composable -fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Unit) { +fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Unit, onOpenBrowser: (String) -> Unit) { val context = LocalContext.current - var title by remember(url) { mutableStateOf(null) } - var content by remember(url) { mutableStateOf(null) } - var failed by remember(url) { mutableStateOf(false) } + val db = remember { AppDatabase.get(context) } + val scope = rememberCoroutineScope() + val snackbarHostState = remember { SnackbarHostState() } + val articleId = remember(url) { urlToArticleId(url) } + + var article by remember(url) { mutableStateOf(null) } + var extractedTitle by remember(url) { mutableStateOf(null) } + var extractedContent by remember(url) { mutableStateOf(null) } + var extractionFailed by remember(url) { mutableStateOf(false) } + var isReaderMode by remember(url) { mutableStateOf(true) } + var isSaving by remember(url) { mutableStateOf(false) } + var moreMenuExpanded by remember(url) { mutableStateOf(false) } LaunchedEffect(url) { - val extracted = ReaderExtractor.extract(context, url) - val extractedContent = extracted?.content - if (extractedContent != null) { - title = extracted.title - content = extractedContent + val existing = db.articleDao().getById(articleId) + article = existing + if (existing?.extractedContent != null) { + extractedTitle = existing.title + extractedContent = existing.extractedContent } else { - failed = true + val extracted = ReaderExtractor.extract(context, url) + val content = extracted?.content + if (content != null) { + extractedTitle = extracted.title + extractedContent = content + } else { + extractionFailed = true + isReaderMode = false + } } } + // Both save actions create the standalone row on first use (there's nothing to save + // until the user asks) -- subsequent taps just update the existing one. + suspend fun ensureArticleRow(): ArticleEntity { + article?.let { return it } + val a = ArticleEntity( + id = articleId, + feedId = null, + title = extractedTitle ?: url, + link = url, + extractedContent = extractedContent, + createdAt = System.currentTimeMillis() + ) + db.articleDao().upsert(a) + article = a + return a + } + Scaffold( topBar = { TopAppBar( title = { - Text(title ?: Uri.parse(url).host ?: "Article", maxLines = 1, overflow = TextOverflow.Ellipsis) + Text(extractedTitle ?: Uri.parse(url).host ?: "Article", maxLines = 1, overflow = TextOverflow.Ellipsis) }, navigationIcon = { IconButton(onClick = onBack) { @@ -69,42 +132,126 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un } }, actions = { + val currentArticle = article + if (extractedContent != null) { + IconButton(onClick = { isReaderMode = !isReaderMode }) { + Icon( + if (isReaderMode) Icons.Filled.Article else Icons.Outlined.Article, + contentDescription = "Toggle reader view" + ) + } + } IconButton(onClick = { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(url))) }) { Icon(Icons.Default.OpenInBrowser, contentDescription = "Open in browser") } + IconButton(onClick = { + scope.launch { + val a = ensureArticleRow() + val newSaved = !a.isSaved + article = a.copy(isSaved = newSaved) + db.articleDao().setSaved(a.id, newSaved) + } + }) { + Icon( + if (currentArticle?.isSaved == true) Icons.Default.Bookmark else Icons.Default.BookmarkBorder, + contentDescription = "Save locally" + ) + } + IconButton( + enabled = !isSaving, + onClick = { + isSaving = true + scope.launch { + val a = ensureArticleRow() + val settings = Settings(context) + val client = StashClient(Net.client, settings.stashBaseUrlNow(), settings.stashApiKeyNow()) + when (val result = client.saveArticle(a.link, a.title)) { + is StashSaveResult.Saved -> { + db.articleDao().setStashArtifactId(a.id, result.artifact.id) + article = a.copy(stashArtifactId = result.artifact.id) + snackbarHostState.showSnackbar("Saved to Stash") + } + is StashSaveResult.AlreadySaved -> { + db.articleDao().setStashArtifactId(a.id, result.existingId) + article = a.copy(stashArtifactId = result.existingId) + snackbarHostState.showSnackbar("Already in Stash") + } + is StashSaveResult.Failed -> + snackbarHostState.showSnackbar("Save failed: ${result.message}") + } + isSaving = false + } + } + ) { + Icon( + if (currentArticle?.stashArtifactId != null) Icons.Default.CloudDone else Icons.Default.CloudUpload, + contentDescription = "Save to Stash" + ) + } + IconButton(onClick = { moreMenuExpanded = true }) { + Icon(Icons.Default.MoreVert, contentDescription = "More") + } + DropdownMenu(expanded = moreMenuExpanded, onDismissRequest = { moreMenuExpanded = false }) { + DropdownMenuItem( + text = { Text("Find another source") }, + onClick = { + moreMenuExpanded = false + val query = URLEncoder.encode(extractedTitle ?: url, "UTF-8") + onOpenBrowser("https://www.google.com/search?q=$query") + } + ) + DropdownMenuItem( + text = { Text("Log in to this site") }, + onClick = { + moreMenuExpanded = false + onOpenBrowser(url) + } + ) + } } ) - } + }, + snackbarHost = { SnackbarHost(snackbarHostState) } ) { padding -> Box(modifier = Modifier.fillMaxSize().padding(padding)) { - val currentContent = content + val currentExtractedContent = extractedContent when { - currentContent != null -> { + isReaderMode && currentExtractedContent != null -> { val loadedContent = remember(url) { mutableStateOf(null) } AndroidView( modifier = Modifier.fillMaxSize(), factory = { ctx -> newReaderWebView(ctx, onOpenLink).apply { - loadDataWithBaseURL(url, wrapReaderHtml(currentContent), "text/html", "UTF-8", null) - loadedContent.value = currentContent + loadDataWithBaseURL(url, wrapReaderHtml(currentExtractedContent), "text/html", "UTF-8", null) + loadedContent.value = currentExtractedContent } }, update = { webView -> - if (loadedContent.value != currentContent) { - webView.loadDataWithBaseURL(url, wrapReaderHtml(currentContent), "text/html", "UTF-8", null) - loadedContent.value = currentContent + if (loadedContent.value != currentExtractedContent) { + webView.loadDataWithBaseURL(url, wrapReaderHtml(currentExtractedContent), "text/html", "UTF-8", null) + loadedContent.value = currentExtractedContent + } + }, + onRelease = { webView -> webView.destroy() } + ) + } + // Reader mode off, or extraction failed -- fall back to the live page + // itself (JS on) rather than leaving the screen blank. + !isReaderMode || extractionFailed -> { + AndroidView( + modifier = Modifier.fillMaxSize(), + factory = { ctx -> + WebView(ctx).apply { + settings.javaScriptEnabled = true + settings.domStorageEnabled = true + loadUrl(url) } }, onRelease = { webView -> webView.destroy() } ) } - failed -> Text( - "Couldn't extract a reader view for this page -- try \"Open in browser\" instead.", - modifier = Modifier.align(Alignment.Center).padding(24.dp), - color = MaterialTheme.colorScheme.onSurfaceVariant - ) else -> CircularProgressIndicator(modifier = Modifier.align(Alignment.Center)) } } diff --git a/app/src/main/java/org/terst/feedreader/ui/MainActivity.kt b/app/src/main/java/org/terst/feedreader/ui/MainActivity.kt index bd09393..43e1680 100644 --- a/app/src/main/java/org/terst/feedreader/ui/MainActivity.kt +++ b/app/src/main/java/org/terst/feedreader/ui/MainActivity.kt @@ -103,7 +103,8 @@ fun FeedReaderApp() { LinkReaderScreen( url = url, onBack = { navController.popBackStack() }, - onOpenLink = { nextUrl -> navController.navigate("linkReader/${encodeNavArg(nextUrl)}") } + onOpenLink = { nextUrl -> navController.navigate("linkReader/${encodeNavArg(nextUrl)}") }, + onOpenBrowser = { browserUrl -> navController.navigate("browser/${encodeNavArg(browserUrl)}") } ) } composable( 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 8c2f64e..9b53f8c 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -3,6 +3,7 @@ package org.terst.feedreader.ui import android.content.Intent import android.net.Uri import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.gestures.detectDragGestures import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize @@ -40,8 +41,11 @@ 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 import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch @@ -54,6 +58,7 @@ import org.terst.feedreader.readability.ReaderExtractor import org.terst.feedreader.stash.StashClient import org.terst.feedreader.stash.StashSaveResult import java.net.URLEncoder +import kotlin.math.abs // Below this length (of the plain feed-provided HTML), assume it's a teaser/summary, // not the real article body, and auto-fall-back to on-device Readability extraction. @@ -100,14 +105,72 @@ fun ReaderScreen( val safeIds = if (ids.contains(initialArticleId)) ids else listOf(initialArticleId) val initialIndex = safeIds.indexOf(initialArticleId).coerceAtLeast(0) val pagerState = rememberPagerState(initialPage = initialIndex) { safeIds.size } + val scope = rememberCoroutineScope() + val directionLockPx = with(LocalDensity.current) { 16.dp.toPx() } + val swipeThresholdPx = with(LocalDensity.current) { 80.dp.toPx() } - HorizontalPager(state = pagerState, modifier = Modifier.fillMaxSize()) { page -> - ReaderContent( - articleId = safeIds[page], - onBack = onBack, - onOpenLink = onOpenLink, - onOpenBrowser = onOpenBrowser - ) + // HorizontalPager's own full-surface drag detection was grabbing ordinary vertical + // scrolling inside the embedded WebView (a native View, which doesn't arbitrate with + // Compose's gesture system the way a pure-Compose scrollable child would) as an + // attempted horizontal page-turn on anything less than a perfectly vertical swipe -- + // which is most real thumb scrolling. An edge-zone-gated version of this was tried + // first, but on gesture-nav Android the screen edges belong to the OS's own back + // gesture, which wins before the app ever sees the touch (confirmed live: an edge + // swipe triggered system back, not the pager). So instead: disable the Pager's own + // gesture and drive it from a detector that works anywhere on screen but only commits + // to a page turn once the drag is clearly horizontal-dominant (>2x the vertical + // movement) -- ordinary scrolling, even sloppy/diagonal, stays vertical-dominant and + // is left completely unconsumed for the WebView to handle normally. + Box( + modifier = Modifier + .fillMaxSize() + .pointerInput(safeIds.size) { + var totalDx = 0f + var totalDy = 0f + var committedHorizontal = false + detectDragGestures( + onDragStart = { + totalDx = 0f + totalDy = 0f + committedHorizontal = false + }, + onDragCancel = { committedHorizontal = false }, + onDragEnd = { + if (committedHorizontal) { + val direction = when { + totalDx <= -swipeThresholdPx -> 1 + totalDx >= swipeThresholdPx -> -1 + else -> 0 + } + if (direction != 0) { + val target = (pagerState.currentPage + direction).coerceIn(0, safeIds.size - 1) + scope.launch { pagerState.animateScrollToPage(target) } + } + } + committedHorizontal = false + }, + onDrag = { change, dragAmount -> + totalDx += dragAmount.x + totalDy += dragAmount.y + if (!committedHorizontal && + abs(totalDx) > directionLockPx && + abs(totalDx) > abs(totalDy) * 2f + ) { + committedHorizontal = true + } + if (committedHorizontal) change.consume() + } + ) + } + ) { + HorizontalPager(state = pagerState, userScrollEnabled = false, modifier = Modifier.fillMaxSize()) { page -> + ReaderContent( + articleId = safeIds[page], + onBack = onBack, + onOpenLink = onOpenLink, + onOpenBrowser = onOpenBrowser + ) + } } } -- cgit v1.2.3