#1586 issue: add custom app icon option in Appearance settings#1587
#1586 issue: add custom app icon option in Appearance settings#1587iRahul360 wants to merge 1 commit into
Conversation
Reviewer's GuideImplements a user-selectable app icon feature by adding launcher activity aliases, a preference-backed app icon selector in Appearance settings, and an AppIconManager utility to enable/disable the corresponding launcher components at runtime. Sequence diagram for applying a new app icon selectionsequenceDiagram
actor User
participant SettingsAppearanceScreen
participant UiPreferences
participant AppIconManager
participant PackageManager
User->>SettingsAppearanceScreen: Open Appearance settings
SettingsAppearanceScreen->>UiPreferences: Read appIcon preference
UiPreferences-->>SettingsAppearanceScreen: Current icon id
User->>SettingsAppearanceScreen: Change App Icon preference
SettingsAppearanceScreen->>UiPreferences: Persist new icon id
SettingsAppearanceScreen->>AppIconManager: switchIcon(context, AppIcon.fromId(newIconId))
AppIconManager->>AppIconManager: Resolve AppIcon from id
loop Disable all aliases
AppIconManager->>PackageManager: setComponentEnabledSetting(MainActivityDefault, DISABLED)
AppIconManager->>PackageManager: setComponentEnabledSetting(MainActivityAlt1, DISABLED)
end
AppIconManager->>PackageManager: setComponentEnabledSetting(selected_alias, ENABLED)
PackageManager-->>User: Updated launcher icon for app
Updated class diagram for app icon preferences and managerclassDiagram
class UiPreferences {
- preferenceStore
+ appIcon() String
}
class SettingsAppearanceScreen {
+ getAppIconGroup(uiPreferences UiPreferences) PreferenceGroup
}
class AppIconManager {
+ switchIcon(context Context, icon AppIcon)
}
class AppIcon {
<<sealed>>
+ id String
+ alias String
+ AppIcon(id String, alias String)
+ icons List~AppIcon~
+ fromId(id String) AppIcon
}
class Default {
+ Default()
}
class Alt1 {
+ Alt1()
}
UiPreferences <.. SettingsAppearanceScreen : uses
AppIconManager <.. SettingsAppearanceScreen : uses
AppIconManager *-- AppIcon : manages
AppIcon <|-- Default
AppIcon <|-- Alt1
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a feature that allows users to customize the application icon directly from the Appearance settings. It leverages Android's activity-alias system to swap between different launcher icons and provides a new management class to handle the state changes and UI integration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. An icon choice for all to see, A simple switch for you and me. From default look to something new, The app now wears a different hue. Footnotes
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The string IDs for icons ("default", "alt1") are duplicated between
UiPreferences.appIcon, the settings entries, andAppIconManager.AppIcon; consider referencingAppIconManager.AppIconIDs/constants everywhere to avoid mismatches when adding or renaming icons. - In
AppIconManager.switchIcon, all component enabling/disabling is done without error handling or user feedback; consider wrapping the PackageManager calls to handle failures gracefully (e.g., logging or surfacing an error to the user) so a bad state doesn’t silently persist.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The string IDs for icons ("default", "alt1") are duplicated between `UiPreferences.appIcon`, the settings entries, and `AppIconManager.AppIcon`; consider referencing `AppIconManager.AppIcon` IDs/constants everywhere to avoid mismatches when adding or renaming icons.
- In `AppIconManager.switchIcon`, all component enabling/disabling is done without error handling or user feedback; consider wrapping the PackageManager calls to handle failures gracefully (e.g., logging or surfacing an error to the user) so a bad state doesn’t silently persist.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request implements a feature to change the application icon via settings. It adds activity aliases to the manifest, updates UI preferences, and introduces an AppIconManager. Feedback focuses on preventing duplicate launcher icons, optimizing PackageManager calls, utilizing data object for icon definitions, and using remember in the UI to avoid redundant map allocations.
| android:resource="@xml/shortcuts" /> | ||
| </activity> | ||
| <!-- KMK --> | ||
| <activity-alias |
There was a problem hiding this comment.
Adding these activity-alias entries with the LAUNCHER category while the base MainActivity (lines 65-74) still retains its own launcher intent filter will result in duplicate icons appearing in the app drawer. You should remove the <intent-filter> containing android.intent.category.LAUNCHER from the MainActivity declaration to ensure only the selected alias is used as the entry point.
| fun switchIcon(context: Context, icon: AppIcon) { | ||
| val pm = context.packageManager | ||
| val pkg = context.packageName | ||
|
|
||
| // Disable all first | ||
| AppIcon.icons.forEach { | ||
| pm.setComponentEnabledSetting( | ||
| ComponentName(pkg, "$pkg${it.alias}"), | ||
| PackageManager.COMPONENT_ENABLED_STATE_DISABLED, | ||
| PackageManager.DONT_KILL_APP, | ||
| ) | ||
| } | ||
|
|
||
| // Enable selected | ||
| pm.setComponentEnabledSetting( | ||
| ComponentName(pkg, "$pkg${icon.alias}"), | ||
| PackageManager.COMPONENT_ENABLED_STATE_ENABLED, | ||
| PackageManager.DONT_KILL_APP, | ||
| ) | ||
| } |
There was a problem hiding this comment.
The current implementation performs redundant PackageManager calls by disabling all icons (including the one about to be enabled) and then enabling the target. It's more efficient to check if the target icon is already enabled and only disable the others. Also, note that PackageManager.DONT_KILL_APP might delay the icon update on some launchers until the app is manually restarted or the launcher process refreshes.
fun switchIcon(context: Context, icon: AppIcon) {
val pm = context.packageManager
val pkg = context.packageName
val targetComponent = ComponentName(pkg, "$pkg${icon.alias}")
// Avoid redundant work if the icon is already active
if (pm.getComponentEnabledSetting(targetComponent) == PackageManager.COMPONENT_ENABLED_STATE_ENABLED) {
return
}
// Disable all other icons
AppIcon.icons.filter { it != icon }.forEach {
pm.setComponentEnabledSetting(
ComponentName(pkg, "$pkg${it.alias}"),
PackageManager.COMPONENT_ENABLED_STATE_DISABLED,
PackageManager.DONT_KILL_APP,
)
}
// Enable selected icon
pm.setComponentEnabledSetting(
targetComponent,
PackageManager.COMPONENT_ENABLED_STATE_ENABLED,
PackageManager.DONT_KILL_APP,
)
}| object Default : AppIcon("default", ".MainActivityDefault") | ||
| object Alt1 : AppIcon("alt1", ".MainActivityAlt1") |
There was a problem hiding this comment.
Since these objects are part of a sealed class and do not hold state, it is recommended to use data object (available in Kotlin 1.9+) for better toString(), equals(), and hashCode() implementations.
| object Default : AppIcon("default", ".MainActivityDefault") | |
| object Alt1 : AppIcon("alt1", ".MainActivityAlt1") | |
| data object Default : AppIcon("default", ".MainActivityDefault") | |
| data object Alt1 : AppIcon("alt1", ".MainActivityAlt1") |
| private fun getAppIconGroup( | ||
| uiPreferences: UiPreferences, | ||
| ): Preference.PreferenceGroup { | ||
| val context = LocalContext.current | ||
| return Preference.PreferenceGroup( | ||
| title = stringResource(KMR.strings.pref_app_icon), | ||
| preferenceItems = persistentListOf( | ||
| Preference.PreferenceItem.ListPreference( | ||
| preference = uiPreferences.appIcon(), | ||
| entries = mapOf( | ||
| "default" to stringResource(KMR.strings.pref_app_icon_default), | ||
| "alt1" to stringResource(KMR.strings.pref_app_icon_alt1), | ||
| ).toImmutableMap(), | ||
| title = stringResource(KMR.strings.pref_app_icon), | ||
| onValueChanged = { newIcon -> | ||
| AppIconManager.switchIcon( | ||
| context, | ||
| AppIconManager.AppIcon.fromId(newIcon), | ||
| ) | ||
| true | ||
| }, | ||
| ), | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
The entries map is currently recreated on every recomposition of getAppIconGroup. It is better to remember the map to avoid unnecessary allocations, especially since it depends on localized strings.
@Composable
private fun getAppIconGroup(
uiPreferences: UiPreferences,
): Preference.PreferenceGroup {
val context = LocalContext.current
val defaultLabel = stringResource(KMR.strings.pref_app_icon_default)
val alt1Label = stringResource(KMR.strings.pref_app_icon_alt1)
val entries = remember(defaultLabel, alt1Label) {
mapOf(
"default" to defaultLabel,
"alt1" to alt1Label,
).toImmutableMap()
}
return Preference.PreferenceGroup(
title = stringResource(KMR.strings.pref_app_icon),
preferenceItems = persistentListOf(
Preference.PreferenceItem.ListPreference(
preference = uiPreferences.appIcon(),
entries = entries,
title = stringResource(KMR.strings.pref_app_icon),
onValueChanged = { newIcon ->
AppIconManager.switchIcon(
context,
AppIconManager.AppIcon.fromId(newIcon),
)
true
},
),
),
)
}
Closes #1586
Added custom app icon feature:
Summary by Sourcery
Add a selectable app icon setting and wire it to launcher icon aliases on Android.
New Features:
Enhancements: