RunicGateway/Android-app
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 11 Wiki Activity

fix(theme): let the shard's panel color and pill radius reach Material's defaults (M12 phase 8)

Browse Source 
Two defects found on device by phase 8's AC-5 walk, both the same trap phase 2
hit with card elevation: Material takes these values as DEFAULT ARGUMENTS, not
from the theme, so mapping the token is not enough on its own.

1. Every ShardCard drew in Material's grey, not the shard's panel color.
   CardDefaults.cardColors() takes its container from surfaceContainerHighest -
   FilledCardTokens.ContainerColor, checked in the material3 1.3.0 artifact's
   bytecode - and shardColorScheme mapped surfaceContainer, High and Low but not
   Highest. All 26 ShardCard sites across 20 files were affected. Themed
   instances showed it worst: on Fantasy the page went brown and the cards
   stayed grey.

   This is NOT an M12 regression. The untouched app draws the same grey cards
   and has since M5; M12 only made it obvious by theming everything around them.
   Fixing it therefore changes the untouched app too - cards move from Material's
   grey to --panel-flat - which is the milestone's second deliberate change to a
   shard that has set nothing, alongside phase 2's card shadow. AC-1 is updated
   to record that rather than absorb it: every other role is still asserted
   byte-for-byte against the verbatim pre-M12 scheme, and the two that moved are
   named, given their new values, and checked to have actually differed before.

   surfaceContainerLowest is mapped alongside it for consistency with
   surfaceContainerLow. It has no reader in this app - the phase 8 sweep checked
   every Material component the app draws against the roles the mapping leaves at
   Material defaults, and surfaceContainerHighest was the only live one. The
   drawer scrim reads the unmapped `scrim`, which stays Material's black
   deliberately.

2. The drawer's selected row ignored --radius-pill. NavigationDrawerItem takes
   `shape` as a default argument (CircleShape); the three call sites set `colors`
   but never `shape`, so on Fantasy every other radius went square while the
   selected row stayed fully round.

Verified on device against a Fantasy-themed local instance: the three ShardCards
on the shard screen now paint --panel-flat, and the selected drawer row is the
4px rectangle the preset asks for.

477 unit tests green (476 + 1), lintDebug clean.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
claude
2026-08-08 11:00:06 -05:00
parent c14342aa51
commit b84a973559
3 changed files with 72 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
app/src/main/java/com/runicgateway/app/ui/RunicApp.kt
View File

@@ -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),

7
app/src/main/java/com/runicgateway/app/ui/theme/Theme.kt
View File

@@ -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

61
app/src/test/java/com/runicgateway/app/ui/theme/ShardColorSchemeTest.kt
View File

@@ -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