From 5e8a63a0463f7f84d5ba9aa27daf02a3d42f8c9f Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sun, 11 Jun 2023 13:34:22 +0200 Subject: [PATCH] Throttle UI actions instead of debouncing (#3651) Introduce Flow.throttleFirst(). In a flow this emits the first value, and each value afterwards that is > some timeout after the previous value. This prevents accidental double-taps on UI elements from generating multiple-actions. The previous code used debounce(). That has a similar effect, but with debounce() the code has to wait until after the timeout period has elapsed before it can process the action, leading to an unnecessary UI delay. With throttleFirst a value is emitted immediately, there's no need to wait. It's subsequent values that are potentially throttled. --- .../notifications/NotificationsViewModel.kt | 12 ++-- .../tusky/util/FlowExtensions.kt | 69 +++++++++++++++++++ .../tusky/util/FlowExtensionsTest.kt | 55 +++++++++++++++ 3 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/com/keylesspalace/tusky/util/FlowExtensions.kt create mode 100644 app/src/test/java/com/keylesspalace/tusky/util/FlowExtensionsTest.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 0cf5d46b..ea0f2e8e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -40,6 +40,7 @@ import com.keylesspalace.tusky.usecase.TimelineCases import com.keylesspalace.tusky.util.StatusDisplayOptions import com.keylesspalace.tusky.util.deserialize import com.keylesspalace.tusky.util.serialize +import com.keylesspalace.tusky.util.throttleFirst import com.keylesspalace.tusky.util.toViewData import com.keylesspalace.tusky.viewdata.NotificationViewData import com.keylesspalace.tusky.viewdata.StatusViewData @@ -52,7 +53,6 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterIsInstance @@ -65,6 +65,8 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.rx3.await import retrofit2.HttpException import javax.inject.Inject +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.ExperimentalTime data class UiState( /** Filtered notification types */ @@ -274,7 +276,7 @@ sealed class UiError( } } -@OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class) +@OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class, ExperimentalTime::class) class NotificationsViewModel @Inject constructor( private val repository: NotificationsRepository, private val preferences: SharedPreferences, @@ -390,7 +392,7 @@ class NotificationsViewModel @Inject constructor( // Handle NotificationAction.* viewModelScope.launch { uiAction.filterIsInstance() - .debounce(DEBOUNCE_TIMEOUT_MS) + .throttleFirst(THROTTLE_TIMEOUT) .collect { action -> try { when (action) { @@ -409,7 +411,7 @@ class NotificationsViewModel @Inject constructor( // Handle StatusAction.* viewModelScope.launch { uiAction.filterIsInstance() - .debounce(DEBOUNCE_TIMEOUT_MS) // avoid double-taps + .throttleFirst(THROTTLE_TIMEOUT) // avoid double-taps .collect { action -> try { when (action) { @@ -517,6 +519,6 @@ class NotificationsViewModel @Inject constructor( companion object { private const val TAG = "NotificationsViewModel" - private const val DEBOUNCE_TIMEOUT_MS = 500L + private val THROTTLE_TIMEOUT = 500.milliseconds } } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/FlowExtensions.kt b/app/src/main/java/com/keylesspalace/tusky/util/FlowExtensions.kt new file mode 100644 index 00000000..7fcf7735 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/util/FlowExtensions.kt @@ -0,0 +1,69 @@ +/* + * Copyright 2023 Tusky Contributors + * + * This file is a part of Tusky. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Tusky; if not, + * see . + */ + +package com.keylesspalace.tusky.util + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow +import kotlin.time.Duration +import kotlin.time.ExperimentalTime +import kotlin.time.TimeMark +import kotlin.time.TimeSource + +/** + * Returns a flow that mirrors the original flow, but filters out values that occur within + * [timeout] of the previously emitted value. The first value is always emitted. + * + * Example: + * + * ```kotlin + * flow { + * emit(1) + * delay(90.milliseconds) + * emit(2) + * delay(90.milliseconds) + * emit(3) + * delay(1010.milliseconds) + * emit(4) + * delay(1010.milliseconds) + * emit(5) + * }.throttleFirst(1000.milliseconds) + * ``` + * + * produces the following emissions. + * + * ```text + * 1, 4, 5 + * ``` + * + * @see kotlinx.coroutines.flow.debounce(Duration) + * @param timeout Emissions within this duration of the last emission are filtered + * @param timeSource Used to measure elapsed time. Normally only overridden in tests + */ +@OptIn(ExperimentalTime::class) +fun Flow.throttleFirst( + timeout: Duration, + timeSource: TimeSource = TimeSource.Monotonic +) = flow { + var marker: TimeMark? = null + collect { + if (marker == null || marker!!.elapsedNow() >= timeout) { + emit(it) + marker = timeSource.markNow() + } + } +} diff --git a/app/src/test/java/com/keylesspalace/tusky/util/FlowExtensionsTest.kt b/app/src/test/java/com/keylesspalace/tusky/util/FlowExtensionsTest.kt new file mode 100644 index 00000000..5cbb231b --- /dev/null +++ b/app/src/test/java/com/keylesspalace/tusky/util/FlowExtensionsTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2023 Tusky Contributors + * + * This file is a part of Tusky. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Tusky; if not, + * see . + */ + +package com.keylesspalace.tusky.util + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.ExperimentalTime + +@OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class) +class FlowExtensionsTest { + @Test + fun `throttleFirst throttles first`() = runTest { + flow { + emit(1) // t = 0, emitted + delay(90.milliseconds) + emit(2) // throttled, t = 90 + delay(90.milliseconds) + emit(3) // throttled, t == 180 + delay(1010.milliseconds) + emit(4) // t = 1190, emitted + delay(1010.milliseconds) + emit(5) // t = 2200, emitted + } + .throttleFirst(1000.milliseconds, timeSource = testScheduler.timeSource) + .test { + advanceUntilIdle() + assertThat(awaitItem()).isEqualTo(1) + assertThat(awaitItem()).isEqualTo(4) + assertThat(awaitItem()).isEqualTo(5) + awaitComplete() + } + } +}