-
-
Notifications
You must be signed in to change notification settings - Fork 47
ADFA-4826: Add shared IDE Compose theming in common-compose #1653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import com.itsaky.androidide.build.config.BuildConfig | ||
|
|
||
| plugins { | ||
| id("com.android.library") | ||
| id("kotlin-android") | ||
| alias(libs.plugins.kotlin.compose) | ||
| } | ||
|
|
||
| android { | ||
| namespace = "${BuildConfig.PACKAGE_NAME}.common.compose" | ||
|
|
||
| buildFeatures { | ||
| compose = true | ||
| } | ||
| } | ||
|
|
||
| dependencies { | ||
| // api, not implementation: consumers write Compose against these types (ColorScheme, Typography), | ||
| // so they must be on the consumer's compile classpath. | ||
| api(platform(libs.compose.bom)) | ||
| api(libs.compose.runtime) | ||
| api(libs.compose.material3) | ||
| api(libs.compose.ui) | ||
|
|
||
| implementation(libs.compose.foundation) | ||
| implementation(libs.google.material) | ||
|
|
||
| testImplementation(projects.testing.unit) | ||
| } |
86 changes: 86 additions & 0 deletions
86
common-compose/src/main/java/com/itsaky/androidide/common/compose/IdeColorScheme.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| package com.itsaky.androidide.common.compose | ||
|
|
||
| import android.content.Context | ||
| import androidx.compose.material3.ColorScheme | ||
| import androidx.compose.material3.darkColorScheme | ||
| import androidx.compose.material3.lightColorScheme | ||
| import androidx.compose.ui.graphics.Color | ||
| import com.google.android.material.color.MaterialColors | ||
| import com.google.android.material.R as MaterialR | ||
|
|
||
| /** | ||
| * Resolves a theme colour attribute, or null when the attribute is not defined. | ||
| * | ||
| * Exists so [ideColorScheme] can be exercised without an Android [Context]: the mapping from Material | ||
| * attributes to Compose colour roles is the part worth testing, and it is pure once resolution is a | ||
| * parameter. | ||
| */ | ||
| typealias ColorAttrResolver = (attr: Int) -> Color? | ||
|
|
||
| /** | ||
| * A Compose [ColorScheme] built from the IDE's XML theme, so Compose UI matches the surrounding | ||
| * View-based IDE exactly -- including the user's light/dark choice and any theme overlay in effect. | ||
| * | ||
| * Every role falls back to the stock Material baseline ([lightColorScheme]/[darkColorScheme]) when its | ||
| * attribute is undefined, so a partial XML theme degrades to sensible colours rather than to | ||
| * transparent or black. | ||
| * | ||
| * [dark] selects the baseline. It is the caller's business rather than something read from the context | ||
| * here, because the attribute values already come from whichever theme is applied; the baseline only | ||
| * matters for roles the theme does not define. | ||
| */ | ||
| fun ideColorScheme( | ||
| dark: Boolean, | ||
| resolve: ColorAttrResolver, | ||
| ): ColorScheme { | ||
| val base = if (dark) darkColorScheme() else lightColorScheme() | ||
|
|
||
| fun role( | ||
| attr: Int, | ||
| fallback: Color, | ||
| ): Color = resolve(attr) ?: fallback | ||
|
|
||
| return base.copy( | ||
| primary = role(MaterialR.attr.colorPrimary, base.primary), | ||
| onPrimary = role(MaterialR.attr.colorOnPrimary, base.onPrimary), | ||
| primaryContainer = role(MaterialR.attr.colorPrimaryContainer, base.primaryContainer), | ||
| onPrimaryContainer = role(MaterialR.attr.colorOnPrimaryContainer, base.onPrimaryContainer), | ||
| secondary = role(MaterialR.attr.colorSecondary, base.secondary), | ||
| onSecondary = role(MaterialR.attr.colorOnSecondary, base.onSecondary), | ||
| secondaryContainer = role(MaterialR.attr.colorSecondaryContainer, base.secondaryContainer), | ||
| onSecondaryContainer = role(MaterialR.attr.colorOnSecondaryContainer, base.onSecondaryContainer), | ||
| tertiary = role(MaterialR.attr.colorTertiary, base.tertiary), | ||
| onTertiary = role(MaterialR.attr.colorOnTertiary, base.onTertiary), | ||
| tertiaryContainer = role(MaterialR.attr.colorTertiaryContainer, base.tertiaryContainer), | ||
| onTertiaryContainer = role(MaterialR.attr.colorOnTertiaryContainer, base.onTertiaryContainer), | ||
| // colorBackground is a platform attribute, not a Material one. | ||
| background = role(android.R.attr.colorBackground, base.background), | ||
| onBackground = role(MaterialR.attr.colorOnBackground, base.onBackground), | ||
| surface = role(MaterialR.attr.colorSurface, base.surface), | ||
| onSurface = role(MaterialR.attr.colorOnSurface, base.onSurface), | ||
| surfaceVariant = role(MaterialR.attr.colorSurfaceVariant, base.surfaceVariant), | ||
| onSurfaceVariant = role(MaterialR.attr.colorOnSurfaceVariant, base.onSurfaceVariant), | ||
| outline = role(MaterialR.attr.colorOutline, base.outline), | ||
| outlineVariant = role(MaterialR.attr.colorOutlineVariant, base.outlineVariant), | ||
| error = role(MaterialR.attr.colorError, base.error), | ||
| onError = role(MaterialR.attr.colorOnError, base.onError), | ||
| errorContainer = role(MaterialR.attr.colorErrorContainer, base.errorContainer), | ||
| onErrorContainer = role(MaterialR.attr.colorOnErrorContainer, base.onErrorContainer), | ||
| ) | ||
| } | ||
|
|
||
| /** [ideColorScheme] reading the live attribute values off this context's theme. */ | ||
| fun Context.ideColorScheme(dark: Boolean): ColorScheme = ideColorScheme(dark, materialColorResolver()) | ||
|
|
||
| /** | ||
| * Resolves through [MaterialColors], which handles both direct colour values and colour-resource | ||
| * references. A sentinel distinguishes "undefined" from a legitimately resolved colour -- returning 0 | ||
| * would be indistinguishable from transparent black. | ||
| */ | ||
| private fun Context.materialColorResolver(): ColorAttrResolver = | ||
| { attr -> | ||
| val resolved = MaterialColors.getColor(this, attr, UNRESOLVED) | ||
| if (resolved == UNRESOLVED) null else Color(resolved) | ||
| } | ||
|
|
||
| private const val UNRESOLVED = Int.MIN_VALUE |
48 changes: 48 additions & 0 deletions
48
common-compose/src/main/java/com/itsaky/androidide/common/compose/IdeTheme.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package com.itsaky.androidide.common.compose | ||
|
|
||
| import androidx.compose.foundation.isSystemInDarkTheme | ||
| import androidx.compose.material3.LocalContentColor | ||
| import androidx.compose.material3.MaterialTheme | ||
| import androidx.compose.material3.Typography | ||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.CompositionLocalProvider | ||
| import androidx.compose.runtime.remember | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.platform.LocalContext | ||
|
|
||
| /** | ||
| * Wraps Compose content in a [MaterialTheme] whose colours come from the IDE's XML theme, so a Compose | ||
| * surface is indistinguishable from the View-based UI around it. | ||
| * | ||
| * Use this instead of a bare `MaterialTheme { }`: the bare form falls back to Material's purple | ||
| * baseline, which looks nothing like the IDE and ignores the user's theme entirely. | ||
| * | ||
| * [typography] is a parameter because branding type is a separate concern from colour -- overlay | ||
| * windows brand theirs with the IDE's Atkinson Hyperlegible face, while most surfaces want the | ||
| * default. | ||
| * | ||
| * [contentColor] seeds [LocalContentColor], which is **not** something [MaterialTheme] sets. Its | ||
| * global default is [androidx.compose.ui.graphics.Color.Black], and normally only a `Surface` replaces | ||
| * it (via `contentColorFor`). Content hosted inside a View that already draws the background -- a | ||
| * `BottomSheetDialog`, an overlay window, a `ComposeView` in an XML layout -- has no `Surface`, so | ||
| * every `Text` would render black regardless of how dark the background is. Defaulting to `onSurface` | ||
| * makes that case correct; a `Surface` further down still overrides it, so screens that do use one are | ||
| * unaffected. | ||
| */ | ||
| @Composable | ||
| fun IdeTheme( | ||
| typography: Typography = MaterialTheme.typography, | ||
| contentColor: Color? = null, | ||
| content: @Composable () -> Unit, | ||
| ) { | ||
| val context = LocalContext.current | ||
| val dark = isSystemInDarkTheme() | ||
| // Attribute resolution reads the theme, so it is keyed on both the context and the dark-mode flag. | ||
| val colorScheme = remember(context, dark) { context.ideColorScheme(dark) } | ||
| MaterialTheme(colorScheme = colorScheme, typography = typography) { | ||
| CompositionLocalProvider( | ||
| LocalContentColor provides (contentColor ?: colorScheme.onSurface), | ||
| content = content, | ||
| ) | ||
| } | ||
| } |
123 changes: 123 additions & 0 deletions
123
common-compose/src/test/java/com/itsaky/androidide/common/compose/IdeColorSchemeTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| package com.itsaky.androidide.common.compose | ||
|
|
||
| import androidx.compose.material3.ColorScheme | ||
| import androidx.compose.material3.darkColorScheme | ||
| import androidx.compose.material3.lightColorScheme | ||
| import androidx.compose.ui.graphics.Color | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test | ||
| import com.google.android.material.R as MaterialR | ||
|
|
||
| /** | ||
| * The attribute-to-role mapping, tested with a fake resolver rather than a real themed | ||
| * [android.content.Context]. | ||
| * | ||
| * The interesting behaviour is entirely in the mapping and the per-role fallback, so making resolution | ||
| * a parameter buys full coverage with no Robolectric and no theme fixtures. | ||
| */ | ||
| class IdeColorSchemeTest { | ||
| private val red = Color(0xFFFF0000) | ||
| private val green = Color(0xFF00FF00) | ||
|
|
||
| /** | ||
| * Every role [ideColorScheme] claims to map, paired with its name for readable failures. | ||
| * | ||
| * [ColorScheme] has no structural `equals`, so whole-scheme comparison would compare identity and | ||
| * pass vacuously. Listing the roles also makes "did the mapping forget one?" a real assertion. | ||
| */ | ||
| private fun mappedRoles(scheme: ColorScheme): List<Pair<String, Color>> = | ||
| listOf( | ||
| "primary" to scheme.primary, | ||
| "onPrimary" to scheme.onPrimary, | ||
| "primaryContainer" to scheme.primaryContainer, | ||
| "onPrimaryContainer" to scheme.onPrimaryContainer, | ||
| "secondary" to scheme.secondary, | ||
| "onSecondary" to scheme.onSecondary, | ||
| "secondaryContainer" to scheme.secondaryContainer, | ||
| "onSecondaryContainer" to scheme.onSecondaryContainer, | ||
| "tertiary" to scheme.tertiary, | ||
| "onTertiary" to scheme.onTertiary, | ||
| "tertiaryContainer" to scheme.tertiaryContainer, | ||
| "onTertiaryContainer" to scheme.onTertiaryContainer, | ||
| "background" to scheme.background, | ||
| "onBackground" to scheme.onBackground, | ||
| "surface" to scheme.surface, | ||
| "onSurface" to scheme.onSurface, | ||
| "surfaceVariant" to scheme.surfaceVariant, | ||
| "onSurfaceVariant" to scheme.onSurfaceVariant, | ||
| "outline" to scheme.outline, | ||
| "outlineVariant" to scheme.outlineVariant, | ||
| "error" to scheme.error, | ||
| "onError" to scheme.onError, | ||
| "errorContainer" to scheme.errorContainer, | ||
| "onErrorContainer" to scheme.onErrorContainer, | ||
| ) | ||
|
|
||
| @Test | ||
| fun `a resolved attribute wins over the baseline`() { | ||
| val scheme = ideColorScheme(dark = false) { attr -> red.takeIf { attr == MaterialR.attr.colorPrimary } } | ||
|
|
||
| assertEquals(red, scheme.primary) | ||
| } | ||
|
|
||
| @Test | ||
| fun `an undefined attribute falls back to the light baseline`() { | ||
| val scheme = ideColorScheme(dark = false) { null } | ||
|
|
||
| assertEquals(mappedRoles(lightColorScheme()), mappedRoles(scheme)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `an undefined attribute falls back to the dark baseline`() { | ||
| val scheme = ideColorScheme(dark = true) { null } | ||
|
|
||
| assertEquals(mappedRoles(darkColorScheme()), mappedRoles(scheme)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `roles fall back individually, so a partial theme still yields sensible colours`() { | ||
| // A theme defining only the surface pair, as a minimal overlay might. | ||
| val scheme = | ||
| ideColorScheme(dark = false) { attr -> | ||
| when (attr) { | ||
| MaterialR.attr.colorSurface -> red | ||
| MaterialR.attr.colorOnSurface -> green | ||
| else -> null | ||
| } | ||
| } | ||
|
|
||
| assertEquals(red, scheme.surface) | ||
| assertEquals(green, scheme.onSurface) | ||
| // Everything else keeps the baseline rather than going transparent or black. | ||
| assertEquals(lightColorScheme().primary, scheme.primary) | ||
| assertEquals(lightColorScheme().error, scheme.error) | ||
| } | ||
|
|
||
| @Test | ||
| fun `background reads the platform attribute, not a Material one`() { | ||
| // colorBackground has no Material equivalent; mapping it to one would silently lose the theme's | ||
| // window background. | ||
| val scheme = ideColorScheme(dark = false) { attr -> red.takeIf { attr == android.R.attr.colorBackground } } | ||
|
|
||
| assertEquals(red, scheme.background) | ||
| } | ||
|
|
||
| @Test | ||
| fun `every role the mapping claims to cover is actually resolved`() { | ||
| // Resolving everything to one colour proves no listed role was left out of the copy() call: an | ||
| // unmapped role would still hold its baseline value. | ||
| val scheme = ideColorScheme(dark = false) { red } | ||
|
|
||
| val unmapped = mappedRoles(scheme).filter { (_, color) -> color != red } | ||
| assertTrue("roles not read from the theme: ${unmapped.map { it.first }}", unmapped.isEmpty()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `the dark baseline differs from the light one, so the flag is not ignored`() { | ||
| val light = ideColorScheme(dark = false) { null } | ||
| val dark = ideColorScheme(dark = true) { null } | ||
|
|
||
| assertTrue(mappedRoles(light) != mappedRoles(dark)) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use JUnit Jupiter and Truth in this new test.
Replace
org.junit.Testwithorg.junit.jupiter.api.Test. ReplaceassertEqualsandassertTruewith Truth assertions. This keeps new tests consistent with the shared test conventions.As per coding guidelines, “Use JUnit Jupiter, Truth, MockK for new tests.”
Also applies to: 57-121
🤖 Prompt for AI Agents
Source: Coding guidelines