From e4bd22f6b1b3c2f3c59bcca96a846538f4db4ec6 Mon Sep 17 00:00:00 2001 From: Sami Kabbaj Date: Fri, 9 Oct 2026 19:51:00 +0200 Subject: [PATCH 1/5] Add a repository for one-time hints Remembers on-device which tips the user dismissed, like the pass zoom does for its preference. Co-Authored-By: Claude Sonnet 5.5 --- .../com/github/se/pooltrack/MainActivity.kt | 2 + .../se/pooltrack/model/hint/HintRepository.kt | 19 ++++++ .../model/hint/HintRepositoryLocal.kt | 27 +++++++++ .../model/hint/HintRepositoryProvider.kt | 19 ++++++ .../model/hint/HintRepositoryLocalTest.kt | 59 +++++++++++++++++++ .../model/hint/HintRepositoryProviderTest.kt | 53 +++++++++++++++++ .../se/pooltrack/utils/FakeHintRepository.kt | 23 ++++++++ 7 files changed, 202 insertions(+) create mode 100644 app/src/main/java/com/github/se/pooltrack/model/hint/HintRepository.kt create mode 100644 app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryLocal.kt create mode 100644 app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryProvider.kt create mode 100644 app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryLocalTest.kt create mode 100644 app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryProviderTest.kt create mode 100644 app/src/test/java/com/github/se/pooltrack/utils/FakeHintRepository.kt diff --git a/app/src/main/java/com/github/se/pooltrack/MainActivity.kt b/app/src/main/java/com/github/se/pooltrack/MainActivity.kt index 007b2b1..80d55d8 100644 --- a/app/src/main/java/com/github/se/pooltrack/MainActivity.kt +++ b/app/src/main/java/com/github/se/pooltrack/MainActivity.kt @@ -26,6 +26,7 @@ import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument import com.github.se.pooltrack.model.entry.EntryRepositoryProvider +import com.github.se.pooltrack.model.hint.HintRepositoryProvider import com.github.se.pooltrack.model.subscription.PassZoomRepositoryProvider import com.github.se.pooltrack.model.subscription.SubscriptionRepositoryProvider import com.github.se.pooltrack.model.swim.isSwimPending @@ -65,6 +66,7 @@ class MainActivity : ComponentActivity() { EntryRepositoryProvider.init(this) SubscriptionRepositoryProvider.init(this) PassZoomRepositoryProvider.init(this) + HintRepositoryProvider.init(this) UpdateRepositoryProvider.init(this) requestNotificationPermissionIfNeeded() startSwimTrackingWhileASwimIsPending() diff --git a/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepository.kt b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepository.kt new file mode 100644 index 0000000..c4a3e6e --- /dev/null +++ b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepository.kt @@ -0,0 +1,19 @@ +package com.github.se.pooltrack.model.hint + +import kotlinx.coroutines.flow.Flow + +/** A one-time tip the app shows until the user has seen it. */ +enum class Hint { + /** Visits in the history list can be deleted by swiping them to the left. */ + SwipeToDelete, +} + +/** Represents a repository that remembers which [Hint]s the user has dealt with. */ +interface HintRepository { + + /** Whether [hint] was dismissed, emitting again whenever that changes. */ + fun isDismissed(hint: Hint): Flow + + /** Remembers that the user dealt with [hint], so it is not shown again. */ + suspend fun dismiss(hint: Hint) +} diff --git a/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryLocal.kt b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryLocal.kt new file mode 100644 index 0000000..457407b --- /dev/null +++ b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryLocal.kt @@ -0,0 +1,27 @@ +package com.github.se.pooltrack.model.hint + +import android.content.Context +import androidx.datastore.preferences.core.booleanPreferencesKey +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.preferencesDataStore +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.map + +private val Context.hintDataStore by preferencesDataStore(name = "hint_prefs") + +private fun dismissedKey(hint: Hint) = booleanPreferencesKey("dismissed_${hint.name}") + +/** + * Stores which hints were dismissed on-device, using Jetpack DataStore. Like the pass zoom, it is a + * per-device preference: neither scoped to an account nor backed up. + */ +class HintRepositoryLocal(private val context: Context) : HintRepository { + + override fun isDismissed(hint: Hint): Flow = + context.hintDataStore.data.map { it[dismissedKey(hint)] ?: false }.distinctUntilChanged() + + override suspend fun dismiss(hint: Hint) { + context.hintDataStore.edit { it[dismissedKey(hint)] = true } + } +} diff --git a/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryProvider.kt b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryProvider.kt new file mode 100644 index 0000000..36d4810 --- /dev/null +++ b/app/src/main/java/com/github/se/pooltrack/model/hint/HintRepositoryProvider.kt @@ -0,0 +1,19 @@ +package com.github.se.pooltrack.model.hint + +import android.content.Context + +/** + * Provides a single instance of the repository in the app. `repository` is mutable for testing + * purposes. The on-device implementation needs a [Context], so [init] must be called (e.g. from + * `MainActivity`) before `repository` is first read. + */ +object HintRepositoryProvider { + lateinit var repository: HintRepository + + /** Creates the on-device repository, unless one was already set (e.g. a fake in a test). */ + fun init(context: Context) { + if (!::repository.isInitialized) { + repository = HintRepositoryLocal(context.applicationContext) + } + } +} diff --git a/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryLocalTest.kt b/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryLocalTest.kt new file mode 100644 index 0000000..1fb9903 --- /dev/null +++ b/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryLocalTest.kt @@ -0,0 +1,59 @@ +package com.github.se.pooltrack.model.hint + +import android.content.Context +import com.github.se.pooltrack.model.clearPreferencesDataStore +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class HintRepositoryLocalTest { + + private lateinit var appContext: Context + private lateinit var repository: HintRepositoryLocal + + @Before + fun setUp() = runTest { + appContext = RuntimeEnvironment.getApplication() + clearPreferencesDataStore(appContext, FILE_CLASS, PROPERTY) + repository = HintRepositoryLocal(appContext) + } + + @Test + fun isDismissed_isFalse_beforeTheHintWasDismissed() = runTest { + assertFalse(repository.isDismissed(Hint.SwipeToDelete).first()) + } + + @Test + fun isDismissed_isTrue_afterDismissing() = runTest { + repository.dismiss(Hint.SwipeToDelete) + + assertTrue(repository.isDismissed(Hint.SwipeToDelete).first()) + } + + @Test + fun isDismissed_survivesANewRepositoryInstance() = runTest { + repository.dismiss(Hint.SwipeToDelete) + + assertTrue(HintRepositoryLocal(appContext).isDismissed(Hint.SwipeToDelete).first()) + } + + @Test + fun dismiss_twice_changesNothing() = runTest { + repository.dismiss(Hint.SwipeToDelete) + repository.dismiss(Hint.SwipeToDelete) + + assertTrue(repository.isDismissed(Hint.SwipeToDelete).first()) + } + + private companion object { + const val FILE_CLASS = "com.github.se.pooltrack.model.hint.HintRepositoryLocalKt" + const val PROPERTY = "hintDataStore" + } +} diff --git a/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryProviderTest.kt b/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryProviderTest.kt new file mode 100644 index 0000000..917c947 --- /dev/null +++ b/app/src/test/java/com/github/se/pooltrack/model/hint/HintRepositoryProviderTest.kt @@ -0,0 +1,53 @@ +package com.github.se.pooltrack.model.hint + +import com.github.se.pooltrack.utils.FakeHintRepository +import org.junit.After +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class HintRepositoryProviderTest { + + private var previous: HintRepository? = null + + @Before + fun setUp() { + previous = field().get(HintRepositoryProvider) as? HintRepository + } + + @After + fun tearDown() { + field().set(HintRepositoryProvider, previous) + } + + // `repository` is a lateinit, so the only way to put the singleton back into its "never + // initialised" state is through its backing field. + private fun field() = + HintRepositoryProvider::class.java.getDeclaredField("repository").apply { + isAccessible = true + } + + @Test + fun init_createsOnDeviceRepository_whenNoneWasSet() { + field().set(HintRepositoryProvider, null) + + HintRepositoryProvider.init(RuntimeEnvironment.getApplication()) + + assertTrue(HintRepositoryProvider.repository is HintRepositoryLocal) + } + + @Test + fun init_keepsExistingRepository_whenOneWasAlreadySet() { + val fake = FakeHintRepository() + HintRepositoryProvider.repository = fake + + HintRepositoryProvider.init(RuntimeEnvironment.getApplication()) + + assertSame(fake, HintRepositoryProvider.repository) + } +} diff --git a/app/src/test/java/com/github/se/pooltrack/utils/FakeHintRepository.kt b/app/src/test/java/com/github/se/pooltrack/utils/FakeHintRepository.kt new file mode 100644 index 0000000..800ffe3 --- /dev/null +++ b/app/src/test/java/com/github/se/pooltrack/utils/FakeHintRepository.kt @@ -0,0 +1,23 @@ +package com.github.se.pooltrack.utils + +import com.github.se.pooltrack.model.hint.Hint +import com.github.se.pooltrack.model.hint.HintRepository +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.map + +/** [HintRepository] for unit tests: keeps the dismissed hints in memory. */ +class FakeHintRepository(dismissed: Set = emptySet()) : HintRepository { + + private val state = MutableStateFlow(dismissed) + + /** The hints dismissed so far. */ + val dismissed: Set + get() = state.value + + override fun isDismissed(hint: Hint): Flow = state.map { hint in it } + + override suspend fun dismiss(hint: Hint) { + state.value = state.value + hint + } +} From 7e8727c493ebfa80c1ca48253e996806b45f08b5 Mon Sep 17 00:00:00 2001 From: Sami Kabbaj Date: Fri, 9 Oct 2026 19:51:04 +0200 Subject: [PATCH 2/5] Expose the swipe tip from the history view model Shown until it is dismissed, and false until the stored answer is read so it never flashes for someone who already dismissed it. Co-Authored-By: Claude Sonnet 5.5 --- .../pooltrack/ui/history/HistoryScreenTest.kt | 2 + .../pooltrack/ui/history/HistoryViewModel.kt | 22 +++++ .../ui/history/HistoryViewModelTest.kt | 83 +++++++++++++++---- 3 files changed, 93 insertions(+), 14 deletions(-) diff --git a/app/src/androidTest/java/com/github/se/pooltrack/ui/history/HistoryScreenTest.kt b/app/src/androidTest/java/com/github/se/pooltrack/ui/history/HistoryScreenTest.kt index 97b3a37..ffd11c6 100644 --- a/app/src/androidTest/java/com/github/se/pooltrack/ui/history/HistoryScreenTest.kt +++ b/app/src/androidTest/java/com/github/se/pooltrack/ui/history/HistoryScreenTest.kt @@ -8,6 +8,7 @@ import androidx.compose.ui.test.performClick import androidx.datastore.preferences.preferencesDataStoreFile import androidx.test.platform.app.InstrumentationRegistry import com.github.se.pooltrack.model.entry.EntryRepositoryLocal +import com.github.se.pooltrack.model.hint.HintRepositoryLocal import com.github.se.pooltrack.model.subscription.SubscriptionRepositoryLocal import kotlinx.coroutines.flow.flowOf import org.junit.Before @@ -48,6 +49,7 @@ class HistoryScreenTest { HistoryViewModel( EntryRepositoryLocal(application, currentUid = uid), SubscriptionRepositoryLocal(application, currentUid = uid), + HintRepositoryLocal(application), ) composeTestRule.setContent { HistoryScreen(viewModel = viewModel) } } diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryViewModel.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryViewModel.kt index f7d955b..d2ce5b4 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryViewModel.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryViewModel.kt @@ -6,6 +6,9 @@ import com.github.se.pooltrack.model.entry.Entry import com.github.se.pooltrack.model.entry.EntryRepository import com.github.se.pooltrack.model.entry.EntryRepositoryProvider import com.github.se.pooltrack.model.entry.timestamp +import com.github.se.pooltrack.model.hint.Hint +import com.github.se.pooltrack.model.hint.HintRepository +import com.github.se.pooltrack.model.hint.HintRepositoryProvider import com.github.se.pooltrack.model.subscription.Subscription import com.github.se.pooltrack.model.subscription.SubscriptionRepository import com.github.se.pooltrack.model.subscription.SubscriptionRepositoryProvider @@ -17,6 +20,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -44,16 +48,34 @@ data class EntryLink(val entry: Entry, val subscriptionId: String) * @property repository The repository used to read, add and delete entries. * @property subscriptionRepository The repository used to find which subscription a past entry * belongs to. + * @property hintRepository The repository that remembers which one-time tips were dismissed. */ class HistoryViewModel( private val repository: EntryRepository = EntryRepositoryProvider.repository, private val subscriptionRepository: SubscriptionRepository = SubscriptionRepositoryProvider.repository, + private val hintRepository: HintRepository = HintRepositoryProvider.repository, ) : ViewModel() { val entries: StateFlow> = repository.getEntries().stateIn(viewModelScope, SharingStarted.Eagerly, emptyList()) + /** + * Whether to show the tip about swiping a visit to delete it: until the user dismisses it, or + * proves they know by deleting a visit that way. `false` until the stored answer is read, so it + * never flashes up for someone who already dismissed it. + */ + val showSwipeHint: StateFlow = + hintRepository + .isDismissed(Hint.SwipeToDelete) + .map { !it } + .stateIn(viewModelScope, SharingStarted.Eagerly, false) + + /** Remembers that the swipe tip was dealt with, so it is not shown again. */ + fun onSwipeHintDismissed() { + viewModelScope.launch { hintRepository.dismiss(Hint.SwipeToDelete) } + } + private val _addPastEntryResult = MutableStateFlow(null) /** The outcome of the last [onAddPastEntry], or `null` before one ran or once it was handled. */ diff --git a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryViewModelTest.kt b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryViewModelTest.kt index 4bd9cb1..3a88432 100644 --- a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryViewModelTest.kt +++ b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryViewModelTest.kt @@ -1,8 +1,10 @@ package com.github.se.pooltrack.ui.history import com.github.se.pooltrack.model.entry.Entry +import com.github.se.pooltrack.model.hint.Hint import com.github.se.pooltrack.model.subscription.Subscription import com.github.se.pooltrack.utils.FakeEntryRepository +import com.github.se.pooltrack.utils.FakeHintRepository import com.github.se.pooltrack.utils.FakeSubscriptionRepository import com.github.se.pooltrack.utils.MainDispatcherRule import java.time.Instant @@ -21,7 +23,8 @@ class HistoryViewModelTest { @Test fun entries_isEmpty_whenNoEntriesRecorded() { - val viewModel = HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository()) + val viewModel = + HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository(), FakeHintRepository()) assertEquals(emptyList(), viewModel.entries.value) } @@ -32,6 +35,7 @@ class HistoryViewModelTest { HistoryViewModel( FakeEntryRepository(listOf(older, newer)), FakeSubscriptionRepository(), + FakeHintRepository(), ) assertEquals(listOf(newer, older), viewModel.entries.value) @@ -40,7 +44,7 @@ class HistoryViewModelTest { @Test fun entries_updates_whenRepositoryRecordsAnEntry() = runTest { val repository = FakeEntryRepository(listOf(older)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) repository.addEntry(newer) @@ -50,7 +54,7 @@ class HistoryViewModelTest { @Test fun onDeleteEntry_removesOnlyThatEntry() { val repository = FakeEntryRepository(listOf(older, newer)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onDeleteEntry(newer) @@ -62,7 +66,7 @@ class HistoryViewModelTest { fun onRestoreEntry_putsBackADeletedEntryWithAllItsData() { val detailed = newer.copy(swimDistanceMeters = 1200, swimDurationMillis = 3_000_000L) val repository = FakeEntryRepository(listOf(older, detailed)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onDeleteEntry(detailed) viewModel.onRestoreEntry(detailed) @@ -85,7 +89,7 @@ class HistoryViewModelTest { @Test fun onAddPastEntry_recordsTheEntry_andReportsItAsAdded() { val repository = FakeEntryRepository() - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onAddPastEntry(past, now) @@ -99,7 +103,12 @@ class HistoryViewModelTest { val expired = subscription("old", "2024-09-01T00:00:00Z", "2025-08-31T00:00:00Z") val valid = subscription("pass", "2025-09-14T08:39:00Z", "2026-09-14T08:39:00Z") val repository = FakeEntryRepository() - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(expired, valid))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(expired, valid)), + FakeHintRepository(), + ) viewModel.onAddPastEntry(past, now) @@ -110,7 +119,12 @@ class HistoryViewModelTest { fun onAddPastEntry_leavesSubscriptionEmpty_whenEveryoneExpiredBeforeThen() { val expired = subscription("expired", "2024-01-01T00:00:00Z", "2025-01-01T00:00:00Z") val repository = FakeEntryRepository() - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(expired))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(expired)), + FakeHintRepository(), + ) viewModel.onAddPastEntry(past, now) @@ -120,7 +134,7 @@ class HistoryViewModelTest { @Test fun onAddPastEntry_refusesAFutureTime_andRecordsNothing() { val repository = FakeEntryRepository() - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onAddPastEntry(now.plusSeconds(60), now) @@ -135,7 +149,7 @@ class HistoryViewModelTest { fun onAddPastEntry_refusesADuplicateTimestamp() { val existing = Entry(timestampEpochMilli = past.toEpochMilli(), subscriptionId = "pass") val repository = FakeEntryRepository(listOf(existing)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onAddPastEntry(past, now) @@ -148,7 +162,8 @@ class HistoryViewModelTest { @Test fun onAddPastEntryResultHandled_clearsTheResult() { - val viewModel = HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository()) + val viewModel = + HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository(), FakeHintRepository()) viewModel.onAddPastEntry(past, now) viewModel.onAddPastEntryResultHandled() @@ -169,7 +184,12 @@ class HistoryViewModelTest { // Subscriptions are "added" when put in the app, not when bought, so that date says nothing. val addedToday = subscription("pass", "2026-10-06T10:00:00Z", "2026-09-14T08:39:00Z") val repository = FakeEntryRepository() - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(addedToday))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(addedToday)), + FakeHintRepository(), + ) viewModel.onAddPastEntry(past, now) @@ -203,6 +223,7 @@ class HistoryViewModelTest { HistoryViewModel( FakeEntryRepository(listOf(unlinked, linked)), FakeSubscriptionRepository(listOf(expiredPass)), + FakeHintRepository(), ) assertEquals(listOf(EntryLink(unlinked, "pass")), viewModel.linkSuggestions.value) @@ -211,7 +232,11 @@ class HistoryViewModelTest { @Test fun linkSuggestions_isEmpty_whenNoSubscriptionCoversTheEntry() { val viewModel = - HistoryViewModel(FakeEntryRepository(listOf(unlinked)), FakeSubscriptionRepository()) + HistoryViewModel( + FakeEntryRepository(listOf(unlinked)), + FakeSubscriptionRepository(), + FakeHintRepository(), + ) assertEquals(emptyList(), viewModel.linkSuggestions.value) } @@ -219,7 +244,12 @@ class HistoryViewModelTest { @Test fun onLinkEntries_attachesTheEntriesToTheirSubscription_andClearsTheSuggestions() { val repository = FakeEntryRepository(listOf(unlinked)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(expiredPass))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(expiredPass)), + FakeHintRepository(), + ) viewModel.onLinkEntries(viewModel.linkSuggestions.value) @@ -230,7 +260,12 @@ class HistoryViewModelTest { @Test fun onUnlinkEntries_takesTheLinkBack() { val repository = FakeEntryRepository(listOf(unlinked)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(expiredPass))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(expiredPass)), + FakeHintRepository(), + ) val links = viewModel.linkSuggestions.value viewModel.onLinkEntries(links) @@ -238,4 +273,24 @@ class HistoryViewModelTest { assertEquals(listOf(unlinked), repository.storedEntries) } + + @Test + fun showSwipeHint_isTrue_untilItIsDismissed() { + val hints = FakeHintRepository() + val viewModel = HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository(), hints) + assertEquals(true, viewModel.showSwipeHint.value) + + viewModel.onSwipeHintDismissed() + + assertEquals(false, viewModel.showSwipeHint.value) + assertEquals(setOf(Hint.SwipeToDelete), hints.dismissed) + } + + @Test + fun showSwipeHint_isFalse_whenItWasDismissedBefore() { + val hints = FakeHintRepository(dismissed = setOf(Hint.SwipeToDelete)) + val viewModel = HistoryViewModel(FakeEntryRepository(), FakeSubscriptionRepository(), hints) + + assertEquals(false, viewModel.showSwipeHint.value) + } } From 6e2299a0fe4823d81292921175567cabe8c00cf2 Mon Sep 17 00:00:00 2001 From: Sami Kabbaj Date: Fri, 9 Oct 2026 19:51:07 +0200 Subject: [PATCH 3/5] Say when a weekly distance is only partly logged A week where only some visits have a distance reads 4.2 km logged, since the total undercounts; the row chevrons go, they only covered by the add button. Co-Authored-By: Claude Sonnet 5.5 --- .../pooltrack/ui/history/HistoryListItems.kt | 31 ++++++++++++------- .../pooltrack/ui/history/HistoryListModel.kt | 9 +++++- .../ui/history/HistoryListModelTest.kt | 18 +++++++---- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt index a126d9c..eea0282 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt @@ -11,7 +11,6 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.automirrored.filled.KeyboardArrowRight import androidx.compose.material.icons.filled.Delete import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.HorizontalDivider @@ -75,7 +74,13 @@ internal fun weekLabel( /** The sticky title above the entries of one week, with how many visits it had. */ @Composable -internal fun WeekHeader(label: String, visits: Int, meters: Int, modifier: Modifier = Modifier) { +internal fun WeekHeader( + label: String, + visits: Int, + meters: Int, + loggedVisits: Int, + modifier: Modifier = Modifier, +) { Surface( modifier = modifier.fillMaxWidth().testTag(HistoryScreenTestTags.WEEK_HEADER), color = MaterialTheme.colorScheme.background, @@ -91,7 +96,7 @@ internal fun WeekHeader(label: String, visits: Int, meters: Int, modifier: Modif fontWeight = FontWeight.Bold, ) Text( - text = weekTotals(visits, meters), + text = weekTotals(visits, meters, loggedVisits), style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -214,11 +219,6 @@ internal fun SwipeableEntryRow( ) } } - Icon( - imageVector = Icons.AutoMirrored.Filled.KeyboardArrowRight, - contentDescription = null, - tint = MaterialTheme.colorScheme.onSurfaceVariant, - ) } HorizontalDivider(color = MaterialTheme.colorScheme.outlineVariant) } @@ -226,7 +226,14 @@ internal fun SwipeableEntryRow( } } -/** "5 visits · 4.2 km"; the distance is left out when none was logged that week. */ -internal fun weekTotals(visits: Int, meters: Int): String = - listOfNotNull(visitsLabel(visits), if (meters > 0) formatTotalDistance(meters) else null) - .joinToString(" · ") +/** + * "5 visits · 4.2 km". The distance is left out when none was logged that week, and reads "logged" + * (as in "4.2 km logged") when only some of the visits have one, since the total then undercounts. + */ +internal fun weekTotals(visits: Int, meters: Int, loggedVisits: Int): String { + val distance = + if (meters > 0) { + formatTotalDistance(meters) + if (loggedVisits < visits) " logged" else "" + } else null + return listOfNotNull(visitsLabel(visits), distance).joinToString(" · ") +} diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListModel.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListModel.kt index 131af9a..7af5a8c 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListModel.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListModel.kt @@ -20,8 +20,14 @@ sealed interface HistoryRow { * * @property visits Entries in the week. * @property meters The distance logged during the week, in metres. + * @property loggedVisits How many of the [visits] have a distance, which the [meters] add up. */ - data class Week(val start: LocalDate, val visits: Int, val meters: Int) : HistoryRow { + data class Week( + val start: LocalDate, + val visits: Int, + val meters: Int, + val loggedVisits: Int, + ) : HistoryRow { override val key: Any = "week-$start" } @@ -50,6 +56,7 @@ fun buildHistoryRows( start = start, visits = items.size, meters = items.sumOf { it.entry.swimDistanceMeters ?: 0 }, + loggedVisits = items.count { it.entry.swimDistanceMeters != null }, ) ) + items } diff --git a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryListModelTest.kt b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryListModelTest.kt index 42b6d7c..bd0db28 100644 --- a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryListModelTest.kt +++ b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryListModelTest.kt @@ -42,10 +42,10 @@ class HistoryListModelTest { assertEquals( listOf( - HistoryRow.Week(thisWeekMon, visits = 2, meters = 0), + HistoryRow.Week(thisWeekMon, visits = 2, meters = 0, loggedVisits = 0), HistoryRow.Item(thisWeekWed, wednesday), HistoryRow.Item(thisWeekMon, monday9), - HistoryRow.Week(LocalDate.of(2026, 9, 28), visits = 1, meters = 0), + HistoryRow.Week(LocalDate.of(2026, 9, 28), visits = 1, meters = 0, loggedVisits = 0), HistoryRow.Item(lastWeekSun, sunday), ), rows, @@ -62,6 +62,7 @@ class HistoryListModelTest { ) assertEquals(1500, (rows.first() as HistoryRow.Week).meters) + assertEquals(2, (rows.first() as HistoryRow.Week).loggedVisits) } @Test @@ -129,13 +130,18 @@ class HistoryListModelTest { } @Test - fun weekTotals_listsVisitsAndDistance_whenThereIsOne() { - assertEquals("3 visits · 4.2 km", weekTotals(3, 4200)) - assertEquals("1 visit · 600 m", weekTotals(1, 600)) + fun weekTotals_listsVisitsAndDistance_whenEveryVisitHasOne() { + assertEquals("3 visits · 4.2 km", weekTotals(3, 4200, loggedVisits = 3)) + assertEquals("1 visit · 600 m", weekTotals(1, 600, loggedVisits = 1)) + } + + @Test + fun weekTotals_saysLogged_whenOnlySomeVisitsHaveADistance() { + assertEquals("4 visits · 2.6 km logged", weekTotals(4, 2600, loggedVisits = 3)) } @Test fun weekTotals_leavesOutTheDistance_whenNoneWasLogged() { - assertEquals("2 visits", weekTotals(2, 0)) + assertEquals("2 visits", weekTotals(2, 0, loggedVisits = 0)) } } From fb4df5e707654f8123957b3fbbeaa390ee72590c Mon Sep 17 00:00:00 2001 From: Sami Kabbaj Date: Fri, 9 Oct 2026 19:51:10 +0200 Subject: [PATCH 4/5] Simplify the overview and add the swipe tip Drop the this-week line, expand the calendar with a bar across the card, show a dismissible swipe-to-delete tip, and collapse the add button to a plus once the list is scrolled. Co-Authored-By: Claude Sonnet 5.5 --- .../se/pooltrack/ui/history/HistoryScreen.kt | 45 ++++++++- .../ui/history/heatmap/HistoryOverview.kt | 91 +++++++++---------- .../history/HistoryScreenRobolectricTest.kt | 78 +++++++++++++--- .../heatmap/HistoryOverviewTextTest.kt | 14 --- 4 files changed, 152 insertions(+), 76 deletions(-) delete mode 100644 app/src/test/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverviewTextTest.kt diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt index 01a8f5d..77405bf 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt @@ -61,6 +61,8 @@ object HistoryScreenTestTags { const val EMPTY_MESSAGE = "HistoryScreenEmptyMessage" const val ENTRY_LIST = "HistoryScreenEntryList" const val WEEK_HEADER = "HistoryScreenWeekHeader" + const val SWIPE_HINT = "HistoryScreenSwipeHint" + const val SWIPE_HINT_DISMISS = "HistoryScreenSwipeHintDismiss" const val ENTRY_ITEM = "HistoryScreenEntryItem" const val ADD_PAST_ENTRY_BUTTON = "HistoryScreenAddPastEntryButton" const val UNDO_ADD_ACTION = "Undo" @@ -93,6 +95,8 @@ fun HistoryScreen( val snackbarHostState = remember { SnackbarHostState() } val scope = rememberCoroutineScope() val linkSuggestions by viewModel.linkSuggestions.collectAsState() + val showSwipeHint by viewModel.showSwipeHint.collectAsState() + val listState = rememberLazyListState() // On success the sheet closes and an Undo snackbar appears. The snackbar is launched in its own // scope: clearing the result below would otherwise cancel this effect, and the snackbar with it. @@ -115,6 +119,7 @@ fun HistoryScreen( // Deleting needs no confirmation: the swipe is deliberate and the snackbar offers an Undo. val onDeleteEntry: (Entry) -> Unit = { entry -> viewModel.onDeleteEntry(entry) + viewModel.onSwipeHintDismissed() scope.launch { snackbarHostState.currentSnackbarData?.dismiss() val outcome = @@ -153,6 +158,7 @@ fun HistoryScreen( onClick = { showAddSheet = true }, icon = { Icon(Icons.Filled.Add, contentDescription = null) }, text = { Text("Add past entry") }, + expanded = listState.firstVisibleItemIndex == 0, modifier = Modifier.testTag(HistoryScreenTestTags.ADD_PAST_ENTRY_BUTTON), ) }, @@ -193,10 +199,10 @@ fun HistoryScreen( val countsByDay = remember(entries) { entryCountsByDay(entries, ZONE) } val rows = remember(entries, firstDayOfWeek) { buildHistoryRows(entries, firstDayOfWeek, ZONE) } - val listState = rememberLazyListState() val headerClearance = with(LocalDensity.current) { STICKY_HEADER_CLEARANCE.roundToPx() } - // The overview, and the link banner when there is one, come before the rows. - val leadingItems = if (linkSuggestions.isEmpty()) 1 else 2 + // The overview, then the link banner and the swipe tip when they show, come before the rows. + val leadingItems = + 1 + (if (linkSuggestions.isEmpty()) 0 else 1) + (if (showSwipeHint) 1 else 0) // A picked day stays highlighted for a moment, so the eye can find its row. LaunchedEffect(highlightedDate) { @@ -237,6 +243,14 @@ fun HistoryScreen( modifier = Modifier.padding(vertical = 4.dp), ) } + if (showSwipeHint) { + item(key = "swipe-hint") { + SwipeHint( + onDismiss = viewModel::onSwipeHintDismissed, + modifier = Modifier.padding(vertical = 4.dp), + ) + } + } if (linkSuggestions.isNotEmpty()) { item(key = "link-banner") { LinkBanner( @@ -269,6 +283,7 @@ fun HistoryScreen( label = weekLabel(row.start, today, firstDayOfWeek), visits = row.visits, meters = row.meters, + loggedVisits = row.loggedVisits, ) } is HistoryRow.Item -> @@ -295,6 +310,30 @@ fun HistoryScreen( } } +/** A one-time tip that visits can be swiped away, shown until it is dismissed. */ +@Composable +private fun SwipeHint(onDismiss: () -> Unit, modifier: Modifier = Modifier) { + Card(modifier = modifier.fillMaxWidth().testTag(HistoryScreenTestTags.SWIPE_HINT)) { + Row( + modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Text( + text = "Tip: swipe a visit to the left to delete it.", + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.weight(1f).padding(vertical = 12.dp), + ) + TextButton( + onClick = onDismiss, + modifier = Modifier.testTag(HistoryScreenTestTags.SWIPE_HINT_DISMISS), + ) { + Text("Got it") + } + } + } +} + /** Offers to attach entries that belong to no subscription to the one that covers them. */ @Composable private fun LinkBanner(count: Int, onLink: () -> Unit, modifier: Modifier = Modifier) { diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverview.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverview.kt index a3d5c37..13092cd 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverview.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverview.kt @@ -1,5 +1,6 @@ package com.github.se.pooltrack.ui.history.heatmap +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -12,7 +13,6 @@ import androidx.compose.material3.Card import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text -import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -22,7 +22,6 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag -import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import java.time.DayOfWeek import java.time.LocalDate @@ -30,16 +29,12 @@ import java.time.YearMonth object HistoryOverviewTestTags { const val CARD = "HistoryOverviewCard" - const val SUMMARY = "HistoryOverviewSummary" const val TOGGLE = "HistoryOverviewToggle" } -/** The one-line summary: "5 this week". */ -fun overviewSummary(visitsThisWeek: Int): String = "$visitsThisWeek this week" - /** - * The card on top of the History list. By default it is compact: a one-line summary and the current - * week as seven days. Expanding it shows the month calendar, which zooms out to the year. + * The card on top of the History list. By default it is compact: just the current week as seven + * days. The bar along its bottom expands it to the month calendar, which zooms out to the year. * * @param countsByDay Entries per day, see [entryCountsByDay]. * @param today The current day. @@ -57,57 +52,57 @@ fun HistoryOverview( modifier: Modifier = Modifier, ) { var expanded by rememberSaveable { mutableStateOf(false) } - val visitsThisWeek = - remember(countsByDay, today, firstDayOfWeek) { - buildWeekStrip(today, countsByDay, firstDayOfWeek).sumOf { it.count } - } val months = remember(countsByDay, today) { monthsBetween(YearMonth.from(countsByDay.keys.minOrNull() ?: today), YearMonth.from(today)) } Card(modifier = modifier.fillMaxWidth().testTag(HistoryOverviewTestTags.CARD)) { - Column(modifier = Modifier.padding(16.dp), verticalArrangement = Arrangement.spacedBy(8.dp)) { - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically, + Column { + Column( + modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 16.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), ) { - Text( - text = overviewSummary(visitsThisWeek), - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.Bold, - modifier = Modifier.weight(1f).testTag(HistoryOverviewTestTags.SUMMARY), - ) - TextButton( - onClick = { expanded = !expanded }, - modifier = Modifier.testTag(HistoryOverviewTestTags.TOGGLE), - ) { - Text(if (expanded) "Hide" else "Calendar") - Icon( - imageVector = - if (expanded) Icons.Filled.KeyboardArrowUp else Icons.Filled.KeyboardArrowDown, - contentDescription = null, + if (expanded) { + HistoryCalendar( + months = months, + countsByDay = countsByDay, + today = today, + firstDayOfWeek = firstDayOfWeek, + selectedDate = selectedDate, + onDayClick = onDayClick, + ) + HeatmapLegend(modifier = Modifier.align(Alignment.End)) + } else { + WeekStrip( + countsByDay = countsByDay, + today = today, + selectedDate = selectedDate, + onDayClick = onDayClick, + firstDayOfWeek = firstDayOfWeek, ) } } - if (expanded) { - HistoryCalendar( - months = months, - countsByDay = countsByDay, - today = today, - firstDayOfWeek = firstDayOfWeek, - selectedDate = selectedDate, - onDayClick = onDayClick, + // A bar across the whole card, so expanding it doesn't depend on hitting a small button. + Row( + modifier = + Modifier.fillMaxWidth() + .clickable { expanded = !expanded } + .padding(vertical = 12.dp) + .testTag(HistoryOverviewTestTags.TOGGLE), + horizontalArrangement = Arrangement.Center, + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = if (expanded) "Hide calendar" else "Show calendar", + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.primary, ) - HeatmapLegend(modifier = Modifier.align(Alignment.End)) - } else { - WeekStrip( - countsByDay = countsByDay, - today = today, - selectedDate = selectedDate, - onDayClick = onDayClick, - firstDayOfWeek = firstDayOfWeek, + Icon( + imageVector = + if (expanded) Icons.Filled.KeyboardArrowUp else Icons.Filled.KeyboardArrowDown, + contentDescription = null, + tint = MaterialTheme.colorScheme.primary, ) } } diff --git a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt index 7480aa7..cf781fb 100644 --- a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt +++ b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt @@ -16,6 +16,8 @@ import androidx.compose.ui.test.performTouchInput import androidx.compose.ui.test.swipeLeft import com.github.se.pooltrack.model.entry.Entry import com.github.se.pooltrack.model.entry.EntryRepositoryProvider +import com.github.se.pooltrack.model.hint.Hint +import com.github.se.pooltrack.model.hint.HintRepositoryProvider import com.github.se.pooltrack.model.subscription.Subscription import com.github.se.pooltrack.model.subscription.SubscriptionRepositoryProvider import com.github.se.pooltrack.ui.history.heatmap.HistoryOverviewTestTags @@ -26,6 +28,7 @@ import com.github.se.pooltrack.ui.navigation.NavigationActions import com.github.se.pooltrack.ui.navigation.NavigationTestTags import com.github.se.pooltrack.ui.navigation.Screen import com.github.se.pooltrack.utils.FakeEntryRepository +import com.github.se.pooltrack.utils.FakeHintRepository import com.github.se.pooltrack.utils.FakeSubscriptionRepository import com.github.se.pooltrack.utils.MainDispatcherRule import io.mockk.mockk @@ -63,7 +66,7 @@ class HistoryScreenRobolectricTest { ) private fun show(repository: FakeEntryRepository, navigationActions: NavigationActions? = null) { - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository()) + val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(), FakeHintRepository()) composeTestRule.setContent { HistoryScreen(viewModel, navigationActions) } } @@ -282,16 +285,18 @@ class HistoryScreenRobolectricTest { val entry = entryAt(today) EntryRepositoryProvider.repository = FakeEntryRepository(listOf(entry)) SubscriptionRepositoryProvider.repository = FakeSubscriptionRepository() + HintRepositoryProvider.repository = FakeHintRepository() try { assertEquals(listOf(entry), HistoryViewModel().entries.value) } finally { // The providers are lateinit singletons: un-initialise them so later tests are unaffected. - listOf(EntryRepositoryProvider, SubscriptionRepositoryProvider).forEach { provider -> - provider::class.java.getDeclaredField("repository").apply { - isAccessible = true - set(provider, null) - } - } + listOf(EntryRepositoryProvider, SubscriptionRepositoryProvider, HintRepositoryProvider) + .forEach { provider -> + provider::class.java.getDeclaredField("repository").apply { + isAccessible = true + set(provider, null) + } + } } } @@ -365,7 +370,12 @@ class HistoryScreenRobolectricTest { fun historyScreen_offersToLinkUnlinkedEntries_andLinksThemWithUndo() { val unlinked = Entry(entryAt(today.minusDays(30)).timestampEpochMilli, subscriptionId = null) val repository = FakeEntryRepository(listOf(unlinked)) - val viewModel = HistoryViewModel(repository, FakeSubscriptionRepository(listOf(expiredPass))) + val viewModel = + HistoryViewModel( + repository, + FakeSubscriptionRepository(listOf(expiredPass)), + FakeHintRepository(), + ) composeTestRule.setContent { HistoryScreen(viewModel) } composeTestRule.onNodeWithTag(HistoryScreenTestTags.LINK_BANNER).assertIsDisplayed() @@ -394,12 +404,10 @@ class HistoryScreenRobolectricTest { } @Test - fun historyScreen_showsACompactOverview_withTheWeekAndOneSummaryLine() { + fun historyScreen_showsACompactOverview_withJustTheWeek() { show(FakeEntryRepository(listOf(entryAt(today)))) composeTestRule.onNodeWithTag(HistoryOverviewTestTags.CARD).assertIsDisplayed() - composeTestRule.onNodeWithTag(HistoryOverviewTestTags.SUMMARY).assertIsDisplayed() - composeTestRule.onNodeWithText("1 this week").assertIsDisplayed() composeTestRule.onNodeWithTag(WeekStripTestTags.day(today)).assertExists() composeTestRule.onAllNodesWithTag(MonthCalendarTestTags.CALENDAR).assertCountEquals(0) } @@ -474,4 +482,52 @@ class HistoryScreenRobolectricTest { composeTestRule.onNodeWithTag(MonthCalendarTestTags.day(older)).assertExists() composeTestRule.onAllNodesWithTag(YearCalendarTestTags.CALENDAR).assertCountEquals(0) } + + @Test + fun historyScreen_marksTheWeeksDistanceAsLogged_whenSomeVisitsHaveNone() { + val withDistance = entryAt(today, hour = 8).copy(swimDistanceMeters = 1200) + val without = entryAt(today, hour = 18) + show(FakeEntryRepository(listOf(withDistance, without))) + + composeTestRule + .onNode( + hasTestTag(HistoryScreenTestTags.WEEK_HEADER) and + hasAnyDescendant(hasText("2 visits · 1.2 km logged")) + ) + .assertExists() + } + + @Test + fun historyScreen_showsTheSwipeTip_untilItIsDismissed() { + show(FakeEntryRepository(listOf(entryAt(today)))) + composeTestRule.onNodeWithTag(HistoryScreenTestTags.SWIPE_HINT).assertIsDisplayed() + + composeTestRule.onNodeWithTag(HistoryScreenTestTags.SWIPE_HINT_DISMISS).performClick() + composeTestRule.waitForIdle() + + composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.SWIPE_HINT).assertCountEquals(0) + } + + @Test + fun historyScreen_hidesTheSwipeTip_whenItWasDismissedBefore() { + val viewModel = + HistoryViewModel( + FakeEntryRepository(listOf(entryAt(today))), + FakeSubscriptionRepository(), + FakeHintRepository(dismissed = setOf(Hint.SwipeToDelete)), + ) + composeTestRule.setContent { HistoryScreen(viewModel) } + composeTestRule.waitForIdle() + + composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.SWIPE_HINT).assertCountEquals(0) + } + + @Test + fun historyScreen_dismissesTheSwipeTip_afterAVisitWasSwipedAway() { + show(FakeEntryRepository(listOf(entryAt(today), entryAt(today.minusDays(40))))) + + swipeAwayFirstEntry() + + composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.SWIPE_HINT).assertCountEquals(0) + } } diff --git a/app/src/test/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverviewTextTest.kt b/app/src/test/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverviewTextTest.kt deleted file mode 100644 index 06ce702..0000000 --- a/app/src/test/java/com/github/se/pooltrack/ui/history/heatmap/HistoryOverviewTextTest.kt +++ /dev/null @@ -1,14 +0,0 @@ -package com.github.se.pooltrack.ui.history.heatmap - -import org.junit.Assert.assertEquals -import org.junit.Test - -class HistoryOverviewTextTest { - - @Test - fun overviewSummary_countsTheVisitsOfTheWeek() { - assertEquals("0 this week", overviewSummary(0)) - assertEquals("1 this week", overviewSummary(1)) - assertEquals("5 this week", overviewSummary(5)) - } -} From be2046573908d800a0b6e93ba910c5524f222d7b Mon Sep 17 00:00:00 2001 From: Sami Kabbaj Date: Fri, 9 Oct 2026 20:02:44 +0200 Subject: [PATCH 5/5] Confirm before deleting a visit and let a swipe spring back A swipe to the left now only asks to delete: the row springs back and a dialog asks for confirmation, so a swipe by mistake changes nothing. The Undo snackbar after a confirmed delete stays. Co-Authored-By: Claude Sonnet 5.5 --- .../pooltrack/ui/history/HistoryListItems.kt | 16 +++--- .../se/pooltrack/ui/history/HistoryScreen.kt | 42 ++++++++++++++- .../history/HistoryScreenRobolectricTest.kt | 54 +++++++++++++++++-- 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt index eea0282..6169ed6 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryListItems.kt @@ -144,7 +144,9 @@ private fun DateBlock(day: LocalDate, isToday: Boolean, locale: Locale = Locale. * @param texts What the row says, see [entryRowTexts]. * @param highlighted Whether to tint the row, e.g. because its day was picked in the calendar. * @param onClick Called when the row is tapped. - * @param onDelete Called once the row was swiped away. + * @param onDeleteRequested Called when the row was swiped to the left (or the screen reader action + * was used). The row never goes away by itself: it springs back, and the caller asks the user to + * confirm before deleting anything. */ @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -154,16 +156,16 @@ internal fun SwipeableEntryRow( texts: EntryRowTexts, highlighted: Boolean, onClick: () -> Unit, - onDelete: () -> Unit, + onDeleteRequested: () -> Unit, modifier: Modifier = Modifier, ) { val dismissState = rememberSwipeToDismissBoxState( + // Never settle on the dismissed position: the row springs back while the caller asks the + // user to confirm, so a swipe made by mistake leaves nothing changed. confirmValueChange = { value -> - if (value == SwipeToDismissBoxValue.EndToStart) { - onDelete() - true - } else false + if (value == SwipeToDismissBoxValue.EndToStart) onDeleteRequested() + false } ) SwipeToDismissBox( @@ -174,7 +176,7 @@ internal fun SwipeableEntryRow( customActions = listOf( CustomAccessibilityAction("Delete entry") { - onDelete() + onDeleteRequested() true } ) diff --git a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt index 77405bf..7289a5d 100644 --- a/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt +++ b/app/src/main/java/com/github/se/pooltrack/ui/history/HistoryScreen.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.outlined.DateRange +import androidx.compose.material3.AlertDialog import androidx.compose.material3.Card import androidx.compose.material3.ExtendedFloatingActionButton import androidx.compose.material3.Icon @@ -62,6 +63,8 @@ object HistoryScreenTestTags { const val ENTRY_LIST = "HistoryScreenEntryList" const val WEEK_HEADER = "HistoryScreenWeekHeader" const val SWIPE_HINT = "HistoryScreenSwipeHint" + const val CONFIRM_DELETE_BUTTON = "HistoryScreenConfirmDeleteButton" + const val CANCEL_DELETE_BUTTON = "HistoryScreenCancelDeleteButton" const val SWIPE_HINT_DISMISS = "HistoryScreenSwipeHintDismiss" const val ENTRY_ITEM = "HistoryScreenEntryItem" const val ADD_PAST_ENTRY_BUTTON = "HistoryScreenAddPastEntryButton" @@ -87,6 +90,7 @@ fun HistoryScreen( ) { val entries by viewModel.entries.collectAsState() var showAddSheet by remember { mutableStateOf(false) } + var entryPendingDeletion by remember { mutableStateOf(null) } var highlightedDate by remember { mutableStateOf(null) } // Remembered across openings, so adding several entries from the same day takes one tap each. var lastAddedDate by remember { mutableStateOf(LocalDate.now(ZONE)) } @@ -116,7 +120,7 @@ fun HistoryScreen( } } - // Deleting needs no confirmation: the swipe is deliberate and the snackbar offers an Undo. + // Deleting is confirmed first (see the dialog below), and a snackbar still offers an Undo. val onDeleteEntry: (Entry) -> Unit = { entry -> viewModel.onDeleteEntry(entry) viewModel.onSwipeHintDismissed() @@ -150,6 +154,33 @@ fun HistoryScreen( ) } + entryPendingDeletion?.let { entry -> + AlertDialog( + onDismissRequest = { entryPendingDeletion = null }, + title = { Text("Delete this visit?") }, + text = { Text("${deletionSummary(entry)} You can undo it right after.") }, + confirmButton = { + TextButton( + onClick = { + onDeleteEntry(entry) + entryPendingDeletion = null + }, + modifier = Modifier.testTag(HistoryScreenTestTags.CONFIRM_DELETE_BUTTON), + ) { + Text("Delete") + } + }, + dismissButton = { + TextButton( + onClick = { entryPendingDeletion = null }, + modifier = Modifier.testTag(HistoryScreenTestTags.CANCEL_DELETE_BUTTON), + ) { + Text("Cancel") + } + }, + ) + } + Scaffold( topBar = { TopNavigationMenu(Screen.History) }, snackbarHost = { SnackbarHost(snackbarHostState) }, @@ -300,7 +331,7 @@ fun HistoryScreen( onClick = { navigationActions?.navigateToEntryDetails(row.entry.timestampEpochMilli) }, - onDelete = { onDeleteEntry(row.entry) }, + onDeleteRequested = { entryPendingDeletion = row.entry }, ) } } @@ -376,3 +407,10 @@ private const val HIGHLIGHT_MILLIS = 2_500L /** How far below the top a scrolled-to row stops, so the sticky week header doesn't cover it. */ private val STICKY_HEADER_CLEARANCE = 56.dp + +/** "The visit on Wednesday, Oct 7 at 12:00 PM will be removed." */ +internal fun deletionSummary(entry: Entry): String { + val day = entry.timestamp.atZone(ZONE).toLocalDate() + val time = entryTimeFormatter().format(entry.timestamp) + return "The visit on ${dayLabel(day, LocalDate.now(ZONE))} at $time will be removed." +} diff --git a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt index cf781fb..96b3b67 100644 --- a/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt +++ b/app/src/test/java/com/github/se/pooltrack/ui/history/HistoryScreenRobolectricTest.kt @@ -128,6 +128,52 @@ class HistoryScreenRobolectricTest { composeTestRule.waitForIdle() } + /** Swipes the first row to the left and confirms the dialog that asks whether to delete it. */ + private fun deleteFirstEntry() { + swipeAwayFirstEntry() + composeTestRule.onNodeWithTag(HistoryScreenTestTags.CONFIRM_DELETE_BUTTON).performClick() + composeTestRule.waitForIdle() + } + + @Test + fun historyScreen_asksBeforeDeleting_andChangesNothingYet_whenAnEntryIsSwiped() { + val entry = entryAt(today) + val repository = FakeEntryRepository(listOf(entry)) + show(repository) + + swipeAwayFirstEntry() + + composeTestRule.onNodeWithText("Delete this visit?").assertIsDisplayed() + composeTestRule.onNode(hasText("will be removed.", substring = true)).assertIsDisplayed() + assertEquals(listOf(entry), repository.storedEntries) + } + + @Test + fun historyScreen_keepsTheEntry_andTheRowComesBack_whenDeletingIsCancelled() { + val entry = entryAt(today) + val repository = FakeEntryRepository(listOf(entry)) + show(repository) + swipeAwayFirstEntry() + + composeTestRule.onNodeWithTag(HistoryScreenTestTags.CANCEL_DELETE_BUTTON).performClick() + composeTestRule.waitForIdle() + + assertEquals(listOf(entry), repository.storedEntries) + composeTestRule.onAllNodesWithText("Delete this visit?").assertCountEquals(0) + composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.ENTRY_ITEM).assertCountEquals(1) + composeTestRule.onNodeWithText(timeLabel(entry)).assertIsDisplayed() + } + + @Test + fun deletionSummary_namesTheDayAndTime() { + val entry = entryAt(today, hour = 18) + + assertEquals( + "The visit on Today at ${timeLabel(entry)} will be removed.", + deletionSummary(entry), + ) + } + @Test fun historyScreen_deletesOnlyThatEntry_whenSwipedAway() { val keep = entryAt(today, hour = 8) @@ -136,7 +182,7 @@ class HistoryScreenRobolectricTest { show(repository) // Most recent first, so the first row is the 18:00 entry. - swipeAwayFirstEntry() + deleteFirstEntry() assertEquals(listOf(keep), repository.storedEntries) composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.ENTRY_ITEM).assertCountEquals(1) @@ -148,7 +194,7 @@ class HistoryScreenRobolectricTest { val repository = FakeEntryRepository(listOf(entryAt(today))) show(repository) - swipeAwayFirstEntry() + deleteFirstEntry() assertEquals(emptyList(), repository.storedEntries) composeTestRule.onNodeWithTag(HistoryScreenTestTags.EMPTY_MESSAGE).assertIsDisplayed() @@ -159,7 +205,7 @@ class HistoryScreenRobolectricTest { val entry = entryAt(today).copy(swimDistanceMeters = 900, swimDurationMillis = 60_000L) val repository = FakeEntryRepository(listOf(entry)) show(repository) - swipeAwayFirstEntry() + deleteFirstEntry() composeTestRule.onNodeWithText(HistoryScreenTestTags.UNDO_ADD_ACTION).performClick() composeTestRule.waitForIdle() @@ -526,7 +572,7 @@ class HistoryScreenRobolectricTest { fun historyScreen_dismissesTheSwipeTip_afterAVisitWasSwipedAway() { show(FakeEntryRepository(listOf(entryAt(today), entryAt(today.minusDays(40))))) - swipeAwayFirstEntry() + deleteFirstEntry() composeTestRule.onAllNodesWithTag(HistoryScreenTestTags.SWIPE_HINT).assertCountEquals(0) }