feat(theme): make the app a full consumer of the shard's admin theming and nav (M12 cutover) #41
@@ -94,6 +94,7 @@ import com.runicgateway.app.ui.shard.MarketVendorScreen
|
|||||||
import com.runicgateway.app.ui.shard.RulesScreen
|
import com.runicgateway.app.ui.shard.RulesScreen
|
||||||
import com.runicgateway.app.ui.shard.ShardBoard
|
import com.runicgateway.app.ui.shard.ShardBoard
|
||||||
import com.runicgateway.app.ui.shard.ShardScreen
|
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.WikiPageScreen
|
||||||
import com.runicgateway.app.ui.wiki.WikiScreen
|
import com.runicgateway.app.ui.wiki.WikiScreen
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
@@ -263,6 +264,7 @@ fun RunicApp(
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
colors = drawerItemColors,
|
colors = drawerItemColors,
|
||||||
|
shape = LocalShardStructure.current.pill,
|
||||||
modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding),
|
modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding),
|
||||||
)
|
)
|
||||||
NavigationDrawerItem(
|
NavigationDrawerItem(
|
||||||
@@ -273,6 +275,7 @@ fun RunicApp(
|
|||||||
onChangeServer()
|
onChangeServer()
|
||||||
},
|
},
|
||||||
colors = drawerItemColors,
|
colors = drawerItemColors,
|
||||||
|
shape = LocalShardStructure.current.pill,
|
||||||
modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding),
|
modifier = Modifier.padding(NavigationDrawerItemDefaults.ItemPadding),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -383,6 +386,11 @@ private fun NavRow(
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
colors = colors,
|
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
|
modifier = Modifier
|
||||||
.padding(NavigationDrawerItemDefaults.ItemPadding)
|
.padding(NavigationDrawerItemDefaults.ItemPadding)
|
||||||
.padding(start = if (indented) 16.dp else 0.dp),
|
.padding(start = if (indented) 16.dp else 0.dp),
|
||||||
|
|||||||
@@ -36,7 +36,14 @@ internal fun shardColorScheme(palette: ShardPalette): ColorScheme = darkColorSch
|
|||||||
onSurfaceVariant = palette.muted,
|
onSurfaceVariant = palette.muted,
|
||||||
surfaceContainer = palette.elevated,
|
surfaceContainer = palette.elevated,
|
||||||
surfaceContainerHigh = 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,
|
surfaceContainerLow = palette.surface,
|
||||||
|
surfaceContainerLowest = palette.surface,
|
||||||
outline = palette.outline,
|
outline = palette.outline,
|
||||||
outlineVariant = palette.divider,
|
outlineVariant = palette.divider,
|
||||||
secondaryContainer = palette.pillBg, // neutral chips / selected drawer item
|
secondaryContainer = palette.pillBg, // neutral chips / selected drawer item
|
||||||
|
|||||||
@@ -57,16 +57,53 @@ class ShardColorSchemeTest {
|
|||||||
onErrorContainer = ShardDanger,
|
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
|
@Test
|
||||||
fun `the shipped palette reproduces the pre-M12 color scheme exactly`() {
|
fun `the shipped palette reproduces the pre-M12 color scheme but for the card container`() {
|
||||||
assertEquals(roles(preM12Scheme), roles(shardColorScheme(ShardPalette.Shipped)))
|
assertPreM12ApartFromTheCardContainer(shardColorScheme(ShardPalette.Shipped))
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The same claim from the other end: an absent theme map is the shipped app. */
|
/** The same claim from the other end: an absent theme map is the shipped app. */
|
||||||
@Test
|
@Test
|
||||||
fun `an absent theme map reproduces the pre-M12 color scheme`() {
|
fun `an absent theme map reproduces the pre-M12 color scheme`() {
|
||||||
val resolved = shardColorScheme(ShardPalette.resolve(emptyMap()))
|
assertPreM12ApartFromTheCardContainer(shardColorScheme(ShardPalette.resolve(emptyMap())))
|
||||||
assertEquals(roles(preM12Scheme), roles(resolved))
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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. */
|
/** 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
|
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
|
* 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
|
* roles are the `long`-returning getters and their names carry Kotlin's
|
||||||
|
|||||||
@@ -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,
|
# tests), and Android-framework glue (Keystore-backed stores, foreground push service,
|
||||||
# notifications, Hilt modules). Testable logic — ViewModels, repositories, DTOs, and
|
# notifications, Hilt modules). Testable logic — ViewModels, repositories, DTOs, and
|
||||||
# pure core/ code — stays measured. See docs/android/COVERAGE_PLAN.md §1.
|
# 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=\
|
sonar.coverage.exclusions=\
|
||||||
app/src/main/java/**/ui/**/*Screen.kt,\
|
app/src/main/java/**/ui/**/*Screen.kt,\
|
||||||
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/components/**,\
|
||||||
app/src/main/java/**/ui/page/BlockRenderer.kt,\
|
app/src/main/java/**/ui/page/BlockRenderer.kt,\
|
||||||
app/src/main/java/**/ui/shard/ShardComponents.kt,\
|
app/src/main/java/**/ui/shard/ShardComponents.kt,\
|
||||||
|
|||||||
Reference in New Issue
Block a user