From 359807ccf5e91af217d90181c62e5b5a6d704fd4 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Tue, 22 Sep 2026 16:47:20 -0300 Subject: [PATCH] Add remove pack protection dialog. --- .../conversation/v2/ConversationFragment.kt | 18 +++------- .../mediakeyboard/demo/main/MainScreen.kt | 2 +- .../mediakeyboard/MediaKeyboardAction.kt | 4 +-- .../screens/sticker/StickerPageScreen.kt | 28 +++++++++++++++ .../sticker/StickerPageScreenEvents.kt | 2 ++ .../screens/sticker/StickerPageState.kt | 13 ++++++- .../screens/sticker/StickerPageViewModel.kt | 15 +++++++- .../src/main/res/values/strings.xml | 4 +++ .../sticker/StickerPageViewModelTest.kt | 34 +++++++++++++++++++ 9 files changed, 102 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt index 7d86c1d970..b6b79be896 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt @@ -734,19 +734,11 @@ class ConversationFragment : ) } - is MediaKeyboardAction.RemoveStickerPackClicked -> { - container.onHostWindowShown() - MaterialAlertDialogBuilder(requireContext()) - .setTitle(resources.getQuantityString(R.plurals.StickerManagement_delete_n_packs_confirmation, 1, 1)) - .setMessage(resources.getQuantityString(R.plurals.StickerManagement_delete_n_packs_confirmation_body, 1, 1)) - .setPositiveButton(R.string.StickerManagement_menu_remove_pack) { _, _ -> - viewLifecycleOwner.lifecycleScope.launch { - StickerManagementRepository.uninstallStickerPacks(mapOf(StickerPackId(action.packId) to StickerPackKey(action.packKey))) - } - } - .setNegativeButton(android.R.string.cancel, null) - .setOnDismissListener { container.onHostWindowHidden() } - .show() + // The keyboard has already confirmed this with the user. + is MediaKeyboardAction.RemoveStickerPackConfirmed -> { + viewLifecycleOwner.lifecycleScope.launch { + StickerManagementRepository.uninstallStickerPacks(mapOf(StickerPackId(action.packId) to StickerPackKey(action.packKey))) + } } // A screen of its own rather than a window over this one, and picking a gif carries on into diff --git a/demo/media-keyboard/src/main/java/org/signal/mediakeyboard/demo/main/MainScreen.kt b/demo/media-keyboard/src/main/java/org/signal/mediakeyboard/demo/main/MainScreen.kt index f4f0d00f29..08fda4bf9e 100644 --- a/demo/media-keyboard/src/main/java/org/signal/mediakeyboard/demo/main/MainScreen.kt +++ b/demo/media-keyboard/src/main/java/org/signal/mediakeyboard/demo/main/MainScreen.kt @@ -74,7 +74,7 @@ fun MainScreen( MediaKeyboardAction.GifSearchClicked, is MediaKeyboardAction.ViewStickerPackClicked, is MediaKeyboardAction.SendStickerPackClicked, - is MediaKeyboardAction.RemoveStickerPackClicked -> Unit + is MediaKeyboardAction.RemoveStickerPackConfirmed -> Unit } } } diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/MediaKeyboardAction.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/MediaKeyboardAction.kt index cf6148663b..ebd47fdee1 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/MediaKeyboardAction.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/MediaKeyboardAction.kt @@ -67,10 +67,10 @@ sealed interface MediaKeyboardAction { data class SendStickerPackClicked(val packId: String, val packKey: String) : MediaKeyboardAction /** - * Uninstall a sticker pack, confirming with the user first. + * Uninstall a sticker pack. The keyboard has already confirmed it with the user. * * @param packId The pack to remove. * @param packKey Its key, which identifies the pack alongside [packId]. */ - data class RemoveStickerPackClicked(val packId: String, val packKey: String) : MediaKeyboardAction + data class RemoveStickerPackConfirmed(val packId: String, val packKey: String) : MediaKeyboardAction } diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt index 27384909f3..151c2beb19 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue @@ -42,9 +43,11 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.PreviewWrapper import androidx.compose.ui.unit.dp import org.signal.core.ui.compose.DayNightPreviews +import org.signal.core.ui.compose.Dialogs import org.signal.core.ui.compose.DropdownMenus import org.signal.core.ui.compose.SignalIcons import org.signal.core.ui.compose.SignalPreviewWrapper +import org.signal.core.ui.compose.keyboard.LocalKeyboardSheetController import org.signal.core.ui.compose.theme.SignalTheme import org.signal.glide.compose.GlideImage import org.signal.mediakeyboard.R @@ -77,6 +80,31 @@ fun StickerPageScreen( onSearchFieldRevealedChange = onSearchFieldRevealedChange ) } + + if (state.confirmRemovePack != null) { + ConfirmRemovePackDialog(onEvent = onEvent) + } +} + +@Composable +private fun ConfirmRemovePackDialog(onEvent: (StickerPageScreenEvents) -> Unit) { + val host = LocalKeyboardSheetController.current + + // A window of our own in front of the sheet, so the sheet stays put rather than giving way to it. + DisposableEffect(Unit) { + host.onHostWindowShown() + onDispose { host.onHostWindowHidden() } + } + + Dialogs.SimpleAlertDialog( + title = stringResource(R.string.MediaKeyboard__remove_sticker_pack_question), + body = stringResource(R.string.MediaKeyboard__this_will_remove_the_sticker_pack), + confirm = stringResource(R.string.MediaKeyboard__remove_pack), + dismiss = stringResource(android.R.string.cancel), + onConfirm = { onEvent(StickerPageScreenEvents.RemoveStickerPackConfirmed) }, + onDeny = { onEvent(StickerPageScreenEvents.RemoveStickerPackCanceled) }, + onDismissRequest = { onEvent(StickerPageScreenEvents.RemoveStickerPackCanceled) } + ) } @Composable diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreenEvents.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreenEvents.kt index ca351dc062..138d372d0e 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreenEvents.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageScreenEvents.kt @@ -19,5 +19,7 @@ sealed interface StickerPageScreenEvents { data class ViewStickerPackClicked(val packId: String, val packKey: String) : StickerPageScreenEvents data class SendStickerPackClicked(val packId: String, val packKey: String) : StickerPageScreenEvents data class RemoveStickerPackClicked(val packId: String, val packKey: String) : StickerPageScreenEvents + data object RemoveStickerPackConfirmed : StickerPageScreenEvents + data object RemoveStickerPackCanceled : StickerPageScreenEvents data object ClearRecentStickersClicked : StickerPageScreenEvents } diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageState.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageState.kt index fef00e337c..a6bcc3a37f 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageState.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageState.kt @@ -11,5 +11,16 @@ data class StickerPageState( val packs: List = emptyList(), val selectedPackId: String? = null, val scrollTargetPackId: String? = null, - val allowAnimation: Boolean = true + val allowAnimation: Boolean = true, + val confirmRemovePack: ConfirmRemovePack? = null +) + +/** + * The pack the user has asked to remove, held until they confirm or dismiss the prompt. + * + * @param packKey The pack's key, which identifies it alongside [packId] to whoever uninstalls it. + */ +data class ConfirmRemovePack( + val packId: String, + val packKey: String ) diff --git a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt index 9b05d118ae..704e78b768 100644 --- a/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt +++ b/feature/media-keyboard/src/main/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModel.kt @@ -89,7 +89,20 @@ class StickerPageViewModel( } is StickerPageScreenEvents.RemoveStickerPackClicked -> { - onAction(MediaKeyboardAction.RemoveStickerPackClicked(event.packId, event.packKey)) + stateEmitter(state.copy(confirmRemovePack = ConfirmRemovePack(event.packId, event.packKey))) + } + + is StickerPageScreenEvents.RemoveStickerPackConfirmed -> { + val pack = state.confirmRemovePack + stateEmitter(state.copy(confirmRemovePack = null)) + + if (pack != null) { + onAction(MediaKeyboardAction.RemoveStickerPackConfirmed(pack.packId, pack.packKey)) + } + } + + is StickerPageScreenEvents.RemoveStickerPackCanceled -> { + stateEmitter(state.copy(confirmRemovePack = null)) } is StickerPageScreenEvents.ClearRecentStickersClicked -> { diff --git a/feature/media-keyboard/src/main/res/values/strings.xml b/feature/media-keyboard/src/main/res/values/strings.xml index 7df1cac318..614d079a61 100644 --- a/feature/media-keyboard/src/main/res/values/strings.xml +++ b/feature/media-keyboard/src/main/res/values/strings.xml @@ -63,6 +63,10 @@ More options Remove + + Remove sticker pack? + + This will remove the sticker pack. You can\'t add it again without a link or sticker message. Clear recents diff --git a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt index 2433dd89b3..2ff7fcd247 100644 --- a/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt +++ b/feature/media-keyboard/src/test/java/org/signal/mediakeyboard/screens/sticker/StickerPageViewModelTest.kt @@ -6,7 +6,9 @@ package org.signal.mediakeyboard.screens.sticker import assertk.assertThat +import assertk.assertions.isEmpty import assertk.assertions.isEqualTo +import assertk.assertions.isNull import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -109,6 +111,38 @@ class StickerPageViewModelTest { assertThat(actions).isEqualTo(listOf(MediaKeyboardAction.ViewStickerPackClicked("pack-1", "pack-1-key"))) } + @Test + fun `remove pack clicked - prompts instead of removing`() { + val viewModel = createViewModel() + viewModel.onEvent(StickerPageScreenEvents.RemoveStickerPackClicked("pack-1", "pack-1-key")) + testDispatcher.scheduler.advanceUntilIdle() + + assertThat(viewModel.state.value.confirmRemovePack).isEqualTo(ConfirmRemovePack("pack-1", "pack-1-key")) + assertThat(actions).isEmpty() + } + + @Test + fun `remove pack confirmed - dismisses the prompt and hands the pack's ids to the host`() { + val viewModel = createViewModel() + viewModel.onEvent(StickerPageScreenEvents.RemoveStickerPackClicked("pack-1", "pack-1-key")) + viewModel.onEvent(StickerPageScreenEvents.RemoveStickerPackConfirmed) + testDispatcher.scheduler.advanceUntilIdle() + + assertThat(viewModel.state.value.confirmRemovePack).isNull() + assertThat(actions).isEqualTo(listOf(MediaKeyboardAction.RemoveStickerPackConfirmed("pack-1", "pack-1-key"))) + } + + @Test + fun `remove pack canceled - dismisses the prompt without removing`() { + val viewModel = createViewModel() + viewModel.onEvent(StickerPageScreenEvents.RemoveStickerPackClicked("pack-1", "pack-1-key")) + viewModel.onEvent(StickerPageScreenEvents.RemoveStickerPackCanceled) + testDispatcher.scheduler.advanceUntilIdle() + + assertThat(viewModel.state.value.confirmRemovePack).isNull() + assertThat(actions).isEmpty() + } + @Test fun `search clicked - hands off to the host`() { val viewModel = createViewModel()