commit ace4251c3dfa59a68bcdec051752e575c39d2a4f parent 0c3d4805efb05ab6a60891d3c57014e0cbcc48f2 Author: mike a. <mavduevskiy@mozilla.com> Date: Tue, 18 Nov 2025 19:12:49 +0000 Bug 1955887 - Part 2: Add an error snackbar in case changing the icon failed r=android-reviewers,android-l10n-reviewers,flod,twhite Differential Revision: https://phabricator.services.mozilla.com/D270985 Diffstat:
10 files changed, 269 insertions(+), 57 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconAction.kt @@ -9,24 +9,57 @@ import mozilla.components.lib.state.Action /** * Actions related to the app icon selection screen. */ -sealed interface AppIconAction : Action { +sealed interface AppIconAction : Action + +/** + * User-initiated actions. + */ +sealed interface UserAction : AppIconAction { /** - * User has selected a new [appIcon] to set as the app icon. This does not mean it is applied. + * The user has clicked on an icon. * * @property appIcon the new selected icon. */ - data class SelectAppIcon(val appIcon: AppIcon) : AppIconAction + data class Selected(val appIcon: AppIcon) : UserAction + + /** + * The user has confirmed applying the new icon on the warning dialog screen. + * + * @property oldIcon the currently used app icon. + * @property newIcon the new app icon to apply. + */ + data class Confirmed(val oldIcon: AppIcon, val newIcon: AppIcon) : UserAction /** - * Reset the selection from the one selected by the user back to the icon currently used by the app. + * The user has pressed the "Dismiss" button on the warning dialog. */ - data object ResetSelection : AppIconAction + data object Dismissed : UserAction +} +/** + * System-initiated actions. + */ +sealed interface SystemAction : AppIconAction { /** - * Set the new icon as the app icon on a system level. + * The warning dialog has been canceled through means other than dismiss button (back gesture, + * tap outside of the dialog area, etc) + */ + data object DialogDismissed : SystemAction + + /** + * The system app icon has been successfully updated. * - * @property newIcon the icon to apply. - * @property currentIcon the current app icon used by the system. + * @property newIcon the new icon. + */ + data class Applied(val newIcon: AppIcon) : SystemAction + + /** + * The app icon update failed (due to an exception thrown by a system call). + */ + data object UpdateFailed : SystemAction + + /** + * The app icon update error snackbar was dismissed. */ - data class ApplyAppIcon(val newIcon: AppIcon, val currentIcon: AppIcon) : AppIconAction + data object SnackbarDismissed : SystemAction } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconMiddleware.kt @@ -24,12 +24,23 @@ class AppIconMiddleware( next(action) when (action) { - is AppIconAction.ApplyAppIcon -> updateAppIcon( - new = action.newIcon, - old = action.currentIcon, - ) + is UserAction.Confirmed -> { + if (updateAppIcon(old = action.newIcon, new = action.oldIcon)) { + context.dispatch(SystemAction.Applied(action.newIcon)) + } else { + context.dispatch(SystemAction.UpdateFailed) + } + } - is AppIconAction.SelectAppIcon, AppIconAction.ResetSelection -> Unit + is SystemAction.Applied, + is SystemAction.DialogDismissed, + is SystemAction.SnackbarDismissed, + is SystemAction.UpdateFailed, + is UserAction.Dismissed, + is UserAction.Selected, + -> { + // no-op + } } } } @@ -37,6 +48,6 @@ class AppIconMiddleware( /** * An interface for applying a new app icon. */ -fun interface AppIconUpdater : (AppIcon, AppIcon) -> Unit { - override fun invoke(new: AppIcon, old: AppIcon) +fun interface AppIconUpdater : (AppIcon, AppIcon) -> Boolean { + override fun invoke(old: AppIcon, new: AppIcon): Boolean } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconReducer.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconReducer.kt @@ -8,7 +8,29 @@ package org.mozilla.fenix.iconpicker * Function for reducing a new app icon state based on the received action. */ internal fun appIconReducer(state: AppIconState, action: AppIconAction) = when (action) { - is AppIconAction.SelectAppIcon -> state.copy(userSelectedAppIcon = action.appIcon) - AppIconAction.ResetSelection -> state.copy(userSelectedAppIcon = null) - is AppIconAction.ApplyAppIcon -> state.copy(currentAppIcon = action.newIcon, userSelectedAppIcon = null) + is UserAction -> state.handleUserAction(action) + is SystemAction -> state.handleSystemAction(action) +} + +private fun AppIconState.handleUserAction(action: UserAction): AppIconState { + return when (action) { + is UserAction.Selected -> this.copy(userSelectedAppIcon = action.appIcon) + is UserAction.Confirmed -> this + is UserAction.Dismissed -> this.copy(userSelectedAppIcon = null) + } +} + +private fun AppIconState.handleSystemAction(action: SystemAction): AppIconState { + return when (action) { + is SystemAction.Applied -> this.copy( + currentAppIcon = action.newIcon, + userSelectedAppIcon = null, + ) + SystemAction.DialogDismissed -> this.copy(userSelectedAppIcon = null) + SystemAction.SnackbarDismissed -> this.copy(snackbarState = AppIconSnackbarState.None) + SystemAction.UpdateFailed -> this.copy( + userSelectedAppIcon = null, + snackbarState = AppIconSnackbarState.ApplyingNewIconError, + ) + } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconState.kt @@ -12,9 +12,26 @@ import mozilla.components.lib.state.State * @property currentAppIcon The icon currently being used by the application. * @property userSelectedAppIcon The icon the user has selected in the picker, if any. * @property groupedIconOptions A map of all available app icons. + * @property snackbarState The current snackbar state. */ data class AppIconState( val currentAppIcon: AppIcon = AppIcon.AppDefault, val userSelectedAppIcon: AppIcon? = null, val groupedIconOptions: Map<IconGroupTitle, List<AppIcon>> = mapOf(), + val snackbarState: AppIconSnackbarState = AppIconSnackbarState.None, ) : State + +/** + * The state of the snackbar to display. + */ +sealed class AppIconSnackbarState { + /** + * There is no snackbar to display. + */ + data object None : AppIconSnackbarState() + + /** + * Display a snackbar of the app icon update failure. + */ + data object ApplyingNewIconError : AppIconSnackbarState() +} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelection.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelection.kt @@ -9,10 +9,13 @@ import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width @@ -24,9 +27,15 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.AlertDialog import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -40,19 +49,24 @@ import androidx.compose.ui.semantics.heading import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import kotlinx.coroutines.launch import mozilla.components.compose.base.annotation.FlexibleWindowLightDarkPreview import mozilla.components.compose.base.button.TextButton +import mozilla.components.compose.base.snackbar.Snackbar +import mozilla.components.compose.base.snackbar.displaySnackbar import mozilla.components.lib.state.ext.observeAsState import org.mozilla.fenix.R import org.mozilla.fenix.compose.button.RadioButton import org.mozilla.fenix.iconpicker.AppIcon -import org.mozilla.fenix.iconpicker.AppIconAction +import org.mozilla.fenix.iconpicker.AppIconSnackbarState import org.mozilla.fenix.iconpicker.AppIconState import org.mozilla.fenix.iconpicker.AppIconStore import org.mozilla.fenix.iconpicker.DefaultAppIconRepository import org.mozilla.fenix.iconpicker.DefaultPackageManagerWrapper import org.mozilla.fenix.iconpicker.IconBackground import org.mozilla.fenix.iconpicker.IconGroupTitle +import org.mozilla.fenix.iconpicker.SystemAction +import org.mozilla.fenix.iconpicker.UserAction import org.mozilla.fenix.theme.FirefoxTheme private val ListItemHeight = 56.dp @@ -75,11 +89,77 @@ fun AppIconSelection( ) { val state by store.observeAsState(store.state) { it } val selectedIcon = state.userSelectedAppIcon ?: state.currentAppIcon + val scope = rememberCoroutineScope() + val snackbarHostState = remember { SnackbarHostState() } + val errorSnackbarMessage = stringResource(R.string.shortcuts_update_error) + LaunchedEffect(state.snackbarState) { + when (state.snackbarState) { + AppIconSnackbarState.None -> return@LaunchedEffect + AppIconSnackbarState.ApplyingNewIconError -> scope.launch { + snackbarHostState.displaySnackbar( + message = errorSnackbarMessage, + onDismissPerformed = { + store.dispatch(SystemAction.SnackbarDismissed) + }, + ) + } + } + } + + Scaffold( + snackbarHost = { + SnackbarHost( + hostState = snackbarHostState, + snackbar = { snackbarData -> + Snackbar(snackbarData = snackbarData) + }, + modifier = Modifier.imePadding(), + ) + }, + contentWindowInsets = WindowInsets(), // empty insets, activity toolbar is handled by the host fragment + ) { paddingValues -> + AppIconList( + paddingValues = paddingValues, + selectedIcon = selectedIcon, + groupedIcons = state.groupedIconOptions, + onIconSelected = { icon -> store.dispatch(UserAction.Selected(icon)) }, + ) + } + + state.userSelectedAppIcon?.let { + RestartWarningDialog( + onConfirmClicked = { + store.dispatch( + UserAction.Confirmed( + oldIcon = state.currentAppIcon, + newIcon = it, + ), + ) + }, + onDismissClicked = { + store.dispatch(UserAction.Dismissed) + }, + onDismissed = { + store.dispatch(SystemAction.DialogDismissed) + }, + ) + } +} + +@Composable +private fun AppIconList( + paddingValues: PaddingValues, + selectedIcon: AppIcon, + groupedIcons: Map<IconGroupTitle, List<AppIcon>>, + onIconSelected: (AppIcon) -> Unit, +) { LazyColumn( - modifier = Modifier.background(color = FirefoxTheme.colors.layer1), + modifier = Modifier + .padding(paddingValues) + .background(color = FirefoxTheme.colors.layer1), ) { - state.groupedIconOptions.forEach { (header, icons) -> + groupedIcons.forEach { (header, icons) -> item(contentType = { header::class }) { AppIconGroupHeader(header) } @@ -95,7 +175,7 @@ fun AppIconSelection( selected = iconSelected, onClick = { if (!iconSelected) { - store.dispatch(AppIconAction.SelectAppIcon(icon)) + onIconSelected(icon) } }, ) @@ -108,17 +188,6 @@ fun AppIconSelection( } } } - - state.userSelectedAppIcon?.let { - RestartWarningDialog( - onConfirm = { - store.dispatch(AppIconAction.ApplyAppIcon(it, state.currentAppIcon)) - }, - onDismiss = { - store.dispatch(AppIconAction.ResetSelection) - }, - ) - } } @Composable @@ -242,8 +311,9 @@ fun AppIcon( @Composable private fun RestartWarningDialog( - onConfirm: () -> Unit, - onDismiss: () -> Unit, + onConfirmClicked: () -> Unit, + onDismissClicked: () -> Unit, + onDismissed: () -> Unit, ) { AlertDialog( title = { @@ -263,17 +333,17 @@ private fun RestartWarningDialog( style = FirefoxTheme.typography.body2, ) }, - onDismissRequest = { onDismiss() }, + onDismissRequest = { onDismissed() }, confirmButton = { TextButton( text = stringResource(id = R.string.restart_warning_dialog_button_positive_2), - onClick = { onConfirm() }, + onClick = { onConfirmClicked() }, ) }, dismissButton = { TextButton( text = stringResource(id = R.string.restart_warning_dialog_button_negative), - onClick = { onDismiss() }, + onClick = { onDismissClicked() }, ) }, ) @@ -319,8 +389,9 @@ private fun AppIconOptionWithSubtitlePreview() { private fun RestartWarningDialogPreview() { FirefoxTheme { RestartWarningDialog( - onConfirm = {}, - onDismiss = {}, + onConfirmClicked = {}, + onDismissClicked = {}, + onDismissed = {}, ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelectionFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelectionFragment.kt @@ -67,9 +67,9 @@ class AppIconSelectionFragment : Fragment(), UserInteractionHandler { private fun updateAppIcon( currentAliasSuffix: String, newAliasSuffix: String, - ) { + ): Boolean { with(requireContext()) { - changeAppLauncherIcon( + return changeAppLauncherIcon( packageManager = packageManager, shortcutManager = ShortcutManagerWrapperDefault(this), shortcutInfo = ShortcutsUpdaterDefault(this), diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/ChangeAppLauncherIcon.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/ChangeAppLauncherIcon.kt @@ -73,6 +73,8 @@ private fun userHasAlternativeAppIconSet( * @param appAlias The currently used app alias. * @param newAppAlias The app alias we are updating to. * @param updateShortcuts A function that attempts to update the pinned shortcuts to use the [newAppAlias]. + * + * @returns `true` if the app icon was successfully updated, otherwise `false` */ fun changeAppLauncherIcon( packageManager: PackageManager, @@ -82,10 +84,11 @@ fun changeAppLauncherIcon( newAppAlias: ComponentName, updateShortcuts: (ShortcutManagerWrapper, ShortcutsUpdater, ComponentName) -> Boolean = ::updateShortcutsComponentName, -) { +): Boolean { newAppAlias.setEnabledStateTo(packageManager, true) - if (updateShortcuts(shortcutManager, shortcutInfo, newAppAlias)) { + val updated = updateShortcuts(shortcutManager, shortcutInfo, newAppAlias) + if (updated) { logger.info("Successfully attempted to update the app icon to the alternative.") appAlias.setEnabledStateTo(packageManager, false) } else { @@ -93,6 +96,7 @@ fun changeAppLauncherIcon( // If we can't successfully update the user shortcuts, then re-disable the app alias component. newAppAlias.setEnabledStateTo(packageManager, false) } + return updated } private fun resetAppIconsToDefault( diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -971,6 +971,8 @@ <string name="restart_warning_dialog_button_positive_2">Change icon</string> <!-- Dialog button text for cancelling app icon change--> <string name="restart_warning_dialog_button_negative">Cancel</string> + <!-- Error message snackbar shown after the user tried to apply the new icon but the Android OS threw an error while updating existing app shortcuts (e.g. private mode shortcut) --> + <string name="shortcuts_update_error">Failed to update the app icon. Remove existing shortcuts and try again.</string> <!-- Add-ons general availability nimbus message--> <!-- Title of the Nimbus message for extension general availability--> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconMiddlewareTest.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.iconpicker import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.support.test.ext.joinBlocking import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith @@ -14,7 +15,7 @@ import org.junit.runner.RunWith class AppIconMiddlewareTest { @Test - fun `WHEN the store receives an ApplyAppIcon action THEN the middleware calls the updateAppIcon interface with the correct data from the action`() { + fun `WHEN the store receives UserAction Confirmed THEN the middleware calls the updateAppIcon interface with the correct data from the action`() { val currentIcon = AppIcon.AppDefault val newIcon = AppIcon.AppRetro2004 var updatedCurrentIcon: AppIcon? = null @@ -22,6 +23,7 @@ class AppIconMiddlewareTest { val middleware = AppIconMiddleware { newIcon, currentIcon -> updatedNewIcon = newIcon updatedCurrentIcon = currentIcon + true } val store = AppIconStore( initialState = AppIconState( @@ -32,9 +34,59 @@ class AppIconMiddlewareTest { middleware = listOf(middleware), ) - store.dispatch(AppIconAction.ApplyAppIcon(newIcon = newIcon, currentIcon = currentIcon)) + store.dispatch(UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon)) assertEquals(currentIcon, updatedCurrentIcon) assertEquals(newIcon, updatedNewIcon) } + + @Test + fun `WHEN updateAppIcon call is successful THEN the middleware dispatches the Applied system action to the store`() { + val currentIcon = AppIcon.AppDefault + val newIcon = AppIcon.AppRetro2004 + val middleware = AppIconMiddleware { newIcon, currentIcon -> true } + val result = mutableListOf<AppIconAction>() + val store = AppIconStore( + initialState = AppIconState( + currentAppIcon = currentIcon, + ), + reducer = { state, action -> + result.add(action) + state + }, + middleware = listOf(middleware), + ) + + assertTrue(result.isEmpty()) + + val confirmAction = UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon) + store.dispatch(confirmAction) + + assertEquals(listOf(confirmAction, SystemAction.Applied(newIcon)), result) + } + + @Test + fun `WHEN updateAppIcon call returns with an a failure THEN the middleware dispatches the UpdateFailed system action to the store`() { + val currentIcon = AppIcon.AppDefault + val newIcon = AppIcon.AppRetro2004 + val middleware = AppIconMiddleware { newIcon, currentIcon -> false } + val result = mutableListOf<AppIconAction>() + val store = AppIconStore( + initialState = AppIconState( + currentAppIcon = currentIcon, + ), + reducer = { state, action -> + result.add(action) + state + }, + middleware = listOf(middleware), + ) + + assertTrue(result.isEmpty()) + + val confirmAction = UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon) + store.dispatch(confirmAction) + + assertEquals(listOf(confirmAction, SystemAction.UpdateFailed), result) + } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconReducerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconReducerTest.kt @@ -9,53 +9,53 @@ import org.junit.Test class AppIconReducerTest { @Test - fun `GIVEN SelectAppIcon action WHEN reducer is called THEN state is updated with the selected icon`() { + fun `GIVEN Selected user action WHEN reducer is called THEN state is updated with the selected icon`() { val initialState = AppIconState() val newIcon = AppIcon.AppRetro2004 assertEquals(null, initialState.userSelectedAppIcon) - val result = appIconReducer(initialState, AppIconAction.SelectAppIcon(newIcon)) + val result = appIconReducer(initialState, UserAction.Selected(newIcon)) assertEquals(newIcon, result.userSelectedAppIcon) } @Test - fun `GIVEN SelectAppIcon action WHEN reducer is called THEN the current icon is not changed`() { + fun `GIVEN Selected user action WHEN reducer is called THEN the current icon is not changed`() { val initialState = AppIconState() val newIcon = AppIcon.AppRetro2004 assertEquals(AppIcon.AppDefault, initialState.currentAppIcon) - val result = appIconReducer(initialState, AppIconAction.SelectAppIcon(newIcon)) + val result = appIconReducer(initialState, UserAction.Selected(newIcon)) assertEquals(AppIcon.AppDefault, result.currentAppIcon) } @Test - fun `GIVEN ResetSelection action WHEN reducer is called THEN state resets the user selected icon to null`() { + fun `GIVEN Dismissed user action WHEN reducer is called THEN state resets the user selected icon to null`() { val initialState = AppIconState(userSelectedAppIcon = AppIcon.AppRetro2004) assertEquals(AppIcon.AppRetro2004, initialState.userSelectedAppIcon) - val result = appIconReducer(initialState, AppIconAction.ResetSelection) + val result = appIconReducer(initialState, UserAction.Dismissed) assertEquals(null, result.userSelectedAppIcon) } @Test - fun `GIVEN ResetSelection action WHEN reducer is called THEN the current icon is not changed`() { + fun `GIVEN Dismissed user action WHEN reducer is called THEN the current icon is not changed`() { val initialState = AppIconState() assertEquals(AppIcon.AppDefault, initialState.currentAppIcon) - val result = appIconReducer(initialState, AppIconAction.ResetSelection) + val result = appIconReducer(initialState, UserAction.Dismissed) assertEquals(AppIcon.AppDefault, result.currentAppIcon) } @Test - fun `GIVEN ApplyAppIcon action WHEN reducer is called THEN the new icon becomes the new current and user selected icon resets`() { + fun `GIVEN Applied user action WHEN reducer is called THEN the new icon becomes the new current and the user selected icon resets to null`() { val newIcon = AppIcon.AppRetro2004 val currentIcon = AppIcon.AppDefault val initialState = AppIconState( @@ -66,7 +66,7 @@ class AppIconReducerTest { assertEquals(currentIcon, initialState.currentAppIcon) assertEquals(newIcon, initialState.userSelectedAppIcon) - val result = appIconReducer(initialState, AppIconAction.ApplyAppIcon(newIcon = newIcon, currentIcon = currentIcon)) + val result = appIconReducer(initialState, SystemAction.Applied(newIcon = newIcon)) assertEquals(newIcon, result.currentAppIcon) assertEquals(null, result.userSelectedAppIcon)