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.
This commit is contained in:
Benoit Marty
2025-05-13 14:12:19 +02:00
committed by GitHub
parent 3d64afa937
commit b322a4bae5
4 changed files with 84 additions and 27 deletions

View File

@@ -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)

View File

@@ -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<RoomListEvents>()
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<RoomListEvents>()

View File

@@ -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)
}
}

View File

@@ -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()
}
}