diff --git a/changelog.d/928.bugfix b/changelog.d/928.bugfix new file mode 100644 index 0000000000..98a4cd34e0 --- /dev/null +++ b/changelog.d/928.bugfix @@ -0,0 +1 @@ +Make sure Snackbars are only displayed once. diff --git a/libraries/designsystem/build.gradle.kts b/libraries/designsystem/build.gradle.kts index 35b7a65cb4..f6eacf8746 100644 --- a/libraries/designsystem/build.gradle.kts +++ b/libraries/designsystem/build.gradle.kts @@ -43,5 +43,11 @@ android { ksp(libs.showkase.processor) kspTest(libs.showkase.processor) + + testImplementation(libs.test.junit) + testImplementation(libs.coroutines.test) + testImplementation(libs.molecule.runtime) + testImplementation(libs.test.truth) + testImplementation(libs.test.turbine) } } diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/Snackbar.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/Snackbar.kt index c689d7e547..9458bf748a 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/Snackbar.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/Snackbar.kt @@ -34,33 +34,40 @@ import androidx.compose.ui.unit.dp import io.element.android.libraries.designsystem.components.button.ButtonVisuals import io.element.android.libraries.designsystem.theme.components.IconSource import io.element.android.libraries.designsystem.theme.components.Snackbar +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.update +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.isActive -import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock +import java.util.concurrent.atomic.AtomicBoolean /** * A global dispatcher of [SnackbarMessage] to be displayed in [Snackbar] via a [SnackbarHostState]. */ class SnackbarDispatcher { - private val mutex = Mutex() - - private val _snackbarMessage = MutableStateFlow(null) - val snackbarMessage: Flow = _snackbarMessage.asStateFlow() - - suspend fun post(message: SnackbarMessage) { - mutex.withLock { - _snackbarMessage.update { message } + private val queueMutex = Mutex() + private val snackBarMessageQueue = ArrayDeque() + val snackbarMessage: Flow = flow { + while (currentCoroutineContext().isActive) { + queueMutex.lock() + emit(snackBarMessageQueue.firstOrNull()) } } - suspend fun clear() { - mutex.withLock { - _snackbarMessage.update { null } + suspend fun post(message: SnackbarMessage) { + if (snackBarMessageQueue.isEmpty()) { + snackBarMessageQueue.add(message) + if (queueMutex.isLocked) queueMutex.unlock() + } else { + snackBarMessageQueue.add(message) + } + } + + fun clear() { + if (snackBarMessageQueue.isNotEmpty()) { + snackBarMessageQueue.removeFirstOrNull() + if (queueMutex.isLocked) queueMutex.unlock() } } } @@ -87,31 +94,51 @@ fun SnackbarHost(hostState: SnackbarHostState, modifier: Modifier = Modifier) { } } +/** + * Helper method to display a [SnackbarMessage] in a [SnackbarHostState] handling cancellations. + */ @Composable fun rememberSnackbarHostState(snackbarMessage: SnackbarMessage?): SnackbarHostState { val snackbarHostState = remember { SnackbarHostState() } val snackbarMessageText = snackbarMessage?.let { stringResource(id = snackbarMessage.messageResId) - } + } ?: return snackbarHostState + val dispatcher = LocalSnackbarDispatcher.current - LaunchedEffect(snackbarMessage) { - if (snackbarMessageText == null) return@LaunchedEffect - launch { - snackbarHostState.showSnackbar( - message = snackbarMessageText, - duration = snackbarMessage.duration, - ) - if (isActive) { + LaunchedEffect(snackbarMessageText) { + // If the message wasn't already displayed, do it now, and mark it as displayed + // This will prevent the message from appearing in any other active SnackbarHosts + if (snackbarMessage.isDisplayed.getAndSet(true) == false) { + try { + snackbarHostState.showSnackbar( + message = snackbarMessageText, + duration = snackbarMessage.duration, + ) + // The snackbar item was displayed and dismissed, clear its message dispatcher.clear() + } catch (e: CancellationException) { + // The snackbar was being displayed when the coroutine was cancelled, + // so we need to clear its message + dispatcher.clear() + throw e } } } return snackbarHostState } +/** + * A message to be displayed in a [Snackbar]. + * @param messageResId The message to be displayed. + * @param duration The duration of the message. The default value is [SnackbarDuration.Short]. + * @param actionResId The action text to be displayed. The default value is `null`. + * @param isDisplayed Used to track if the current message is already displayed or not. + * @param action The action to be performed when the action is clicked. + */ data class SnackbarMessage( @StringRes val messageResId: Int, val duration: SnackbarDuration = SnackbarDuration.Short, @StringRes val actionResId: Int? = null, + val isDisplayed: AtomicBoolean = AtomicBoolean(false), val action: () -> Unit = {}, ) diff --git a/libraries/designsystem/src/test/kotlin/io/element/android/libraries/designsystem/utils/SnackbarDispatcherTests.kt b/libraries/designsystem/src/test/kotlin/io/element/android/libraries/designsystem/utils/SnackbarDispatcherTests.kt new file mode 100644 index 0000000000..3eb644d800 --- /dev/null +++ b/libraries/designsystem/src/test/kotlin/io/element/android/libraries/designsystem/utils/SnackbarDispatcherTests.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.libraries.designsystem.utils + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class SnackbarDispatcherTests { + + @Test + fun `given an empty queue the flow emits a null item`() = runTest { + val snackbarDispatcher = SnackbarDispatcher() + snackbarDispatcher.snackbarMessage.test { + assertThat(awaitItem()).isNull() + } + } + + @Test + fun `given an empty queue calling clear does nothing`() = runTest { + val snackbarDispatcher = SnackbarDispatcher() + snackbarDispatcher.snackbarMessage.test { + assertThat(awaitItem()).isNull() + snackbarDispatcher.clear() + expectNoEvents() + } + } + + @Test + fun `given a non-empty queue the flow emits an item`() = runTest { + val snackbarDispatcher = SnackbarDispatcher() + snackbarDispatcher.snackbarMessage.test { + snackbarDispatcher.post(SnackbarMessage(0)) + val result = expectMostRecentItem() + assertThat(result).isNotNull() + } + } + + @Test + fun `given a call to clear, the current message is cleared`() = runTest { + val snackbarDispatcher = SnackbarDispatcher() + snackbarDispatcher.snackbarMessage.test { + snackbarDispatcher.post(SnackbarMessage(0)) + val item = expectMostRecentItem() + assertThat(item).isNotNull() + snackbarDispatcher.clear() + assertThat(awaitItem()).isNull() + } + } + + @Test + fun `given 2 message emissions, the next message is displayed only after a call to clear`() = runTest { + val snackbarDispatcher = SnackbarDispatcher() + snackbarDispatcher.snackbarMessage.test { + val messageA = SnackbarMessage(0) + val messageB = SnackbarMessage(1) + + // Send message A - it is the most recent item + snackbarDispatcher.post(messageA) + assertThat(expectMostRecentItem()).isEqualTo(messageA) + + // Send message B - message A is still the most recent item + snackbarDispatcher.post(messageB) + expectNoEvents() + + // Clear the last message - message B is now the most recent item + snackbarDispatcher.clear() + assertThat(expectMostRecentItem()).isEqualTo(messageB) + + // Clear again - the queue is empty + snackbarDispatcher.clear() + assertThat(awaitItem()).isNull() + } + } + +}