From 11232e9169dd1f125b9ded6ac32354696a3b45ec Mon Sep 17 00:00:00 2001 From: darken Date: Thu, 6 Aug 2026 09:22:01 +0200 Subject: [PATCH] fix(overview): Reset the review card latch when it leaves the list The card is a keyed item in the overview's lazy list, which restores the saveable state of removed items when they come back. A review tap left the dismiss action permanently disabled once a higher priority card took the slot and gave it back, and a dismissed card returned fully dead. Fixes review finding F1. --- .../main/ui/overview/cards/ReviewCard.kt | 11 ++++-- .../main/ui/overview/cards/ReviewCardTest.kt | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/eu/darken/capod/main/ui/overview/cards/ReviewCard.kt b/app/src/main/java/eu/darken/capod/main/ui/overview/cards/ReviewCard.kt index b1a412f5..9e40a5a9 100644 --- a/app/src/main/java/eu/darken/capod/main/ui/overview/cards/ReviewCard.kt +++ b/app/src/main/java/eu/darken/capod/main/ui/overview/cards/ReviewCard.kt @@ -19,7 +19,7 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -43,8 +43,13 @@ fun ReviewCard( // the review bookkeeping with a snooze) and a review after a dismiss. A repeated review tap is // harmless, the tool's single-flight lock absorbs it, and blocking it here would leave a dead // card whenever a Play request fails and nothing gets persisted. - var dismissLocked by rememberSaveable { mutableStateOf(false) } - var fullyLatched by rememberSaveable { mutableStateOf(false) } + // + // Plain remember, not rememberSaveable: the card is a keyed item in a lazy list, which hands + // saveable state back when the item returns, so a card that was removed by a higher priority + // card would come back still latched. Disposal is the intended reset, the latch only has to + // survive the sub-second window until the next state emission takes the card away. + var dismissLocked by remember { mutableStateOf(false) } + var fullyLatched by remember { mutableStateOf(false) } Card( modifier = Modifier .fillMaxWidth() diff --git a/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt b/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt index 0e1607ea..6f25d2ba 100644 --- a/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt +++ b/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt @@ -1,6 +1,8 @@ package eu.darken.capod.main.ui.overview.cards import android.content.Context +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.semantics.SemanticsActions import androidx.compose.ui.test.assertIsEnabled import androidx.compose.ui.test.assertIsNotEnabled @@ -126,6 +128,41 @@ class ReviewCardTest : BaseComposeRobolectricTest() { } } + @Test + fun `a card that left the list comes back unlatched`() { + val visible = mutableStateOf(true) + composeRule.setContent { + PreviewWrapper { + // Mirrors OverviewScreen: the card is a keyed item in a lazy list, which keeps the + // saveable state of removed items around and hands it back when they return. + LazyColumn { + if (visible.value) { + item(key = "review") { + ReviewCard( + onReview = { reviews++ }, + onDismiss = { dismisses++ }, + ) + } + } + } + } + } + + composeRule.onNode(reviewButton).performSemanticsAction(SemanticsActions.OnClick) + composeRule.runOnIdle { reviews shouldBe 1 } + composeRule.onNodeWithText(dismissLabel).assertIsNotEnabled() + + // A higher priority card takes the slot and gives it back, e.g. Bluetooth flipping off + composeRule.runOnIdle { visible.value = false } + composeRule.waitForIdle() + composeRule.runOnIdle { visible.value = true } + composeRule.waitForIdle() + + // The latch only guards the taps on one showing of the card, it must not outlive it + composeRule.onNode(reviewButton).assertIsEnabled() + composeRule.onNodeWithText(dismissLabel).assertIsEnabled() + } + @Test fun `a review tap without an activity consumes neither latch`() { setContent(withActivity = false)