From 3edd45d5f429262cb30072ba3b34735d1bee852a Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sat, 8 Aug 2026 06:13:16 -0500 Subject: [PATCH] feat(brand): draw the shard's logo and hero (M12 phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `brand.logo` and `brand.hero` have ridden in `BrandDto` since M1 and neither has ever been drawn — the app spells the instance out in text everywhere the website shows a mark. Phase 4 renders them on the three surfaces §5.6 names: the logo above the name in the drawer header, the logo in place of the uppercased title in the top bar, and the hero as a band above Home's title block. Nothing new is fetched. `LocalAssetResolver` already turns a site-relative `/uploads/…` path into an absolute URL and Coil is already a dependency, so this phase is entirely presentation. The rule that governs the file is §5.6's: an empty slot renders nothing — not a placeholder, not a reserved gap. Every size modifier hangs off the image itself, so when the image is not composed neither is its padding, and a caller that wants space below a hero passes `Modifier.padding` instead of a sibling `Spacer`. A failed load is an empty slot: no broken-image icon, no retry. The top bar is the one place where "empty" is not "nothing". The logo replaces the title there, so a 404 would strand the app in an unnamed shell until the next resume refresh; it falls back to the text, which is what empty already showed. There is no fallback while the load is in flight — drawing the text first would flash text to logo on every navigation for one frame. The hero is a fixed 180dp band, cropped, rather than the intrinsic aspect the app's other images draw at. The website's hero is a CSS background driven by `hero_layout`, which the app does not port, and the website's default hero is a square emblem — at the intrinsic aspect an uploaded square would be a ~360dp block that pushes the status card off the first screenful. It clips to `shapes.medium`, so it follows `--radius-card` like every other surface. The logo carries a content description only in the top bar, where it stands alone; beside the name in text it is decorative, the same call the website's `alt=''` makes. 410 unit tests green (401 + 9), `lintDebug` and `assembleDebug` clean. The drawing itself is out of reach for JVM tests — the app carries no Robolectric, so a composable body cannot run — but the decision of *whether* to draw is pure, and `brandAssetUrl` is pulled out so it can be pinned. Co-Authored-By: Claude --- .../java/com/runicgateway/app/ui/RunicApp.kt | 34 +++- .../app/ui/components/BrandAssets.kt | 157 ++++++++++++++++++ .../runicgateway/app/ui/home/HomeScreen.kt | 7 + .../app/ui/components/BrandAssetsTest.kt | 104 ++++++++++++ 4 files changed, 295 insertions(+), 7 deletions(-) create mode 100644 app/src/main/java/com/runicgateway/app/ui/components/BrandAssets.kt create mode 100644 app/src/test/java/com/runicgateway/app/ui/components/BrandAssetsTest.kt 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 ddcc9a0..3be465b 100644 --- a/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt +++ b/app/src/main/java/com/runicgateway/app/ui/RunicApp.kt @@ -54,6 +54,7 @@ import com.runicgateway.app.ui.auth.LoginScreen import com.runicgateway.app.ui.auth.RecoveryCodesScreen import com.runicgateway.app.ui.auth.TrustedDevicesScreen import com.runicgateway.app.ui.auth.roleLabelRes +import com.runicgateway.app.ui.components.BrandLogo import com.runicgateway.app.ui.contact.ContactScreen import com.runicgateway.app.ui.home.HomeScreen import com.runicgateway.app.ui.navigation.APP_MENU @@ -161,6 +162,14 @@ fun RunicApp( // unreachable. See RunicGateway M10. Column(Modifier.verticalScroll(rememberScrollState())) { Spacer(Modifier.height(12.dp)) + // The instance's logo above its name (§5.6). Decorative — the name + // is the very next line — and absent on an instance that uploaded + // none, in which case the header is exactly what it was before M12. + BrandLogo( + logo = brand?.logo, + height = 32.dp, + modifier = Modifier.padding(start = 24.dp, end = 24.dp, bottom = 4.dp), + ) Text( text = brand?.name?.takeIf { it.isNotBlank() } ?: stringResource(R.string.app_name), style = MaterialTheme.typography.titleLarge, @@ -230,13 +239,24 @@ fun RunicApp( actionIconContentColor = MaterialTheme.colorScheme.onSurface, ), title = { - Text( - text = (brand?.name?.takeIf { it.isNotBlank() } - ?: stringResource(R.string.app_name)).uppercase(), - style = MaterialTheme.typography.titleSmall.copy(letterSpacing = 1.2.sp), - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) + val name = brand?.name?.takeIf { it.isNotBlank() } + ?: stringResource(R.string.app_name) + // The logo stands in for the title here, so unlike the drawer's + // it is named for a screen reader — and it falls back to the + // text when the instance has no logo or the load fails (§5.6). + BrandLogo( + logo = brand?.logo, + height = 24.dp, + contentDescription = name, + ) { + Text( + text = name.uppercase(), + style = MaterialTheme.typography.titleSmall + .copy(letterSpacing = 1.2.sp), + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } }, navigationIcon = { if (isTopLevel) { diff --git a/app/src/main/java/com/runicgateway/app/ui/components/BrandAssets.kt b/app/src/main/java/com/runicgateway/app/ui/components/BrandAssets.kt new file mode 100644 index 0000000..1effaa6 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/ui/components/BrandAssets.kt @@ -0,0 +1,157 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.components + +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import coil.compose.AsyncImage +import com.runicgateway.app.ui.LocalAssetResolver + +/** + * The two brand assets an instance can upload — the logo and the hero + * (THEMING_AND_NAV.md §5.6, M12 phase 4). Both have ridden in `BrandDto` since + * M1 and neither has ever been drawn; the app has always spelled the instance + * out in text wherever the website shows a mark. + * + * **The rule that governs this whole file: an empty slot renders nothing.** Not + * a placeholder, not a reserved gap, not the app's own emblem — an instance + * that has uploaded no logo must lay out exactly as it did before this phase + * existed, which is §2 applied to assets. The website's `BrandLogo.jsx` opens + * with the same `if (!brand.logo) return null`. + * + * **A failed load is an empty slot.** No broken-image icon and no retry: an + * asset that 404s, or that can't be reached because the shard is down, must + * degrade to the same layout as an instance that never uploaded one. That is + * why nothing here reserves its space up front — every size modifier hangs off + * the image itself, so when the image isn't composed neither is its padding. + * A caller that wants space *below* a hero passes it as `Modifier.padding` + * rather than a sibling `Spacer`, and gets both cases right for free. + */ + +/** + * Widest a logo may draw, as a multiple of its height. Mirrors the website's + * `maxWidth: height * 6` — an operator who uploads a long wordmark gets it + * scaled down rather than pushing the drawer header or the top bar's title out + * of shape. + */ +private const val LOGO_MAX_ASPECT = 6f + +/** The Home hero's band height (§5.6, phase 4). See [BrandHero] for why it's fixed. */ +private val HERO_HEIGHT = 180.dp + +/** + * The instance's uploaded logo at [height], or [fallback] when there is none. + * + * [fallback] defaults to drawing nothing, which is what the drawer header wants: + * the instance name sits directly below it, so an instance with no logo simply + * has the name where it has always been. The top bar passes the name itself, + * because there the logo *replaces* the title — leaving that blank on a failed + * load would strand the app in an unnamed shell until the next resume refresh, + * and "a failed load is an empty slot" means the slot falls back to whatever + * empty would have shown, which for the top bar is the text. + * + * There is deliberately no fallback while the load is still in flight. Drawing + * the text first would flash text → logo on every navigation for the sake of + * one frame, since Coil serves the second and later reads from its memory cache. + * + * Pass [contentDescription] only where the logo stands alone. Beside or above + * the name in text it is decorative, and describing it would have a screen + * reader say the instance's name twice — the same call the website's `alt=''` + * makes. + */ +@Composable +fun BrandLogo( + logo: String?, + height: Dp, + modifier: Modifier = Modifier, + contentDescription: String? = null, + fallback: @Composable () -> Unit = {}, +) { + val url = brandAssetUrl(logo, LocalAssetResolver.current) + // Keyed on the url so a refreshed appearance that swaps the logo (§5.5) gets + // a fresh attempt rather than inheriting the old one's failure. + var failed by remember(url) { mutableStateOf(false) } + + if (url == null || failed) { + fallback() + return + } + AsyncImage( + model = url, + contentDescription = contentDescription, + contentScale = ContentScale.Fit, + onError = { failed = true }, + modifier = modifier + .height(height) + .widthIn(max = height * LOGO_MAX_ASPECT), + ) +} + +/** + * The instance's hero image as a full-width band above Home's title block, or + * nothing when there is none. + * + * **Fixed height and cropped**, rather than the intrinsic aspect ratio the app's + * other images (`PostScreen`, `BlockRenderer`) draw at. The website's hero is a + * CSS background driven by `hero_layout`, which the app does not port, so the + * app needs its own rule — and the website's *default* hero is a square emblem, + * so an uploaded square is a case to expect rather than an edge one. At the + * intrinsic aspect that square would be a ~360dp block that pushes the status + * card off the first screenful; cropped to a band, a wide banner and a square + * both give the same frame above the title. + * + * Clipped to `shapes.medium`, so the hero follows the shard's `--radius-card` + * like every other surface the admin can round off (§5.2). + * + * Decorative: Home spells the instance's name and tagline out in text directly + * below, so the hero carries no content description. + */ +@Composable +fun BrandHero(hero: String?, modifier: Modifier = Modifier) { + val url = brandAssetUrl(hero, LocalAssetResolver.current) + var failed by remember(url) { mutableStateOf(false) } + + if (url == null || failed) return + AsyncImage( + model = url, + contentDescription = null, + contentScale = ContentScale.Crop, + onError = { failed = true }, + modifier = modifier + .fillMaxWidth() + .height(HERO_HEIGHT) + .clip(MaterialTheme.shapes.medium), + ) +} + +/** + * Resolve a brand asset slot to a loadable URL, or null when the slot is empty. + * + * The blank check has to happen on **both** sides of [resolve]: `BrandDto` + * defaults every asset field to `""` rather than null (the server publishes the + * empty string for "not set"), and a resolver given a path it cannot make + * absolute may hand one straight back. Null out of here is the signal for "draw + * nothing", so a blank slipping through would put a zero-size image request in + * the layout instead of no image at all. + * + * Pulled out of the composables purely so it can be tested: the app has no + * Robolectric, so a composable body cannot run in a JVM unit test, but this rule + * is the whole of §5.6's "renders nothing when unset" and it is worth pinning. + */ +internal fun brandAssetUrl(path: String?, resolve: (String?) -> String?): String? = + path?.takeIf { it.isNotBlank() } + ?.let(resolve) + ?.takeIf { it.isNotBlank() } diff --git a/app/src/main/java/com/runicgateway/app/ui/home/HomeScreen.kt b/app/src/main/java/com/runicgateway/app/ui/home/HomeScreen.kt index d841497..3fecd9e 100644 --- a/app/src/main/java/com/runicgateway/app/ui/home/HomeScreen.kt +++ b/app/src/main/java/com/runicgateway/app/ui/home/HomeScreen.kt @@ -26,6 +26,7 @@ import com.runicgateway.app.R import com.runicgateway.app.data.api.dto.BrandDto import com.runicgateway.app.data.api.dto.StatusDto import com.runicgateway.app.ui.UiState +import com.runicgateway.app.ui.components.BrandHero import com.runicgateway.app.ui.components.ErrorView import com.runicgateway.app.ui.components.FeatureCard import com.runicgateway.app.ui.components.LoadingView @@ -59,6 +60,12 @@ private fun HomeContent(brand: BrandDto?, status: StatusDto, modifier: Modifier .verticalScroll(rememberScrollState()) .padding(20.dp), ) { + // The instance's hero above the title block (§5.6) — Home is the one screen + // with a hero-shaped space. Its bottom gap rides on the image's own modifier + // rather than a Spacer, so an instance with no hero (or one whose hero fails + // to load) opens on the title exactly where it has always been. + BrandHero(hero = brand?.hero, modifier = Modifier.padding(bottom = 16.dp)) + Text( text = brand?.name?.takeIf { it.isNotBlank() } ?: stringResource(R.string.app_name), style = MaterialTheme.typography.headlineMedium, diff --git a/app/src/test/java/com/runicgateway/app/ui/components/BrandAssetsTest.kt b/app/src/test/java/com/runicgateway/app/ui/components/BrandAssetsTest.kt new file mode 100644 index 0000000..71c23c3 --- /dev/null +++ b/app/src/test/java/com/runicgateway/app/ui/components/BrandAssetsTest.kt @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.ui.components + +import com.runicgateway.app.data.api.dto.BrandDto +import com.runicgateway.app.data.appearance.SiteAppearance +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * §5.6's one testable rule: **an empty slot resolves to nothing.** The drawing + * itself is out of reach here — the app carries no Robolectric, so a composable + * body cannot run in a JVM test and phase 4's layout is AC-5's job — but the + * decision of whether to draw at all is pure, and it is the decision that keeps + * an unbranded instance laying out as it did before M12. + * + * The resolver is faked as the absolute-URL join the real one performs + * (`AppViewModel.resolveAsset`, unchanged by this phase), so these assert + * [brandAssetUrl]'s own contract rather than re-testing the network layer. + */ +class BrandAssetsTest { + + private val resolve: (String?) -> String? = { path -> + when { + path.isNullOrBlank() -> null + path.startsWith("http") -> path + else -> "https://shard.example${if (path.startsWith("/")) "" else "/"}$path" + } + } + + // --- the empty slot: every shape "not set" arrives in ------------------ + + @Test + fun `a null slot resolves to nothing`() { + assertNull(brandAssetUrl(null, resolve)) + } + + @Test + fun `an empty slot resolves to nothing`() { + // The server publishes "" for an asset that was never uploaded, and BrandDto + // defaults to it — this is the case that carries the untouched instance. + assertNull(brandAssetUrl("", resolve)) + } + + @Test + fun `a whitespace-only slot resolves to nothing`() { + assertNull(brandAssetUrl(" ", resolve)) + } + + @Test + fun `the shipped brand has neither a logo nor a hero`() { + // AC-1 for phase 4: nothing about a default BrandDto puts an image on screen. + val brand = BrandDto() + assertNull(brandAssetUrl(brand.logo, resolve)) + assertNull(brandAssetUrl(brand.hero, resolve)) + } + + @Test + fun `a failed settings load leaves no brand to draw`() { + // SiteAppearance.NONE is what a dead backend produces (§2). It has no brand + // at all, so both slots are absent rather than empty. + val brand: BrandDto? = SiteAppearance.NONE.brand + assertNull(brand) + assertNull(brandAssetUrl(brand?.logo, resolve)) + assertNull(brandAssetUrl(brand?.hero, resolve)) + } + + // --- the filled slot --------------------------------------------------- + + @Test + fun `a site-relative upload resolves against the shard's base`() { + assertEquals( + "https://shard.example/uploads/brand/logo.png", + brandAssetUrl("/uploads/brand/logo.png", resolve), + ) + } + + @Test + fun `an absolute URL passes through`() { + // BRAND_LOGO may be set to an off-site URL; the resolver leaves those alone. + assertEquals( + "https://cdn.example/logo.svg", + brandAssetUrl("https://cdn.example/logo.svg", resolve), + ) + } + + // --- the second blank check ------------------------------------------- + + @Test + fun `a resolver that returns nothing resolves to nothing`() { + // No base URL configured yet: the real resolver hands the path back or gives + // up. Either way the slot must not become an image request. + assertNull(brandAssetUrl("/uploads/brand/logo.png") { null }) + } + + @Test + fun `a resolver that returns blank resolves to nothing`() { + // Why the blank check is on both sides of the resolver, not just the input. + assertNull(brandAssetUrl("/uploads/brand/logo.png") { "" }) + assertNull(brandAssetUrl("/uploads/brand/logo.png") { " " }) + } +}