diff --git a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt index d6e438e..059b530 100644 --- a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt +++ b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt @@ -94,6 +94,7 @@ import com.runicgateway.app.ui.shard.MarketVendorScreen import com.runicgateway.app.ui.shard.RulesScreen import com.runicgateway.app.ui.shard.ShardBoard import com.runicgateway.app.ui.shard.ShardScreen +import com.runicgateway.app.ui.theme.LocalShardStructure import com.runicgateway.app.ui.wiki.WikiPageScreen import com.runicgateway.app.ui.wiki.WikiScreen import kotlinx.coroutines.launch @@ -263,6 +264,7 @@ fun RunicApp( } }, colors = drawerItemColors, + shape = LocalShardStructure.current.pill, modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding), ) NavigationDrawerItem( @@ -273,6 +275,7 @@ fun RunicApp( onChangeServer() }, colors = drawerItemColors, + shape = LocalShardStructure.current.pill, modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding), ) } @@ -383,6 +386,11 @@ private fun NavRow( } }, colors = colors, + // Like Card's elevation, NavigationDrawerItem takes its shape as a default + // argument (CircleShape) rather than from the theme, so --radius-pill has to + // be handed to it at every call site or the selected row stays fully round + // while every other radius follows the shard (phase 8's AC-5 walk). + shape = LocalShardStructure.current.pill, modifier = Modifier .padding(NavigationDrawerItemDefaults.ItemPadding) .padding(start = if (indented) 16.dp else 0.dp), diff --git a/app/src/main/java/com/runicgateway/app/ui/theme/Theme.kt b/app/src/main/java/com/runicgateway/app/ui/theme/Theme.kt index 35568bd..5439f7c 100644 --- a/app/src/main/java/com/runicgateway/app/ui/theme/Theme.kt +++ b/app/src/main/java/com/runicgateway/app/ui/theme/Theme.kt @@ -36,7 +36,14 @@ internal fun shardColorScheme(palette: ShardPalette): ColorScheme = darkColorSch onSurfaceVariant = palette.muted, surfaceContainer = palette.elevated, surfaceContainerHigh = palette.elevated, + // Material's filled Card takes its container from surfaceContainerHighest — + // FilledCardTokens.ContainerColor, checked in the 1.3.0 artifact's bytecode. + // Leaving it unmapped is what made every ShardCard draw in darkColorScheme()'s + // default grey instead of --panel-flat, on themed AND untouched instances alike + // (found on device in phase 8's AC-5 walk; see "Phase 8 as landed"). + surfaceContainerHighest = palette.elevated, surfaceContainerLow = palette.surface, + surfaceContainerLowest = palette.surface, outline = palette.outline, outlineVariant = palette.divider, secondaryContainer = palette.pillBg, // neutral chips / selected drawer item diff --git a/app/src/test/java/com/runicgateway/app/ui/theme/ShardColorSchemeTest.kt b/app/src/test/java/com/runicgateway/app/ui/theme/ShardColorSchemeTest.kt index 2c6949a..9ed232f 100644 --- a/app/src/test/java/com/runicgateway/app/ui/theme/ShardColorSchemeTest.kt +++ b/app/src/test/java/com/runicgateway/app/ui/theme/ShardColorSchemeTest.kt @@ -57,16 +57,53 @@ class ShardColorSchemeTest { onErrorContainer = ShardDanger, ) + /** + * The roles phase 8 deliberately moves off Material's defaults, and the values + * they move to. + * + * `surfaceContainerHighest` is the one that matters: it is + * `FilledCardTokens.ContainerColor`, so it is what every `ShardCard` paints with. + * Leaving it unmapped meant all 26 of them drew in `darkColorScheme()`'s grey + * rather than `--panel-a` — on themed instances *and* on untouched ones, which is + * why this is a visible change to the shipped app and not only a theming fix. It + * had been that way since M5; the AC-5 walk in phase 8 is what surfaced it, + * because M12 themed everything around the cards and left them behind. + * + * `surfaceContainerLowest` has no reader in this app today (the phase 8 sweep + * checked every Material component the app draws) and is mapped for consistency + * with `surfaceContainerLow`, not to fix anything. + * + * Everything else stays exactly where it was — that is what the test below is for. + */ + private val deliberatelyChanged = mapOf( + "surfaceContainerHighest" to ShardElevated, + "surfaceContainerLowest" to ShardSurface, + ) + @Test - fun `the shipped palette reproduces the pre-M12 color scheme exactly`() { - assertEquals(roles(preM12Scheme), roles(shardColorScheme(ShardPalette.Shipped))) + fun `the shipped palette reproduces the pre-M12 color scheme but for the card container`() { + assertPreM12ApartFromTheCardContainer(shardColorScheme(ShardPalette.Shipped)) } /** The same claim from the other end: an absent theme map is the shipped app. */ @Test fun `an absent theme map reproduces the pre-M12 color scheme`() { - val resolved = shardColorScheme(ShardPalette.resolve(emptyMap())) - assertEquals(roles(preM12Scheme), roles(resolved)) + assertPreM12ApartFromTheCardContainer(shardColorScheme(ShardPalette.resolve(emptyMap()))) + } + + /** + * Every role but [deliberatelyChanged] is byte-for-byte the pre-M12 value, and + * each of those really did move — asserting the new value alone would still pass + * if Material's default happened to equal it. + */ + private fun assertPreM12ApartFromTheCardContainer(actual: ColorScheme) { + val before = roles(preM12Scheme) + val after = roles(actual) + assertEquals(before - deliberatelyChanged.keys, after - deliberatelyChanged.keys) + for ((role, expected) in deliberatelyChanged) { + assertEquals("$role should follow the palette", expected, after[role]) + assertNotEquals("$role was already the palette's value", expected, before[role]) + } } /** Sanity: the comparison is capable of failing, and covers the whole scheme. */ @@ -97,6 +134,22 @@ class ShardColorSchemeTest { assertEquals(ShardCta, roles["primary"]) // untouched by these two tokens } + /** + * The phase 8 fix, stated as the thing a shard operator actually sees: set + * `--panel-flat` and the app's cards follow. This is the assertion that would have + * failed before the AC-5 walk, when `surfaceContainerHighest` — Material's filled + * `Card` container — was left at `darkColorScheme()`'s grey. + */ + @Test + fun `--panel-flat reaches the Material card container`() { + val roles = roles(shardColorScheme(ShardPalette.resolve(mapOf("--panel-flat" to "#1f160d")))) + val panel = Color(0xFF1F160D) + assertEquals(panel, roles["surfaceContainerHighest"]) // CardDefaults.cardColors() + assertEquals(panel, roles["surfaceVariant"]) + assertEquals(panel, roles["surfaceContainer"]) + assertEquals(panel, roles["surfaceContainerHigh"]) + } + /** * Every color role of a scheme, by name. `Color` is a value class, so the * roles are the `long`-returning getters and their names carry Kotlin's diff --git a/sonar-project.properties b/sonar-project.properties index e22a60b..a429479 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -32,10 +32,17 @@ sonar.coverage.jacoco.xmlReportPaths=app/build/reports/jacoco/jacocoTestReport/j # tests), and Android-framework glue (Keystore-backed stores, foreground push service, # notifications, Hilt modules). Testable logic — ViewModels, repositories, DTOs, and # pure core/ code — stays measured. See docs/android/COVERAGE_PLAN.md §1. +# +# ui/theme/ is excluded FILE BY FILE, not as a directory. It held only constants and +# composables when COVERAGE_PLAN.md §2 phase 0 drew the list; M12 added three pure +# resolvers to it (ShardPalette, ShardStructure, ShardTypeface) which are the +# milestone's core logic and are covered 98–100%. A `ui/theme/**` glob would drop them +# out of the denominator and hide a future regression in them. Theme.kt is the one +# composable left in the directory. sonar.coverage.exclusions=\ app/src/main/java/**/ui/**/*Screen.kt,\ app/src/main/java/**/ui/**/*Screen*.kt,\ - app/src/main/java/**/ui/theme/**,\ + app/src/main/java/**/ui/theme/Theme.kt,\ app/src/main/java/**/ui/components/**,\ app/src/main/java/**/ui/page/BlockRenderer.kt,\ app/src/main/java/**/ui/shard/ShardComponents.kt,\