From b322a4bae55307cd044b1319043ccdf5319098a9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 13 May 2025 14:12:19 +0200 Subject: [PATCH] Disable mutliple click (parallel or serial) on a room (#4683) * Disable mutliple click (parallel or serial) on a room (Fixes #4619) * Rename method from FirstThrottler * Move check to the Compose and add unit test on it. --- .../features/roomlist/impl/RoomListView.kt | 14 +++++-- .../roomlist/impl/RoomListViewTest.kt | 26 +++++++++++++ .../androidutils/throttler/FirstThrottler.kt | 39 ++++++++----------- .../throttler/FirstThrottlerTest.kt | 32 +++++++++++++++ 4 files changed, 84 insertions(+), 27 deletions(-) create mode 100644 libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottlerTest.kt diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt index 016cc4d116..b00796c1fe 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt @@ -17,6 +17,8 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.rememberTopAppBarState import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.res.stringResource @@ -30,6 +32,7 @@ import io.element.android.features.roomlist.impl.components.RoomListMenuAction import io.element.android.features.roomlist.impl.components.RoomListTopBar import io.element.android.features.roomlist.impl.model.RoomListRoomSummary import io.element.android.features.roomlist.impl.search.RoomListSearchView +import io.element.android.libraries.androidutils.throttler.FirstThrottler import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.theme.components.FloatingActionButton @@ -54,6 +57,9 @@ fun RoomListView( modifier: Modifier = Modifier, acceptDeclineInviteView: @Composable () -> Unit, ) { + val coroutineScope = rememberCoroutineScope() + val firstThrottler = remember { FirstThrottler(300, coroutineScope) } + ConnectivityIndicatorContainer( modifier = modifier, isOnline = state.hasNetworkConnection, @@ -83,9 +89,9 @@ fun RoomListView( state = state, onSetUpRecoveryClick = onSetUpRecoveryClick, onConfirmRecoveryKeyClick = onConfirmRecoveryKeyClick, - onRoomClick = onRoomClick, - onOpenSettings = onSettingsClick, - onCreateRoomClick = onCreateRoomClick, + onRoomClick = { if (firstThrottler.canHandle()) onRoomClick(it) }, + onOpenSettings = { if (firstThrottler.canHandle()) onSettingsClick() }, + onCreateRoomClick = { if (firstThrottler.canHandle()) onCreateRoomClick() }, onMenuActionClick = onMenuActionClick, modifier = Modifier.padding(top = topPadding), ) @@ -94,7 +100,7 @@ fun RoomListView( state = state.searchState, eventSink = state.eventSink, hideInvitesAvatars = state.hideInvitesAvatars, - onRoomClick = onRoomClick, + onRoomClick = { if (firstThrottler.canHandle()) onRoomClick(it) }, modifier = Modifier .statusBarsPadding() .padding(top = topPadding) diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListViewTest.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListViewTest.kt index ee4cc0b564..0242591f55 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListViewTest.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListViewTest.kt @@ -5,6 +5,8 @@ * Please see LICENSE files in the repository root for full details. */ +@file:OptIn(ExperimentalCoroutinesApi::class) + package io.element.android.features.roomlist.impl import androidx.activity.ComponentActivity @@ -27,6 +29,7 @@ import io.element.android.tests.testutils.EventsRecorder import io.element.android.tests.testutils.clickOn import io.element.android.tests.testutils.ensureCalledOnce import io.element.android.tests.testutils.ensureCalledOnceWithParam +import kotlinx.coroutines.ExperimentalCoroutinesApi import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule @@ -168,6 +171,29 @@ class RoomListViewTest { eventsRecorder.assertEmpty() } + @Test + fun `clicking on a room twice invokes the expected callback only once`() { + val eventsRecorder = EventsRecorder() + val state = aRoomListState( + eventSink = eventsRecorder, + ) + val room0 = state.contentAsRooms().summaries.first { + it.displayType == RoomSummaryDisplayType.ROOM + } + ensureCalledOnceWithParam(room0.roomId) { callback -> + rule.setRoomListView( + state = state, + onRoomClick = callback, + ) + // Remove automatic initial events + eventsRecorder.clear() + rule.onNodeWithText(room0.lastMessage!!.toString()) + .performClick() + .performClick() + } + eventsRecorder.assertEmpty() + } + @Test fun `long clicking on a room emits the expected Event`() { val eventsRecorder = EventsRecorder() diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottler.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottler.kt index 30919d5228..8f6766cdd6 100644 --- a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottler.kt +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottler.kt @@ -6,36 +6,29 @@ */ package io.element.android.libraries.androidutils.throttler -import android.os.SystemClock +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import java.util.concurrent.atomic.AtomicBoolean /** * Simple ThrottleFirst * See https://raw.githubusercontent.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleFirst.png */ -class FirstThrottler(private val minimumInterval: Long = 800) { - private var lastDate = 0L +class FirstThrottler( + private val minimumInterval: Long = 800, + private val coroutineScope: CoroutineScope, +) { + private val canHandle = AtomicBoolean(true) - sealed interface CanHandleResult { - data object Yes : CanHandleResult - data class No(val shouldWaitMillis: Long) : CanHandleResult - - fun waitMillis(): Long { - return when (this) { - Yes -> 0 - is No -> shouldWaitMillis + fun canHandle(): Boolean { + return canHandle.getAndSet(false).also { result -> + if (result) { + coroutineScope.launch { + delay(minimumInterval) + canHandle.set(true) + } } } } - - fun canHandle(): CanHandleResult { - val now = SystemClock.elapsedRealtime() - val delaySinceLast = now - lastDate - if (delaySinceLast > minimumInterval) { - lastDate = now - return CanHandleResult.Yes - } - - // Too early - return CanHandleResult.No(minimumInterval - delaySinceLast) - } } diff --git a/libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottlerTest.kt b/libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottlerTest.kt new file mode 100644 index 0000000000..f33a3f237a --- /dev/null +++ b/libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/throttler/FirstThrottlerTest.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +@file:OptIn(ExperimentalCoroutinesApi::class) + +package io.element.android.libraries.androidutils.throttler + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class FirstThrottlerTest { + @Test + fun `throttle canHandle returns the expected result`() = runTest { + val throttler = FirstThrottler( + minimumInterval = 300, + coroutineScope = backgroundScope, + ) + assertThat(throttler.canHandle()).isTrue() + assertThat(throttler.canHandle()).isFalse() + advanceTimeBy(200) + assertThat(throttler.canHandle()).isFalse() + advanceTimeBy(110) + assertThat(throttler.canHandle()).isTrue() + } +}