diff --git a/changelog.d/1647.bugfix b/changelog.d/1647.bugfix new file mode 100644 index 0000000000..31fdd2e9f9 --- /dev/null +++ b/changelog.d/1647.bugfix @@ -0,0 +1 @@ +Improve UX on notification setting changes. diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt index 5cfc14545e..d61a05a935 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt @@ -29,7 +29,7 @@ import androidx.compose.runtime.setValue import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.architecture.runCatchingUpdatingState +import io.element.android.libraries.architecture.runUpdatingStateNoSuccess import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService import io.element.android.libraries.matrix.api.room.RoomNotificationMode @@ -74,7 +74,7 @@ class NotificationSettingsPresenter @Inject constructor( LaunchedEffect(Unit) { fetchSettings(matrixSettings) - observeNotificationSettings(matrixSettings) + observeNotificationSettings(matrixSettings, changeNotificationSettingAction) } // List of PushProvider -> Distributor @@ -172,11 +172,15 @@ class NotificationSettingsPresenter @Inject constructor( } @OptIn(FlowPreview::class) - private fun CoroutineScope.observeNotificationSettings(target: MutableState) { + private fun CoroutineScope.observeNotificationSettings( + target: MutableState, + changeNotificationSettingAction: MutableState>, + ) { notificationSettingsService.notificationSettingsChangeFlow .debounce(0.5.seconds) .onEach { fetchSettings(target) + changeNotificationSettingAction.value = AsyncAction.Uninitialized } .launchIn(this) } @@ -238,21 +242,21 @@ class NotificationSettingsPresenter @Inject constructor( } private fun CoroutineScope.setAtRoomNotificationsEnabled(enabled: Boolean, action: MutableState>) = launch { - suspend { - notificationSettingsService.setRoomMentionEnabled(enabled).getOrThrow() - }.runCatchingUpdatingState(action) + action.runUpdatingStateNoSuccess { + notificationSettingsService.setRoomMentionEnabled(enabled) + } } private fun CoroutineScope.setCallNotificationsEnabled(enabled: Boolean, action: MutableState>) = launch { - suspend { - notificationSettingsService.setCallEnabled(enabled).getOrThrow() - }.runCatchingUpdatingState(action) + action.runUpdatingStateNoSuccess { + notificationSettingsService.setCallEnabled(enabled) + } } private fun CoroutineScope.setInviteForMeNotificationsEnabled(enabled: Boolean, action: MutableState>) = launch { - suspend { - notificationSettingsService.setInviteForMeEnabled(enabled).getOrThrow() - }.runCatchingUpdatingState(action) + action.runUpdatingStateNoSuccess { + notificationSettingsService.setInviteForMeEnabled(enabled) + } } private fun CoroutineScope.setNotificationsEnabled(userPushStore: UserPushStore, enabled: Boolean) = launch { diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingPresenter.kt index 04accef4ea..3f1f62aafc 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingPresenter.kt @@ -29,7 +29,7 @@ import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.architecture.runCatchingUpdatingState +import io.element.android.libraries.architecture.runUpdatingStateNoSuccess import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService import io.element.android.libraries.matrix.api.room.RoomNotificationMode @@ -73,7 +73,7 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor( val localCoroutineScope = rememberCoroutineScope() LaunchedEffect(Unit) { fetchSettings(mode) - observeNotificationSettings(mode) + observeNotificationSettings(mode, changeNotificationSettingAction) observeRoomSummaries(roomsWithUserDefinedMode) displayMentionsOnlyDisclaimer = !notificationSettingsService.canHomeServerPushEncryptedEventsToDevice().getOrDefault(true) } @@ -102,11 +102,15 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor( } @OptIn(FlowPreview::class) - private fun CoroutineScope.observeNotificationSettings(mode: MutableState) { + private fun CoroutineScope.observeNotificationSettings( + mode: MutableState, + changeNotificationSettingAction: MutableState>, + ) { notificationSettingsService.notificationSettingsChangeFlow .debounce(0.5.seconds) .onEach { fetchSettings(mode) + changeNotificationSettingAction.value = AsyncAction.Uninitialized } .launchIn(this) } @@ -139,10 +143,12 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor( } private fun CoroutineScope.setDefaultNotificationMode(mode: RoomNotificationMode, action: MutableState>) = launch { - suspend { + action.runUpdatingStateNoSuccess { // On modern clients, we don't have different settings for encrypted and non-encrypted rooms (Legacy clients did). - notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, mode = mode, isOneToOne = isOneToOne).getOrThrow() - notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, mode = mode, isOneToOne = isOneToOne).getOrThrow() - }.runCatchingUpdatingState(action) + notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, mode = mode, isOneToOne = isOneToOne) + .map { + notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, mode = mode, isOneToOne = isOneToOne) + } + } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsEvents.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsEvents.kt index 0ce6e56176..e0370b58c8 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsEvents.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsEvents.kt @@ -19,7 +19,7 @@ package io.element.android.features.roomdetails.impl.notificationsettings import io.element.android.libraries.matrix.api.room.RoomNotificationMode sealed interface RoomNotificationSettingsEvents { - data class RoomNotificationModeChanged(val mode: RoomNotificationMode) : RoomNotificationSettingsEvents + data class ChangeRoomNotificationMode(val mode: RoomNotificationMode) : RoomNotificationSettingsEvents data class SetNotificationMode(val isDefault: Boolean) : RoomNotificationSettingsEvents data object DeleteCustomNotification : RoomNotificationSettingsEvents data object ClearSetNotificationError : RoomNotificationSettingsEvents diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsPresenter.kt index 84831fe92b..170c0fabba 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsPresenter.kt @@ -32,6 +32,7 @@ import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runCatchingUpdatingState +import io.element.android.libraries.core.coroutine.suspendWithMinimumDuration import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomNotificationMode @@ -95,7 +96,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor( fun handleEvents(event: RoomNotificationSettingsEvents) { when (event) { - is RoomNotificationSettingsEvents.RoomNotificationModeChanged -> { + is RoomNotificationSettingsEvents.ChangeRoomNotificationMode -> { localCoroutineScope.setRoomNotificationMode(event.mode, pendingRoomNotificationMode, pendingSetDefault, setNotificationSettingAction) } is RoomNotificationSettingsEvents.SetNotificationMode -> { @@ -171,7 +172,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor( pendingDefaultState: MutableState, action: MutableState> ) = launch { - suspend { + suspendWithMinimumDuration { pendingModeState.value = mode pendingDefaultState.value = false val result = notificationSettingsService.setRoomNotificationMode(room.roomId, mode) @@ -187,7 +188,7 @@ class RoomNotificationSettingsPresenter @AssistedInject constructor( action: MutableState>, pendingDefaultState: MutableState ) = launch { - suspend { + suspendWithMinimumDuration { pendingDefaultState.value = true val result = notificationSettingsService.restoreDefaultRoomNotificationMode(room.roomId) if (result.isFailure) { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsView.kt index 2022c87de0..4ba812a068 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/RoomNotificationSettingsView.kt @@ -149,7 +149,7 @@ private fun RoomSpecificNotificationSettingsView( enabled = !state.displayIsDefault.orTrue(), displayMentionsOnlyDisclaimer = state.displayMentionsOnlyDisclaimer, onSelectOption = { - state.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(it.mode)) + state.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(it.mode)) }, ) } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/UserDefinedRoomNotificationSettingsView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/UserDefinedRoomNotificationSettingsView.kt index 81f0c5f9d9..f9f07f1303 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/UserDefinedRoomNotificationSettingsView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/notificationsettings/UserDefinedRoomNotificationSettingsView.kt @@ -68,7 +68,7 @@ fun UserDefinedRoomNotificationSettingsView( enabled = !state.displayIsDefault.orTrue(), displayMentionsOnlyDisclaimer = state.displayMentionsOnlyDisclaimer, onSelectOption = { - state.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(it.mode)) + state.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(it.mode)) }, ) } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/notificationsettings/RoomNotificationSettingsPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/notificationsettings/RoomNotificationSettingsPresenterTest.kt index 54e23a3995..e1fc3bddf9 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/notificationsettings/RoomNotificationSettingsPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/notificationsettings/RoomNotificationSettingsPresenterTest.kt @@ -55,7 +55,7 @@ class RoomNotificationSettingsPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - awaitItem().eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) + awaitItem().eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) val updatedState = consumeItemsUntilPredicate { it.roomNotificationSettings.dataOrNull()?.mode == RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY }.last() @@ -129,7 +129,7 @@ class RoomNotificationSettingsPresenterTest { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) + initialState.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) initialState.eventSink(RoomNotificationSettingsEvents.SetNotificationMode(true)) val defaultState = consumeItemsUntilPredicate { it.roomNotificationSettings.dataOrNull()?.mode == RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY @@ -148,7 +148,7 @@ class RoomNotificationSettingsPresenterTest { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(RoomNotificationSettingsEvents.RoomNotificationModeChanged(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) + initialState.eventSink(RoomNotificationSettingsEvents.ChangeRoomNotificationMode(RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY)) initialState.eventSink(RoomNotificationSettingsEvents.SetNotificationMode(true)) val failedState = consumeItemsUntilPredicate { it.restoreDefaultAction.isFailure() diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt index 9545f1ab68..3f5f4ac8d0 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt @@ -125,6 +125,24 @@ suspend inline fun MutableState>.runUpdatingState( resultBlock = resultBlock, ) +/** + * Run the given block and update the state accordingly, using only Loading and Failure states. + * It's up to the caller to manage the Success state. + */ +@OptIn(ExperimentalContracts::class) +inline fun MutableState>.runUpdatingStateNoSuccess( + resultBlock: () -> Result, +): Result { + contract { + callsInPlace(resultBlock, InvocationKind.EXACTLY_ONCE) + } + value = AsyncAction.Loading + return resultBlock() + .onFailure { failure -> + value = AsyncAction.Failure(failure) + } +} + /** * Calls the specified [Result]-returning function [resultBlock] * encapsulating its progress and return value into an [AsyncAction] while diff --git a/libraries/core/src/main/kotlin/io/element/android/libraries/core/coroutine/Suspend.kt b/libraries/core/src/main/kotlin/io/element/android/libraries/core/coroutine/Suspend.kt new file mode 100644 index 0000000000..46a52c8443 --- /dev/null +++ b/libraries/core/src/main/kotlin/io/element/android/libraries/core/coroutine/Suspend.kt @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2024 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.core.coroutine + +import kotlinx.coroutines.delay +import kotlin.system.measureTimeMillis + +fun suspendWithMinimumDuration( + minimumDurationMillis: Long = 500, + block: suspend () -> Unit +) = suspend { + val duration = measureTimeMillis { + block() + } + delay(minimumDurationMillis - duration) +}