ADFA-3849: Add icon support to plugin MenuItem#1309
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughMenuItem data model gains an optional icon field with Java interoperability annotations. PluginActionItem is updated to resolve plugin-specific drawable icons based on menu item metadata, using a default package icon as fallback. ChangesPlugin Action Icon Resolution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/actions/PluginActionItem.kt (1)
25-31: 💤 Low valueSimplify the icon resolution and fix indentation.
Two small things on this block:
- Line 25 has stray indentation (extra leading spaces) relative to the surrounding
initblock.- The
ContextCompat.getDrawable(context, R.drawable.ic_package)fallback is duplicated across both branches. An elvis chain expresses the intent more directly: "try to resolve the plugin icon; otherwise use the default."♻️ Proposed simplification
init { label = menuItem.title - val iconResId = menuItem.icon - icon = if (iconResId != null) { - PluginDrawableResolver.resolve(iconResId, pluginId, context) - ?: ContextCompat.getDrawable(context, R.drawable.ic_package) - } else { - ContextCompat.getDrawable(context, R.drawable.ic_package) - } + icon = menuItem.icon + ?.let { PluginDrawableResolver.resolve(it, pluginId, context) } + ?: ContextCompat.getDrawable(context, R.drawable.ic_package) location = ActionItem.Location.EDITOR_TOOLBAR requiresUIThread = true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/actions/PluginActionItem.kt` around lines 25 - 31, Remove the stray leading spaces in the init block and simplify the icon resolution by replacing the two-branch null-check with a single elvis chain: get the menuItem.icon into iconResId, then set icon = PluginDrawableResolver.resolve(iconResId, pluginId, context) ?: ContextCompat.getDrawable(context, R.drawable.ic_package); ensure indentation matches surrounding init block and reference PluginDrawableResolver.resolve, iconResId, pluginId, context, and R.drawable.ic_package when making the change.plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/UIExtension.kt (1)
52-69: 💤 Low valueConsider documenting the new
iconfield and annotating it as a drawable resource.Minor consistency nit: the
tooltipTagfield has KDoc explaining its semantics, but the newiconfield lacks documentation describing that it's expected to be a plugin-owned drawable resource id (resolved viaPluginDrawableResolverinPluginActionItem). Without this context, plugin authors may pass an IDE-internalR.drawable.*id and be surprised when resolution falls back to the default package icon.Also, since
iconrepresents a drawable resource id, annotating it with@DrawableReswould improve type safety for Kotlin callers. (Note:NavigationItem.icon,ToolbarAction.icon, andFabAction.iconin this same file also lack@DrawableRes— this could be applied consistently or deferred.)✏️ Proposed docs/annotation addition
val tooltipTag: String? = null, - val icon: Int? = null, + /** + * Optional drawable resource id from the plugin's resources, used as the + * icon for this menu item on toolbar/menu surfaces. The id is resolved + * against the plugin's package via `PluginDrawableResolver`; if resolution + * fails or this is null, the IDE falls back to a default icon. + */ + `@androidx.annotation.DrawableRes` + val icon: Int? = null, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/UIExtension.kt` around lines 52 - 69, The new MenuItem.icon field lacks documentation and a drawable resource annotation; add KDoc to MenuItem.icon clarifying that it must be a plugin-owned drawable resource id (resolved via PluginDrawableResolver in PluginActionItem) to avoid passing IDE-internal R.drawable ids, and annotate the property with `@DrawableRes` (importing the annotation), and apply the same KDoc/@DrawableRes change to NavigationItem.icon, ToolbarAction.icon, and FabAction.icon for consistency across this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/actions/PluginActionItem.kt`:
- Around line 25-31: Remove the stray leading spaces in the init block and
simplify the icon resolution by replacing the two-branch null-check with a
single elvis chain: get the menuItem.icon into iconResId, then set icon =
PluginDrawableResolver.resolve(iconResId, pluginId, context) ?:
ContextCompat.getDrawable(context, R.drawable.ic_package); ensure indentation
matches surrounding init block and reference PluginDrawableResolver.resolve,
iconResId, pluginId, context, and R.drawable.ic_package when making the change.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/UIExtension.kt`:
- Around line 52-69: The new MenuItem.icon field lacks documentation and a
drawable resource annotation; add KDoc to MenuItem.icon clarifying that it must
be a plugin-owned drawable resource id (resolved via PluginDrawableResolver in
PluginActionItem) to avoid passing IDE-internal R.drawable ids, and annotate the
property with `@DrawableRes` (importing the annotation), and apply the same
KDoc/@DrawableRes change to NavigationItem.icon, ToolbarAction.icon, and
FabAction.icon for consistency across this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ecff043-ef42-4d5a-9d5c-899f8a490d5e
📒 Files selected for processing (2)
app/src/main/java/com/itsaky/androidide/actions/PluginActionItem.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/UIExtension.kt
No description provided.