ADFA-1718: Add Material 3 theme support and IdeThemeService for plugins #937
ADFA-1718: Add Material 3 theme support and IdeThemeService for plugins #937Daniel-ADFA merged 7 commits intostagefrom
Conversation
Plugins can now define a PluginTheme style extending Material 3 DayNight to automatically match the IDE's light/dark mode. PluginResourceContext dynamically resolves the plugin's theme via getIdentifier() and syncs configuration for correct resource qualifier selection. Hardcoded status colors in KeystoreGeneratorFragment are replaced with themed color resources, and requireContext() calls are fixed to use the view's plugin context to avoid Resources.
Define PluginTheme with BlueWave color overrides and day/night color resources matching the keystore plugin. Update the layout to use themed colorBackground and textColorSecondary attributes for proper light/dark mode rendering.
📝 Walkthrough
WalkthroughAdds per‑plugin theming: new light/dark color resources and PluginTheme styles for two plugins, a theme service API and implementation (IdeThemeService + IdeThemeServiceImpl), dynamic plugin resource/theme resolution on UI mode changes, wiring of per‑plugin theme services in PluginManager, and UI updates to use theme colors. Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Host Activity
participant PM as PluginManager
participant PRC as PluginResourceContext
participant TS as IdeThemeService
participant Plugin as Plugin UI
Host->>PM: request plugin context
PM->>PRC: create PluginResourceContext(plugin resources)
PM->>TS: register IdeThemeServiceImpl(pluginContext)
PRC->>TS: isDarkMode()
TS-->>PRC: darkModeFlag
alt darkModeFlag = true
PRC->>PRC: resolve PluginTheme (night) and rebuild resources
else
PRC->>PRC: resolve PluginTheme (day) or fallback
end
PRC->>PRC: build/cache Theme & inflater
PRC-->>PM: plugin context ready
Host->>Plugin: inflate UI via PRC.inflateLayout(...)
Plugin->>TS: addThemeChangeListener / isDarkMode() (optional)
TS-->>Plugin: notify on theme changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt`:
- Around line 46-50: The theme lookup uses pluginPackageInfo?.packageName which
can be null; change the call that computes pluginThemeResId (the getIdentifier
invocation in PluginResourceContext) to pass the class's non-null packageName
(i.e., the overridden getPackageName()/packageName property on
PluginResourceContext) as the package parameter so getIdentifier reliably finds
"PluginTheme" and doesn't return 0 when pluginPackageInfo is null.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt`:
- Around line 12-29: Listeners are stored in a mutableList which can throw
ConcurrentModificationException if a listener removes itself during
notifyThemeChanged; replace the mutableList backing field with a thread-safe
CopyOnWriteArrayList and update addThemeChangeListener/removeThemeChangeListener
to operate on that list so notifyThemeChanged can safely call listeners.forEach
{ it.onThemeChanged(isDark) } without risking concurrent modification; reference
symbols: listeners, ThemeChangeListener, notifyThemeChanged,
addThemeChangeListener, removeThemeChangeListener.
🧹 Nitpick comments (1)
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/PluginFragment.kt (1)
85-95: Avoid mutating shared Resources viaupdateConfiguration.
Resources.updateConfigurationis deprecated and mutates the sharedpluginContextresources, which can leak configuration changes across fragments/activities. Prefer a configuration-scoped context and clone the inflater from it instead.♻️ Suggested refactor
- `@Suppress`("DEPRECATION") - pluginContext.resources.updateConfiguration( - activityContext.resources.configuration, - activityContext.resources.displayMetrics - ) - - val inflater = pluginContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE) as? LayoutInflater - return inflater ?: defaultInflater.cloneInContext(pluginContext) + val configContext = + pluginContext.createConfigurationContext(activityContext.resources.configuration) + val inflater = + configContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE) as? LayoutInflater + return inflater ?: defaultInflater.cloneInContext(configContext)
...nager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt
Show resolved
Hide resolved
...anager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/PluginFragment.kt`:
- Around line 88-89: The current inflation logic in PluginFragment obtains the
LayoutInflater via pluginContext.getSystemService which yields a plain
PhoneLayoutInflater and loses AppCompat/Material view factories; instead always
use the existing default inflater and clone it into pluginContext so
AppCompatViewInflater/Material factories are preserved. Modify the method that
returns the inflater in PluginFragment to remove the system-service path and
return defaultInflater.cloneInContext(pluginContext) unconditionally, ensuring
XML tags are correctly replaced by AppCompat/Material implementations.
🧹 Nitpick comments (5)
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/PluginFragment.kt (1)
86-86: BarepluginContext.themeaccess needs an explanatory comment.This expression has no visible side effect. If the intent is to force lazy initialization of the
Resources.Theme(so that the theme is resolved before the inflater is obtained), add a comment explaining that — otherwise it reads like dead code and will likely be removed in a future cleanup.Suggested change
- pluginContext.theme + // Force lazy initialization of the theme so that themed attributes + // are resolved correctly when the inflater is used. + pluginContext.themeplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt (1)
26-26: Suppress or annotate the emptyonLowMemorybody.Detekt flags this empty block. A brief inline comment (e.g.,
// no-op) or a@Suppress("EmptyFunctionBlock")annotation silences the warning while signalling intent.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (2)
444-448: Consider addingdispose()to theIdeThemeServiceinterface to avoid the downcast.The downcast to
IdeThemeServiceImplcouplesPluginManagerto the concrete type. Ifdispose()were part of theIdeThemeServicecontract (or a separateDisposableinterface), you could eliminate theischeck and keep the manager decoupled. Not blocking — current code is functionally correct and null-safe.
949-956:createPluginContextandcreatePluginContextWithResourcesare nearly identical — consider extracting shared service registration.Both methods register the exact same set of services (project, UI, build, tooltip, editor_tab, file, sidebar, theme) with identical logic. The only difference is the
Contextpassed toPluginContextImpl. Extracting aregisterCommonServices(pluginServiceRegistry, pluginId, permissions)helper would eliminate duplication and ensure future service additions don't need to be made in two places.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt (1)
144-147: Nit: redundant FQN whenViewis already imported.
android.view.ViewGroupandandroid.view.Vieware fully qualified in the signature, butViewis already imported at Line 13. Consider importingViewGroupas well for consistency.Proposed fix
Add to imports:
import android.view.ViewGroupThen simplify the signature:
- fun inflateLayout(layoutResId: Int, root: android.view.ViewGroup? = null, attachToRoot: Boolean = false): android.view.View { + fun inflateLayout(layoutResId: Int, root: ViewGroup? = null, attachToRoot: Boolean = false): View {
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/PluginFragment.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
keystore-generator-plugin/src/main/AndroidManifest.xml (1)
6-6:⚠️ Potential issue | 🟡 MinorUpdate manifest theme to
PluginThemefor consistency.The PR correctly introduces
PluginTheme(Material 3 DayNight-based) instyles.xml. However, the manifest still declares@style/Theme.AppCompat, which is superseded at runtime.PluginResourceContext.getTheme()explicitly resolvesPluginThemeviagetIdentifier()and applies it, making the manifest-level declaration legacy/unused. Update line 6 toandroid:theme="@style/PluginTheme"to align manifest metadata with the actual runtime theme applied byPluginResourceContext.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt`:
- Around line 35-43: recreatePluginResources currently returns early when
pluginPackageInfo is null, leaving pluginResources stale and preventing
night-mode changes in getTheme; change the method so that if pluginPackageInfo
is null it logs a warning and rebuilds pluginResources from the host/base
context instead (use baseContext.resources.assets or baseContext.resources to
create a Resources instance with newConfig) so the fallback theme respects the
new configuration, and keep the existing asset-manager path logic when
pluginPackageInfo is present (refer to recreatePluginResources,
pluginPackageInfo, pluginResources, getTheme, and baseContext).
- Around line 156-158: The inflateLayout function uses fully-qualified types for
ViewGroup and View; add an import for android.view.ViewGroup and change the
signature of PluginResourceContext.inflateLayout (function name: inflateLayout)
to use the short type names (root: ViewGroup? = null, return type: View) to
match the existing imported View and keep the code consistent; update any
references in that file accordingly.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt`:
- Around line 11-13: IdeThemeServiceImpl currently stores the incoming Context
and registers ComponentCallbacks in its init block, which can leak an Activity
if one is passed; update the constructor/storage to use
context.applicationContext (store it in the class field used by the init block
and by dispose()) so the long-lived IdeThemeServiceImpl, its ComponentCallbacks
registration, and the dispose() lifecycle reference the application context
rather than a potential Activity context (identify places by the class name
IdeThemeServiceImpl, the constructor parameter context, the init block that
registers ComponentCallbacks, and the dispose() method).
- Line 27: The empty onLowMemory() in IdeThemeServiceImpl (from the
ComponentCallbacks interface) is flagged by detekt; either add a brief comment
explaining it's intentionally no-op (e.g., "// No-op: handled elsewhere" or
similar) or explicitly return Unit, or annotate the function with
`@Suppress`("EmptyFunctionBlock") to satisfy detekt; update the override of
onLowMemory() accordingly so the intent is explicit and the EmptyFunctionBlock
rule is addressed.
🧹 Nitpick comments (2)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt`: - Around line 156-158: The inflateLayout function uses fully-qualified types for ViewGroup and View; add an import for android.view.ViewGroup and change the signature of PluginResourceContext.inflateLayout (function name: inflateLayout) to use the short type names (root: ViewGroup? = null, return type: View) to match the existing imported View and keep the code consistent; update any references in that file accordingly. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt`: - Line 27: The empty onLowMemory() in IdeThemeServiceImpl (from the ComponentCallbacks interface) is flagged by detekt; either add a brief comment explaining it's intentionally no-op (e.g., "// No-op: handled elsewhere" or similar) or explicitly return Unit, or annotate the function with `@Suppress`("EmptyFunctionBlock") to satisfy detekt; update the override of onLowMemory() accordingly so the intent is explicit and the EmptyFunctionBlock rule is addressed.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt (1)
156-158: Use short names with an import forViewGroupinstead of FQNs.
Viewis already imported (line 13). Add an import forViewGroupand use short names in the signature for consistency.+import android.view.ViewGroup ... - fun inflateLayout(layoutResId: Int, root: android.view.ViewGroup? = null, attachToRoot: Boolean = false): android.view.View { + fun inflateLayout(layoutResId: Int, root: ViewGroup? = null, attachToRoot: Boolean = false): View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginResourceContext.kt` around lines 156 - 158, The inflateLayout function uses fully-qualified types for ViewGroup and View; add an import for android.view.ViewGroup and change the signature of PluginResourceContext.inflateLayout (function name: inflateLayout) to use the short type names (root: ViewGroup? = null, return type: View) to match the existing imported View and keep the code consistent; update any references in that file accordingly.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt (1)
27-27: Suppress or document the emptyonLowMemory()to satisfy detekt.This is required by the
ComponentCallbacksinterface but flagged by detekt'sEmptyFunctionBlockrule. A brief comment orUnitreturn makes the intent explicit.- override fun onLowMemory() {} + override fun onLowMemory() { /* Required by ComponentCallbacks; nothing to release */ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeThemeServiceImpl.kt` at line 27, The empty onLowMemory() in IdeThemeServiceImpl (from the ComponentCallbacks interface) is flagged by detekt; either add a brief comment explaining it's intentionally no-op (e.g., "// No-op: handled elsewhere" or similar) or explicitly return Unit, or annotate the function with `@Suppress`("EmptyFunctionBlock") to satisfy detekt; update the override of onLowMemory() accordingly so the intent is explicit and the EmptyFunctionBlock rule is addressed.
Add Material 3 theme support and IdeThemeService for plugins
Plugins can now define a PluginTheme style extending Material 3 DayNight
to automatically match the IDE's light/dark mode. PluginResourceContext
dynamically resolves the plugin's theme via getIdentifier() and syncs
configuration for correct resource qualifier selection.
Changes:
dark mode queries
analyzer plugins
context for plugin resource resolution