diff options
| -rw-r--r-- | BACKLOG.md | 34 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/data/AppDatabase.kt | 17 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/data/FeedParser.kt | 10 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt | 18 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt | 21 |
5 files changed, 73 insertions, 27 deletions
@@ -47,3 +47,37 @@ extracted or feed-provided HTML and swap it for a tap-to-play thumbnail (image + play button) that opens the video in the YouTube app/browser on 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 + +**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. + +**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. + +## Smaller title text + +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. + +## 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. diff --git a/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt b/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt index 441556b..66722fd 100644 --- a/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/feedreader/data/AppDatabase.kt @@ -9,7 +9,7 @@ import androidx.room.migration.Migration @Database( entities = [FeedEntity::class, ArticleEntity::class, FilterRuleEntity::class], - version = 4, + version = 5, exportSchema = false ) abstract class AppDatabase : RoomDatabase() { @@ -77,12 +77,25 @@ abstract class AppDatabase : RoomDatabase() { } } + // v5: a feed with a literal empty <guid>/<id> element (confirmed live: a Mother + // Jones entry) produced an ArticleEntity with id = "" -- FeedParser fell back to + // itemLink only when the guid was null, not when it was blank. An empty-string + // primary key broke Navigation Compose's route matching and crashed on tap. The + // parser is fixed for future fetches; this cleans out any row already on-device + // with the bad id so it doesn't keep crashing. It'll come back with a real, + // hash-based id on the next fetch of that feed. + private val MIGRATION_4_5 = object : Migration(4, 5) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("DELETE FROM articles WHERE trim(id) = ''") + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: Room.databaseBuilder( context.applicationContext, AppDatabase::class.java, "feedreader.db" - ).addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4).build().also { instance = it } + ).addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5).build().also { instance = it } } } } diff --git a/app/src/main/java/org/terst/feedreader/data/FeedParser.kt b/app/src/main/java/org/terst/feedreader/data/FeedParser.kt index 629afe6..8b37e55 100644 --- a/app/src/main/java/org/terst/feedreader/data/FeedParser.kt +++ b/app/src/main/java/org/terst/feedreader/data/FeedParser.kt @@ -120,7 +120,12 @@ object FeedParser { inItem = false val content = itemContent ?: itemDescription items += ParsedItem( - guid = itemGuid ?: itemLink.ifBlank { null }, + // .ifBlank, not just ?: -- a feed with a literal empty <guid></guid> + // (confirmed live: a Mother Jones entry) gives itemGuid = "" here, + // which is non-null and so passed straight through a plain `?:`, + // landing as the article's primary key. An empty-string id broke + // Navigation Compose's route matching and crashed on tap. + guid = itemGuid?.ifBlank { null } ?: itemLink.ifBlank { null }, title = itemTitle, link = itemLink, author = itemAuthor, @@ -195,7 +200,8 @@ object FeedParser { inEntry = false val content = entryContent ?: entrySummary items += ParsedItem( - guid = entryId ?: entryLink.ifBlank { null }, + // Same blank-vs-null fix as the RSS path above. + guid = entryId?.ifBlank { null } ?: entryLink.ifBlank { null }, title = entryTitle, link = entryLink, author = entryAuthor, diff --git a/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt b/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt index 6202016..6eb5b71 100644 --- a/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/BrowserScreen.kt @@ -50,21 +50,21 @@ fun BrowserScreen(startUrl: String, onBack: () -> Unit) { var title by remember(startUrl) { mutableStateOf<String?>(null) } var webViewRef by remember(startUrl) { mutableStateOf<WebView?>(null) } - BackHandler { - val wv = webViewRef - if (wv != null && wv.canGoBack()) wv.goBack() else onBack() - } + // Deliberately NOT "goBack() if the WebView has history, else exit" -- that means + // however many pages deep a search/login session went (search results -> article -> + // another link -> ...), leaving requires tapping back that many times first. Reported + // live as not making sense for this kind of multi-page browsing. Back always just + // closes the screen now, both the system gesture and the in-app arrow; there's no + // separate in-page-back affordance, matching how every other screen in this app behaves. + BackHandler(onBack = onBack) Scaffold( topBar = { TopAppBar( title = { Text(title ?: "Browser", maxLines = 1, overflow = TextOverflow.Ellipsis) }, navigationIcon = { - IconButton(onClick = { - val wv = webViewRef - if (wv != null && wv.canGoBack()) wv.goBack() else onBack() - }) { - Icon(Icons.Default.ArrowBack, contentDescription = "Back") + IconButton(onClick = onBack) { + Icon(Icons.Default.ArrowBack, contentDescription = "Close browser") } }, actions = { 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 806926c..82cb792 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -105,20 +105,13 @@ fun ReaderScreen( val scope = rememberCoroutineScope() val swipeThresholdPx = with(LocalDensity.current) { 80.dp.toPx() } - // 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. - // detectHorizontalDragGestures has its own orientation-aware touch-slop detection, so - // it already declines to engage for vertical-dominant movement -- confirmed live: a - // diagonal mid-screen scroll stays vertical, doesn't page-turn. (A generic, - // non-oriented detectDragGestures was tried for a stricter dx/dy ratio check and broke - // scrolling entirely: its slop detector claims the first bit of movement in ANY - // direction isotropically, before it knows which way the gesture is really going -- - // reported live as "the article view doesn't scroll at all anymore.") Also: page-turn - // commit is based on total drag distance, not where the gesture started, since an - // edge-anchored version collided with Android's own edge-swipe-back gesture (confirmed - // live: an edge swipe triggered system back, not the pager). + // 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() |