From 104bd337243f5ed6056689c2a37d6fdc5ffbf846 Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 01:28:50 +0100 Subject: [PATCH 1/6] Keep recently read in step with items marked unread --- .../io/github/bardesss/wick/db/ItemDao.kt | 6 ++-- .../io/github/bardesss/wick/db/ItemDaoTest.kt | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt b/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt index f86a67b..7bf2451 100644 --- a/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt +++ b/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt @@ -47,10 +47,10 @@ abstract class ItemDao { @Query("SELECT id, readAtMs FROM ItemEntity WHERE readAtMs IS NOT NULL AND id IN (:ids)") protected abstract suspend fun readAtChunk(ids: List): List - /** Sets the read state of one item; marking it read also stamps it with [nowMs] for the recently read view. */ + /** Sets the read state of one item; marking it read stamps it with [nowMs] for the recently read view, marking it unread clears the stamp. */ @Query( "UPDATE ItemEntity SET read = :read, manualUnread = :manualUnread, " + - "readAtMs = CASE WHEN :read THEN :nowMs ELSE readAtMs END WHERE id = :id", + "readAtMs = CASE WHEN :read THEN :nowMs ELSE NULL END WHERE id = :id", ) abstract suspend fun setRead(id: String, read: Boolean, manualUnread: Boolean, nowMs: Long) @@ -99,7 +99,7 @@ abstract class ItemDao { "SELECT id FROM ItemEntity WHERE read = 1 " + "AND id NOT IN (SELECT itemId FROM PendingMarkEntity) " + "AND id NOT IN (SELECT itemId FROM SnapshotEntity) " + - "AND id NOT IN (SELECT id FROM ItemEntity WHERE readAtMs IS NOT NULL ORDER BY readAtMs DESC LIMIT :kept)", + "AND id NOT IN (SELECT id FROM ItemEntity WHERE read = 1 AND readAtMs IS NOT NULL ORDER BY readAtMs DESC, id ASC LIMIT :kept)", ) protected abstract suspend fun prunable(kept: Int): List diff --git a/app/src/test/java/io/github/bardesss/wick/db/ItemDaoTest.kt b/app/src/test/java/io/github/bardesss/wick/db/ItemDaoTest.kt index 7628321..1800c51 100644 --- a/app/src/test/java/io/github/bardesss/wick/db/ItemDaoTest.kt +++ b/app/src/test/java/io/github/bardesss/wick/db/ItemDaoTest.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -74,6 +75,37 @@ class ItemDaoTest { assertEquals(listOf("b"), items.recentlyRead(10)) } + @Test + fun marking_unread_clears_the_read_time() = runTest { + items.upsertKeepingLocal(listOf(item("a"))) + items.setRead("a", read = true, manualUnread = false, nowMs = 10) + items.setRead("a", read = false, manualUnread = true, nowMs = 20) + + assertNull(row("a").readAtMs) + } + + @Test + fun unread_then_mark_all_stays_out_of_recently_read() = runTest { + items.upsertKeepingLocal(listOf(item("x"))) + items.setRead("x", read = true, manualUnread = false, nowMs = 10) + items.setRead("x", read = false, manualUnread = true, nowMs = 20) + items.markReadWhere(null, publishedAtOrBeforeSec = 1000) + + assertFalse("x" in items.recentlyRead(10)) + } + + @Test + fun prune_keeps_the_configured_number_of_read_rows() = runTest { + val ids = (1..RECENT_KEPT + 5).map { "r$it" } + items.upsertKeepingLocal(ids.map { item(it) }) + ids.forEachIndexed { i, id -> items.setRead(id, read = true, manualUnread = false, nowMs = i + 1L) } + ids.takeLast(5).forEach { items.setRead(it, read = false, manualUnread = true, nowMs = 1000) } + + items.pruneReadNotIn(emptySet()) + + assertEquals(RECENT_KEPT, items.recentlyRead(RECENT_KEPT).size) + } + @Test fun upsert_keeps_when_an_item_was_read_here() = runTest { items.upsertKeepingLocal(listOf(item("a"))) From 7192466ab5d133984df12310441797e2a109d122 Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 01:56:18 +0100 Subject: [PATCH 2/6] Write the list only for the view that is showing --- .../bardesss/wick/ui/list/ListViewModel.kt | 54 +++++++----- .../wick/ui/list/ListViewModelTest.kt | 84 ++++++++++++++++++- 2 files changed, 116 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt index c6487e0..b3a3b8b 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt @@ -56,6 +56,8 @@ import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext /** What a row shows apart from its read state, which changes on its own (see [ListViewModel.read]). */ @@ -103,6 +105,8 @@ class ListViewModel( private var busy = false private var again = false private var applying = false + // one snapshot write at a time, so the check of the view and the write go together + private val snapshotLock = Mutex() private var backgroundedAt: Long? = null /** @@ -238,7 +242,8 @@ class ListViewModel( /** * Marks the whole view read on the server. The cutoff is the newest of the rows loaded and the * unread items stored for the view, so it covers what the header counts, whether or not the - * list has caught up. With neither, it is now. Items that arrive later stay unread. + * list has caught up. With neither, it is now. Items that arrive later stay unread. The rows + * count only when the stored list belongs to this view; right after a switch it may not yet. */ fun markAllRead() { val v = _view.value @@ -247,7 +252,8 @@ class ListViewModel( if (loaded.isNullOrEmpty() && count.value <= 0) return viewModelScope.launch { val feedIds = v.feedIds(feedDao.feedsNow()) - val newest = listOfNotNull(loaded?.maxOfOrNull { it.publishedSec }, itemDao.newestUnreadSec(feedIds)).maxOrNull() + val ours = if (snapshotDao.viewRaw() == v.raw) loaded else null + val newest = listOfNotNull(ours?.maxOfOrNull { it.publishedSec }, itemDao.newestUnreadSec(feedIds)).maxOrNull() ?: (clock() / 1000) queue.markAll(v, feedIds, newest) // the refresh must see the mark-all in the queue @@ -356,10 +362,10 @@ class ListViewModel( } result.fold( onSuccess = { r -> - if (v == _view.value) { - replaceSnapshot(r.ids, clock(), v) + if (replaceSnapshot(r.ids, clock(), v)) { _capped.value = r.capped - } else { + } else if (v != _view.value) { + // only a new view is worth another round; a failed write alone would loop again = true } _message.value = if (r.marksStuck) Note(R.string.note_marks_unsent) else null @@ -389,7 +395,7 @@ class ListViewModel( val synced = sync.backgroundSync() val ids = storedIds(StreamId.Read) return synced.map { Refreshed(ids, capped = false, marksStuck = false) }.onFailure { - if (_view.value == StreamId.Read) replaceSnapshot(ids, clock(), StreamId.Read) + replaceSnapshot(ids, clock(), StreamId.Read) } } @@ -401,21 +407,29 @@ class ListViewModel( itemDao.unreadIds(view.feedIds(feedDao.feedsNow()), settings.oldestFirst.first()) } - /** Swaps the snapshot with scroll marking paused. On failure the old list stays. */ - private suspend fun replaceSnapshot(ids: List, writtenAtMs: Long, view: StreamId) { - applying = true - try { - snapshotDao.replaceSnapshot(ids, writtenAtMs, view.raw) - tracker.reset() - lastFrameIndex = HashMap() - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - Log.w(LOG_TAG, "could not store the new list", e) - } finally { - applying = false + /** + * Swaps the snapshot with scroll marking paused, but only while [view] is still the one + * showing. Returns whether it wrote; on failure the old list stays. + */ + private suspend fun replaceSnapshot(ids: List, writtenAtMs: Long, view: StreamId): Boolean = + snapshotLock.withLock { + // the user moved on while the list was being built + if (_view.value != view) return@withLock false + applying = true + try { + snapshotDao.replaceSnapshot(ids, writtenAtMs, view.raw) + tracker.reset() + lastFrameIndex = HashMap() + true + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Log.w(LOG_TAG, "could not store the new list", e) + false + } finally { + applying = false + } } - } private fun idsWhere(list: List, test: (LocalState) -> Boolean): Set = list.filter(test).mapTo(HashSet()) { it.id } diff --git a/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt b/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt index f09df5d..8e7eab0 100644 --- a/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt +++ b/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt @@ -14,7 +14,12 @@ import io.github.bardesss.wick.data.SettingsStore import io.github.bardesss.wick.db.CountEntity import io.github.bardesss.wick.db.FeedEntity import io.github.bardesss.wick.db.ItemEntity +import io.github.bardesss.wick.db.LocalState import io.github.bardesss.wick.db.PendingMarkAllEntity +import io.github.bardesss.wick.db.SnapshotDao +import io.github.bardesss.wick.db.SnapshotEntity +import io.github.bardesss.wick.db.SnapshotMetaEntity +import io.github.bardesss.wick.db.SnapshotRow import io.github.bardesss.wick.db.WickDb import io.github.bardesss.wick.sync.FakeReaderApi import io.github.bardesss.wick.sync.MarkQueue @@ -27,6 +32,7 @@ import io.github.bardesss.wick.ui.picker.PickerEntry import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.first @@ -91,7 +97,7 @@ class ListViewModelTest { sync = SyncEngine(api, db, queue) } - private fun TestScope.vm(handle: SavedStateHandle = SavedStateHandle()): ListViewModel { + private fun TestScope.vm(handle: SavedStateHandle = SavedStateHandle(), snapshots: SnapshotDao = db.snapshotDao()): ListViewModel { val store = ViewModelStore().also { stores += it } val factory = object : ViewModelProvider.Factory { @Suppress("UNCHECKED_CAST") @@ -99,7 +105,7 @@ class ListViewModelTest { savedState = handle, itemDao = db.itemDao(), feedDao = db.feedDao(), - snapshotDao = db.snapshotDao(), + snapshotDao = snapshots, sync = sync, queue = queue, settings = settings, @@ -693,4 +699,78 @@ class ListViewModelTest { assertEquals(StreamId.All, settle(vm.view) { it == StreamId.All }) assertEquals(listOf("b", "a"), ids(vm, listOf("b", "a"))) } + + /** The real snapshot store, except that the write after [holdNext] waits for [gate]. */ + private class HeldSnapshots(private val real: SnapshotDao) : SnapshotDao() { + var holdNext = false + val gate = CompletableDeferred() + + override suspend fun clear() = error("unused") + override suspend fun insert(rows: List) = error("unused") + override suspend fun setMeta(meta: SnapshotMetaEntity) = error("unused") + override fun snapshotIds(): Flow> = real.snapshotIds() + override suspend fun writtenAt(): Long? = real.writtenAt() + override suspend fun viewRaw(): String? = real.viewRaw() + override suspend fun rowsNow(): List = real.rowsNow() + override fun states(): Flow> = real.states() + + override suspend fun replaceSnapshot(ids: List, writtenAtMs: Long, viewRaw: String) { + if (holdNext) { + holdNext = false + gate.await() + } + real.replaceSnapshot(ids, writtenAtMs, viewRaw) + } + } + + @Test + fun quick_view_switches_leave_the_last_views_list() = runTest(main, timeout = 30.seconds) { + engine() + seedServer() + api.addItem("c", "feed/2", 300) + val snapshots = HeldSnapshots(db.snapshotDao()) + val vm = vm(snapshots = snapshots) + ids(vm, listOf("c", "b", "a")) + settle(vm.refreshing) { !it } + val server = CompletableDeferred() + api.beforeUnread = { fetches++; server.await() } + + // the list for feed 1 is still being written when feed 2 is picked + snapshots.holdNext = true + vm.select(StreamId.Feed("feed/1")) + runCurrent() + vm.select(StreamId.Feed("feed/2")) + runCurrent() + snapshots.gate.complete(Unit) + + assertEquals(listOf("c"), ids(vm, listOf("c"))) + runCurrent() + assertEquals(StreamId.Feed("feed/2").raw, db.snapshotDao().viewRaw()) + assertEquals(listOf("c"), vm.rows.value?.map { it.id }) + server.complete(Unit) + } + + @Test + fun mark_all_ignores_rows_of_another_view() = runTest(main, timeout = 30.seconds) { + engine() + db.feedDao().replaceAll(listOf(FeedEntity("feed/1", "News", null, null), FeedEntity("feed/2", "Other", null, null))) + db.itemDao().upsertKeepingLocal( + listOf( + ItemEntity("a", "feed/1", "T a", null, 100, null, read = false), + ItemEntity("b", "feed/1", "T b", null, 200, null, read = false), + ItemEntity("c", "feed/2", "T c", null, 300, null, read = false), + ), + ) + // the stored list belongs to all unread and holds a newer item of feed 2 + db.snapshotDao().replaceSnapshot(listOf("c", "b", "a"), writtenAtMs = now, viewRaw = StreamId.All.raw) + val handle = SavedStateHandle().apply { set("view", StreamId.Feed("feed/1").raw) } + val vm = vm(handle) + ids(vm, listOf("c", "b", "a")) + assertEquals(StreamId.Feed("feed/1"), vm.view.value) + + vm.markAllRead() + awaitMarkAll() + + assertEquals(listOf(StreamId.Feed("feed/1").raw to (200 + 1) * 1_000_000L), api.markAllCalls) + } } From 3dec6dacffbbeeac5a373302a344581a0826f84f Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 02:16:29 +0100 Subject: [PATCH 3/6] Decide a toggle from the stored state --- .../io/github/bardesss/wick/db/ItemDao.kt | 4 ++++ .../io/github/bardesss/wick/sync/MarkQueue.kt | 14 ++++++++++++-- .../bardesss/wick/ui/list/ListViewModel.kt | 2 +- .../bardesss/wick/sync/MarkQueueTest.kt | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt b/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt index 7bf2451..e399296 100644 --- a/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt +++ b/app/src/main/java/io/github/bardesss/wick/db/ItemDao.kt @@ -118,6 +118,10 @@ abstract class ItemDao { @Query("SELECT EXISTS(SELECT 1 FROM ItemEntity)") abstract suspend fun any(): Boolean + /** Whether the item is read locally, or null when it is not stored. */ + @Query("SELECT read FROM ItemEntity WHERE id = :id") + abstract suspend fun isRead(id: String): Boolean? + @Query("SELECT html FROM ItemEntity WHERE id = :id") abstract suspend fun htmlOf(id: String): String? diff --git a/app/src/main/java/io/github/bardesss/wick/sync/MarkQueue.kt b/app/src/main/java/io/github/bardesss/wick/sync/MarkQueue.kt index 1bab2d0..1641a05 100644 --- a/app/src/main/java/io/github/bardesss/wick/sync/MarkQueue.kt +++ b/app/src/main/java/io/github/bardesss/wick/sync/MarkQueue.kt @@ -74,16 +74,26 @@ class MarkQueue( } fun mark(id: String, read: Boolean, manual: Boolean) { + ops.trySend { stateLock.withLock { inTransaction { applyMark(id, read, manual) } } } + } + + /** Flips the stored read state, decided when the op runs so quick taps cancel out. */ + fun toggle(id: String) { ops.trySend { stateLock.withLock { inTransaction { - itemDao.setRead(id, read, manualUnread = manual && !read, nowMs = clock()) - queueDao.enqueueOrCancel(PendingMarkEntity(id, read, nextStamp()), inFlight = id in inFlight) + val wasRead = itemDao.isRead(id) + if (wasRead != null) applyMark(id, read = !wasRead, manual = wasRead) } } } } + private suspend fun applyMark(id: String, read: Boolean, manual: Boolean) { + itemDao.setRead(id, read, manualUnread = manual && !read, nowMs = clock()) + queueDao.enqueueOrCancel(PendingMarkEntity(id, read, nextStamp()), inFlight = id in inFlight) + } + // The server marks only items strictly older than the cutoff, in whole seconds, so ask for // the start of the next second to include items published at newestLoadedSec. fun markAll(stream: StreamId, feedIds: List?, newestLoadedSec: Long) { diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt index b3a3b8b..83cdd5a 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt @@ -304,7 +304,7 @@ class ListViewModel( } fun toggleRead(id: String) { - if (id in read.value) queue.mark(id, read = false, manual = true) else queue.mark(id, read = true, manual = false) + queue.toggle(id) } /** Whether ticks are on; the screen follows it in its [io.github.bardesss.wick.ui.Haptics]. */ diff --git a/app/src/test/java/io/github/bardesss/wick/sync/MarkQueueTest.kt b/app/src/test/java/io/github/bardesss/wick/sync/MarkQueueTest.kt index abf1fae..44e6496 100644 --- a/app/src/test/java/io/github/bardesss/wick/sync/MarkQueueTest.kt +++ b/app/src/test/java/io/github/bardesss/wick/sync/MarkQueueTest.kt @@ -49,6 +49,25 @@ class MarkQueueTest { private fun row(id: String) = db.storedItems(listOf(id)).single() + @Test + fun two_quick_toggles_cancel_out() = runTest { + seed("a") + val q = queue() + q.toggle("a") + q.toggle("a") + runCurrent() + assertFalse(row("a").read) + assertTrue(db.queueDao().pending().isEmpty()) + } + + @Test + fun toggle_ignores_an_unknown_id() = runTest { + val q = queue() + q.toggle("nope") + runCurrent() + assertTrue(db.queueDao().pending().isEmpty()) + } + @Test fun unread_after_mark_all_keeps_item_unread_on_server() = runTest { seed("a", "b") From 611fc8b32d3646c13d484407c20638ff2342bd54 Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 02:43:55 +0100 Subject: [PATCH 4/6] Keep the open sheet and search position across rotation --- .../bardesss/wick/ui/list/ListScreen.kt | 22 +- .../bardesss/wick/ui/list/ListViewModel.kt | 3 + .../wick/ui/list/ListScreenStateTest.kt | 214 ++++++++++++++++++ 3 files changed, 234 insertions(+), 5 deletions(-) create mode 100644 app/src/test/java/io/github/bardesss/wick/ui/list/ListScreenStateTest.kt diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt index 9c5b567..736957a 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt @@ -211,7 +211,6 @@ fun ListScreen(vm: ListViewModel, onOpenSettings: () -> Unit) { val markedRead = stringResource(R.string.list_marked_read) val markedUnread = stringResource(R.string.list_marked_unread) var sheetId by rememberSaveable { mutableStateOf(null) } - val sheetKept = remember { arrayOfNulls(1) } val linkTarget by vm.linkTarget.collectAsStateWithLifecycle(LinkTarget.InApp) val open = remember(vm, context, linkTarget) { @@ -290,8 +289,13 @@ fun ListScreen(vm: ListViewModel, onOpenSettings: () -> Unit) { } // a refresh may drop the row; the open sheet keeps showing it until the user closes it - val sheetRow = sheetId?.let { id -> rows?.firstOrNull { it.id == id } ?: sheetKept[0]?.takeIf { it.id == id } } - sheetKept[0] = sheetRow + // the kept row lives in the view model so it survives rotation too + val sheetRow = sheetId?.let { id -> rows?.firstOrNull { it.id == id } ?: vm.sheetKept?.takeIf { it.id == id } } + vm.sheetKept = sheetRow + // an id with no row anywhere (process death) must not reopen the sheet by itself later + LaunchedEffect(sheetId, rows) { + if (sheetId != null && rows != null && sheetRow == null) sheetId = null + } if (sheetRow != null) { val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) val body by produceState(SheetBody.Loading, sheetRow.id) { @@ -362,8 +366,16 @@ fun ListContent( var query by rememberSaveable(history) { mutableStateOf(searchPreview?.takeIf { history }) } BackHandler(enabled = query != null) { query = null } val found = remember(rows, query) { query?.let { q -> rows?.let { searchRows(it, q) } } ?: rows } - // each new search starts at the top of what it found - LaunchedEffect(query) { if (query != null) listState.scrollToItem(0) } + // each new search starts at the top of what it found; a restored one keeps its position + var scrolledFor by rememberSaveable { mutableStateOf(null) } + LaunchedEffect(query) { + if (query == null) { + scrolledFor = null + } else if (query != scrolledFor) { + listState.scrollToItem(0) + scrolledFor = query + } + } val frameSink by rememberUpdatedState(onFrame) val shownRows by rememberUpdatedState(found) LaunchedEffect(listState) { diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt index 83cdd5a..a2ff086 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt @@ -127,6 +127,9 @@ class ListViewModel( .flowOn(work) .shareIn(viewModelScope, SharingStarted.Eagerly, replay = 1) + /** The row the open sheet shows, so it outlives a refresh that drops it and a rotation. */ + var sheetKept: RowModel? = null + /** Ids of the snapshot rows that are read. */ val read: StateFlow> = marks.map { it.read } .distinctUntilChanged() diff --git a/app/src/test/java/io/github/bardesss/wick/ui/list/ListScreenStateTest.kt b/app/src/test/java/io/github/bardesss/wick/ui/list/ListScreenStateTest.kt new file mode 100644 index 0000000..c65ffd7 --- /dev/null +++ b/app/src/test/java/io/github/bardesss/wick/ui/list/ListScreenStateTest.kt @@ -0,0 +1,214 @@ +package io.github.bardesss.wick.ui.list + +import androidx.activity.ComponentActivity +import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.ui.semantics.SemanticsActions +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.semantics.getOrNull +import androidx.compose.ui.test.junit4.StateRestorationTester +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollToIndex +import androidx.compose.ui.test.performTextInput +import androidx.lifecycle.SavedStateHandle +import io.github.bardesss.wick.api.Auth +import io.github.bardesss.wick.data.CredentialStore +import io.github.bardesss.wick.data.MemoryPreferences +import io.github.bardesss.wick.data.SettingsStore +import io.github.bardesss.wick.db.ItemEntity +import io.github.bardesss.wick.db.WickDb +import io.github.bardesss.wick.sync.FakeReaderApi +import io.github.bardesss.wick.sync.SyncEngine +import io.github.bardesss.wick.sync.inMemoryDb +import io.github.bardesss.wick.sync.newQueue +import io.github.bardesss.wick.ui.theme.WickTheme +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * The screen's sheet and search state across recreation. One view model instance is kept + * across the emulated recreation, which is what survives a rotation in the app. + */ +@RunWith(RobolectricTestRunner::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +@Config(qualifiers = "w411dp-h891dp-xxhdpi") +class ListScreenStateTest { + @get:Rule + val rule = createAndroidComposeRule() + + private class MemoryCreds : CredentialStore { + private val flow = MutableStateFlow(Auth("https://feeds.example.com", "tok")) + override fun auth() = flow.value + override val authFlow: StateFlow = flow + override suspend fun save(auth: Auth) { flow.value = auth } + override suspend fun clear() { flow.value = null } + } + + private lateinit var db: WickDb + private lateinit var scope: CoroutineScope + private lateinit var vm: ListViewModel + private val api = FakeReaderApi() + + @Before + fun setUp() { + db = inMemoryDb() + scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + // the first refresh never finishes, so the stored list stays as the test left it + val gate = CompletableDeferred() + api.beforeUnread = { gate.await() } + val queue = db.newQueue(api, scope) + vm = ListViewModel( + savedState = SavedStateHandle(), + itemDao = db.itemDao(), + feedDao = db.feedDao(), + snapshotDao = db.snapshotDao(), + sync = SyncEngine(api, db, queue), + queue = queue, + settings = SettingsStore(MemoryPreferences()), + creds = MemoryCreds(), + appScope = scope, + work = Dispatchers.Default, + ) + } + + @After + fun tearDown() { + scope.cancel() + db.close() + } + + private fun store(ids: List) = runBlocking { + db.itemDao().upsertKeepingLocal( + ids.mapIndexed { i, id -> ItemEntity(id, "feed/1", "Title $id", null, 100L + i, null, read = false) }, + ) + } + + private fun snapshot(ids: List) = runBlocking { + db.snapshotDao().replaceSnapshot(ids, writtenAtMs = System.currentTimeMillis()) + } + + private fun rowCount() = rule.onAllNodesWithTag("row").fetchSemanticsNodes().size + + private fun showSheetOf(index: Int) { + rule.waitUntil(5_000) { rowCount() > index } + val action = rule.onAllNodesWithTag("row")[index].fetchSemanticsNode().config + .getOrNull(SemanticsActions.CustomActions)!!.first() + rule.runOnUiThread { action.action() } + rule.waitUntil(5_000) { sheetShowing() } + } + + private fun sheetShowing() = rule.onAllNodesWithTag("sheet").fetchSemanticsNodes().isNotEmpty() + + @Test + fun sheet_for_a_dropped_row_survives_recreation() { + store(listOf("a", "b")) + snapshot(listOf("b", "a")) + val tester = StateRestorationTester(rule) + tester.setContent { WickTheme { ListScreen(vm, onOpenSettings = {}) } } + showSheetOf(0) + + // a refresh drops the row the sheet shows + snapshot(listOf("a")) + rule.waitUntil(5_000) { rowCount() == 1 } + assertTrue(sheetShowing()) + + tester.emulateSavedInstanceStateRestore() + rule.waitForIdle() + + assertTrue(sheetShowing()) + } + + @Test + fun sheet_id_without_a_row_is_cleared() { + store(listOf("a", "b")) + snapshot(listOf("b", "a")) + val tester = StateRestorationTester(rule) + tester.setContent { WickTheme { ListScreen(vm, onOpenSettings = {}) } } + showSheetOf(0) + + // the process was rebuilt: nothing is kept and the row is gone from the list + vm.sheetKept = null + snapshot(listOf("a")) + tester.emulateSavedInstanceStateRestore() + rule.waitUntil(5_000) { !sheetShowing() } + + // and it does not come back by itself if the row returns later + snapshot(listOf("b", "a")) + rule.waitUntil(5_000) { rowCount() == 2 } + assertFalse(sheetShowing()) + } + + @Test + fun a_row_still_listed_keeps_its_sheet_and_the_kept_row() { + store(listOf("a", "b")) + snapshot(listOf("b", "a")) + rule.setContent { WickTheme { ListScreen(vm, onOpenSettings = {}) } } + showSheetOf(0) + rule.waitForIdle() + + assertTrue(sheetShowing()) + assertEquals("b", vm.sheetKept?.id) + } + + @Test + fun restored_search_keeps_its_scroll_position() { + val rows = (1..40).map { RowModel("n$it", "Title $it", null, "News", null, 100L + it) } + val tester = StateRestorationTester(rule) + tester.setContent { + WickTheme { + ListContent( + viewName = "Recently read", + history = true, + count = 0, + message = null, + rows = rows, + read = remember { mutableStateOf(rows.map { it.id }.toSet()) }, + nowSec = 0, + refreshing = false, + listState = rememberLazyListState(), + onRefresh = {}, + onOpenPicker = {}, + onOpen = {}, + onFrame = { false }, + ) + } + } + + rule.onNodeWithTag("search").performClick() + rule.onNodeWithTag("search_field").performTextInput("Title") + rule.waitForIdle() + rule.onNodeWithTag("list").performScrollToIndex(20) + rule.waitForIdle() + val before = scrollValue() + assertTrue(before > 0f) + + tester.emulateSavedInstanceStateRestore() + rule.waitForIdle() + + assertEquals(before, scrollValue(), 0.5f) + } + + private fun scrollValue(): Float = rule.onNodeWithTag("list").fetchSemanticsNode().config + .getOrNull(SemanticsProperties.VerticalScrollAxisRange)!!.value() +} From c83cf048b9d8aba2d4d793c9d12186c35281ab5f Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 02:57:25 +0100 Subject: [PATCH 5/6] Mark rows read past a row kept above the screen --- .../bardesss/wick/ui/list/ListViewModel.kt | 3 ++- .../wick/ui/list/ListViewModelTest.kt | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt index a2ff086..a200a9c 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListViewModel.kt @@ -286,7 +286,8 @@ class ListViewModel( var firstIndex = Int.MAX_VALUE for (item in frame.items) { current[item.id] = item.index - if (item.index < firstIndex) firstIndex = item.index + // a row kept composed above the viewport must not hold the first index back + if (item.offset + item.size > frame.viewportStart && item.index < firstIndex) firstIndex = item.index if (item.id in readNow || item.size <= 0) continue val top = maxOf(item.offset, frame.viewportStart) val bottom = minOf(item.offset + item.size, frame.viewportEnd) diff --git a/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt b/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt index 8e7eab0..2afee83 100644 --- a/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt +++ b/app/src/test/java/io/github/bardesss/wick/ui/list/ListViewModelTest.kt @@ -342,6 +342,33 @@ class ListViewModelTest { assertEquals(setOf("b"), settle(vm.read) { it == setOf("b") }) } + @Test + fun row_dropped_below_a_pinned_row_is_marked_read() = runTest(main, timeout = 30.seconds) { + engine() + seedServer() + api.addItem("p", "feed/1", 50) + api.addItem("c", "feed/1", 300) + api.addItem("d", "feed/1", 400) + val vm = vm() + ids(vm, listOf("d", "c", "b", "a", "p")) + settle(vm.refreshing) { !it } + + fun at(t: Long, vararg rows: Triple) = ListFrame( + nowMs = t, + viewportStart = 0, + viewportEnd = 800, + items = rows.map { (id, index, offset) -> FrameItem(id, index, offset, 100) }, + shown = null, + ) + // p is kept composed above the viewport; b is the first row on screen + for (t in 0L..400L step 16) { + vm.onVisible(at(t, Triple("p", 0, -150), Triple("b", 5, 0), Triple("a", 6, 100), Triple("c", 7, 200))) + } + vm.onVisible(at(420, Triple("p", 0, -150), Triple("a", 6, 0), Triple("c", 7, 100), Triple("d", 8, 200))) + + assertEquals(setOf("b"), settle(vm.read) { it == setOf("b") }) + } + @Test fun recently_read_lists_what_was_read_here_and_scrolling_marks_nothing() = runTest(main, timeout = 30.seconds) { engine() From 553bf5bba6502b3fd134b0b8c59668bb5521468a Mon Sep 17 00:00:00 2001 From: Bardesss <30936735+bardesss@users.noreply.github.com> Date: Fri, 13 Nov 2026 03:22:30 +0100 Subject: [PATCH 6/6] Mark a row read only when its link opened --- .../java/io/github/bardesss/wick/ui/Links.kt | 19 +++++++----- .../bardesss/wick/ui/list/ListScreen.kt | 12 ++++++-- .../bardesss/wick/ui/sheet/SummarySheet.kt | 7 +++-- app/src/main/res/values/strings.xml | 1 + .../io/github/bardesss/wick/ui/LinksTest.kt | 30 +++++++++++++++++++ 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/io/github/bardesss/wick/ui/Links.kt b/app/src/main/java/io/github/bardesss/wick/ui/Links.kt index f492f22..aa93ebb 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/Links.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/Links.kt @@ -11,13 +11,15 @@ import io.github.bardesss.wick.data.LinkTarget import io.github.bardesss.wick.ui.theme.WickColors /** True for http and https links, the only ones a feed may open. */ -fun isWebUrl(url: String): Boolean = - url.startsWith("http://", ignoreCase = true) || url.startsWith("https://", ignoreCase = true) +fun isWebUrl(url: String): Boolean { + val trimmed = url.trim() + return trimmed.startsWith("http://", ignoreCase = true) || trimmed.startsWith("https://", ignoreCase = true) +} /** The intent that opens [url] for [target], or null for anything but http and https. */ fun linkIntent(url: String, target: LinkTarget): Intent? { if (!isWebUrl(url)) return null - val uri = url.toUri() + val uri = url.trim().toUri().normalizeScheme() return when (target) { LinkTarget.DefaultApp -> Intent(Intent.ACTION_VIEW, uri) LinkTarget.InApp -> { @@ -38,12 +40,13 @@ fun linkIntent(url: String, target: LinkTarget): Intent? { /** * Opens [url] the way [target] says. A Custom Tab falls back to a plain view intent when no - * browser takes it. Other schemes are ignored. + * browser takes it. Other schemes are ignored. Returns whether an activity started. */ -fun openLink(context: Context, url: String, target: LinkTarget) { - val intent = linkIntent(url, target) ?: return - if (start(context, intent) || target != LinkTarget.InApp) return - start(context, Intent(Intent.ACTION_VIEW, url.toUri())) +fun openLink(context: Context, url: String, target: LinkTarget): Boolean { + val intent = linkIntent(url, target) ?: return false + if (start(context, intent)) return true + if (target != LinkTarget.InApp) return false + return start(context, Intent(Intent.ACTION_VIEW, url.trim().toUri().normalizeScheme())) } // false when nothing on the device can open it diff --git a/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt b/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt index 736957a..6565e4f 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/list/ListScreen.kt @@ -213,11 +213,17 @@ fun ListScreen(vm: ListViewModel, onOpenSettings: () -> Unit) { var sheetId by rememberSaveable { mutableStateOf(null) } val linkTarget by vm.linkTarget.collectAsStateWithLifecycle(LinkTarget.InApp) - val open = remember(vm, context, linkTarget) { + val noBrowser = stringResource(R.string.list_no_browser) + val open = remember(vm, context, linkTarget, noBrowser) { { row: RowModel -> row.url?.takeIf(::isWebUrl)?.let { url -> - openLink(context, url, linkTarget) - vm.open(row.id) + // a row is read only once its link really opened + if (openLink(context, url, linkTarget)) { + vm.open(row.id) + } else { + toast[0]?.cancel() + toast[0] = scope.launch { snackbar.showSnackbar(noBrowser) } + } } Unit } diff --git a/app/src/main/java/io/github/bardesss/wick/ui/sheet/SummarySheet.kt b/app/src/main/java/io/github/bardesss/wick/ui/sheet/SummarySheet.kt index 4075352..1d6b705 100644 --- a/app/src/main/java/io/github/bardesss/wick/ui/sheet/SummarySheet.kt +++ b/app/src/main/java/io/github/bardesss/wick/ui/sheet/SummarySheet.kt @@ -61,6 +61,7 @@ import io.github.bardesss.wick.ui.benchTags import io.github.bardesss.wick.ui.short import io.github.bardesss.wick.ui.spoken import io.github.bardesss.wick.ui.list.RowModel +import io.github.bardesss.wick.ui.isWebUrl import io.github.bardesss.wick.ui.openLink import io.github.bardesss.wick.ui.theme.InstrumentSans import io.github.bardesss.wick.ui.theme.Newsreader @@ -163,7 +164,9 @@ private fun SheetContent(row: RowModel, body: SheetBody, nowSec: Long, linkTarge // links in the text open the same way as the list does val links = remember(context, linkTarget) { object : UriHandler { - override fun openUri(uri: String) = openLink(context, uri, linkTarget) + override fun openUri(uri: String) { + openLink(context, uri, linkTarget) + } } } // the cap sits on the content; a height limit on the sheet itself throws off its anchors @@ -211,7 +214,7 @@ private fun SheetContent(row: RowModel, body: SheetBody, nowSec: Long, linkTarge } } } - if (row.url != null) Button( + if (row.url?.let(::isWebUrl) == true) Button( onClick = onOpen, shape = RoundedCornerShape(14.dp), colors = ButtonDefaults.buttonColors(containerColor = WickColors.Amber, contentColor = WickColors.Night), diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 659c57a..1bc0294 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -24,6 +24,7 @@ Marked read Marked unread + No app can open this link %1$s ago now %1$dm diff --git a/app/src/test/java/io/github/bardesss/wick/ui/LinksTest.kt b/app/src/test/java/io/github/bardesss/wick/ui/LinksTest.kt index 7fa3bfd..2d1f49f 100644 --- a/app/src/test/java/io/github/bardesss/wick/ui/LinksTest.kt +++ b/app/src/test/java/io/github/bardesss/wick/ui/LinksTest.kt @@ -2,7 +2,11 @@ package io.github.bardesss.wick.ui import android.content.Intent import androidx.browser.customtabs.CustomTabsIntent +import androidx.test.core.app.ApplicationProvider import io.github.bardesss.wick.data.LinkTarget +import org.robolectric.Shadows.shadowOf +import android.app.Activity +import org.robolectric.Robolectric import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -48,4 +52,30 @@ class LinksTest { assertFalse(isWebUrl("javascript:alert(1)")) assertFalse(isWebUrl("")) } + + @Test + fun upper_case_scheme_is_normalised() { + val intent = linkIntent("HTTPS://a.example/x", LinkTarget.DefaultApp)!! + assertEquals("https", intent.data!!.scheme) + } + + @Test + fun leading_space_is_trimmed() { + assertTrue(isWebUrl(" https://a.example")) + assertEquals("https://a.example", linkIntent(" https://a.example ", LinkTarget.DefaultApp)!!.dataString) + } + + @Test + fun open_link_reports_when_nothing_opens_it() { + shadowOf(ApplicationProvider.getApplicationContext()).checkActivities(true) + val activity = Robolectric.buildActivity(Activity::class.java).get() + for (target in LinkTarget.entries) assertFalse(openLink(activity, "https://a.example/x", target)) + } + + @Test + fun open_link_reports_true_when_an_activity_starts() { + val activity = Robolectric.buildActivity(Activity::class.java).get() + assertTrue(openLink(activity, "https://a.example/x", LinkTarget.DefaultApp)) + assertFalse(openLink(activity, "market://details?id=x", LinkTarget.DefaultApp)) + } }