From 004abd16dc90073587ef5ad56fa1a541f40b432f Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 13 Sep 2023 21:03:52 +0100 Subject: [PATCH] Address PR review comments. - use util startNotificationSettingsIntent. - add documentation. - use remember with userPushStoreFactory for recomposition. --- .../impl/notifications/NotificationSettingsNode.kt | 2 +- .../notifications/NotificationSettingsPresenter.kt | 2 +- .../impl/notifications/NotificationSettingsView.kt | 12 ++++++------ .../edit/DefaultNotificationSettingOption.kt | 2 +- .../edit/EditDefaultNotificationSettingPresenter.kt | 1 + .../edit/EditDefaultNotificationSettingView.kt | 5 +++++ .../libraries/androidutils/system/SystemUtils.kt | 9 +++++++-- 7 files changed, 22 insertions(+), 11 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsNode.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsNode.kt index 14ffe2868f..0e3861c5ec 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsNode.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsNode.kt @@ -49,7 +49,7 @@ class NotificationSettingsNode @AssistedInject constructor( val state = presenter.present() NotificationSettingsView( state = state, - onOpenEditDefault = { openEditDefault(it) }, + onOpenEditDefault = { openEditDefault(isOneToOne = it) }, onBackPressed = ::navigateUp, modifier = modifier, ) 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 923415b339..697d5887f0 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 @@ -46,7 +46,7 @@ class NotificationSettingsPresenter @Inject constructor( ) : Presenter { @Composable override fun present(): NotificationSettingsState { - val userPushStore = userPushStoreFactory.create(matrixClient.sessionId) + val userPushStore = remember { userPushStoreFactory.create(matrixClient.sessionId) } val systemNotificationsEnabled: MutableState = remember { mutableStateOf(systemNotificationsEnabledProvider.notificationsEnabled()) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt index 9bb0f1cd66..b5aa7db9f6 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsView.kt @@ -39,6 +39,7 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle +import io.element.android.libraries.androidutils.system.startNotificationSettingsIntent import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog import io.element.android.libraries.designsystem.components.preferences.PreferenceCategory import io.element.android.libraries.designsystem.components.preferences.PreferenceSwitch @@ -55,10 +56,13 @@ import io.element.android.libraries.matrix.api.room.RoomNotificationMode import io.element.android.libraries.theme.ElementTheme import io.element.android.libraries.ui.strings.CommonStrings +/** + * A view that allows a user edit their global notification settings. + */ @Composable fun NotificationSettingsView( state: NotificationSettingsState, - onOpenEditDefault: (Boolean) -> Unit, + onOpenEditDefault: (isOneToOne: Boolean) -> Unit, onBackPressed: () -> Unit, modifier: Modifier = Modifier, ) { @@ -113,11 +117,7 @@ private fun NotificationSettingsContentView( subtitle = stringResource(id = CommonStrings.screen_notification_settings_system_notifications_action_required, stringResource(id = CommonStrings.screen_notification_settings_system_notifications_action_required_content_link)), onClick = { - val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS) - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - val uri: Uri = Uri.fromParts("package", context.packageName, null) - intent.data = uri - context.startActivity(intent) + context.startNotificationSettingsIntent() } ) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/DefaultNotificationSettingOption.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/DefaultNotificationSettingOption.kt index 7ece6cc737..e60b2bc8dc 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/DefaultNotificationSettingOption.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/DefaultNotificationSettingOption.kt @@ -80,7 +80,7 @@ fun DefaultNotificationSettingOption( } @DayNightPreviews @Composable -internal fun DefaultNotificationSettingOptionLightPreview() = ElementPreview { ContentToPreview() } +internal fun DefaultNotificationSettingOptionPreview() = ElementPreview { ContentToPreview() } @Composable private fun ContentToPreview() { 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 cc15e5bdd8..764b37c52d 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 @@ -84,6 +84,7 @@ class EditDefaultNotificationSettingPresenter @AssistedInject constructor( } private fun CoroutineScope.setDefaultNotificationMode(mode: RoomNotificationMode) = launch { + // 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) notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, mode = mode, isOneToOne = isOneToOne) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingView.kt index d67b789cc9..4cc95af71f 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/edit/EditDefaultNotificationSettingView.kt @@ -26,6 +26,10 @@ import io.element.android.libraries.designsystem.components.preferences.Preferen import io.element.android.libraries.matrix.api.room.RoomNotificationMode import io.element.android.libraries.ui.strings.CommonStrings +/** + * A view that allows a user to edit the default notification setting for rooms. This can be set separately + * for one-to-one and group rooms, indicated by [EditDefaultNotificationSettingState.isOneToOne]. + */ @Composable fun EditDefaultNotificationSettingView( state: EditDefaultNotificationSettingState, @@ -44,6 +48,7 @@ fun EditDefaultNotificationSettingView( title = stringResource(id = title) ) { + // Only ALL_MESSAGES and MENTIONS_AND_KEYWORDS_ONLY are valid global defaults. val validModes = listOf(RoomNotificationMode.ALL_MESSAGES, RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY) val categoryTitle = if(state.isOneToOne) { diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/system/SystemUtils.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/system/SystemUtils.kt index a9a17dcceb..1aee986d32 100644 --- a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/system/SystemUtils.kt +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/system/SystemUtils.kt @@ -122,7 +122,7 @@ fun Context.copyToClipboard( * Shows notification settings for the current app. * In android O will directly opens the notification settings, in lower version it will show the App settings */ -fun Context.startNotificationSettingsIntent(activityResultLauncher: ActivityResultLauncher) { +fun Context.startNotificationSettingsIntent(activityResultLauncher: ActivityResultLauncher? = null) { val intent = Intent() if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { intent.action = Settings.ACTION_APP_NOTIFICATION_SETTINGS @@ -132,7 +132,12 @@ fun Context.startNotificationSettingsIntent(activityResultLauncher: ActivityResu intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) intent.data = Uri.fromParts("package", packageName, null) } - activityResultLauncher.launch(intent) + + if (activityResultLauncher != null) { + activityResultLauncher.launch(intent) + } else { + startActivity(intent) + } } fun Context.openAppSettingsPage(