From cee39b37515f81d3f81cbc31502ea9333aa27c38 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 09:54:22 +0200 Subject: [PATCH] fix(client/android): two spacing slips in the profile UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name field in "New profile" sat flush against its caption — the `Column` holding them had no arrangement at all, so the explanatory line read as part of the input. And a profile's scope chip didn't line up with the chips beside it: the accent dot was in `FilterChip`'s `leadingIcon` slot, which reserves an 18dp icon and shrinks the chip's leading padding to suit. A 10dp dot in that slot left the chip's insets visibly different from "Default settings" next to it. The dot now rides inside the label, so every chip keeps the same padding and the gap to the name is ours to set rather than a side effect. Both were eyeball-only surfaces: no screenshot covered either. The name field now has one — the dialog itself can't be captured (a focused text field inside a Dialog window never reaches idle under Robolectric, the same trap the PIN scene documents), so its body is extracted and the scene renders exactly that, in both the normal and duplicate-name states. The chip row was already in the profile-scope shot. --- .../kotlin/io/unom/punktfunk/ProfileUi.kt | 83 ++++++++++++------- .../punktfunk/screenshots/ScreenshotTest.kt | 3 + .../unom/punktfunk/screenshots/ShotScenes.kt | 19 +++++ 3 files changed, 74 insertions(+), 31 deletions(-) diff --git a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ProfileUi.kt b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ProfileUi.kt index a076fa84..3ad8a869 100644 --- a/clients/android/app/src/main/kotlin/io/unom/punktfunk/ProfileUi.kt +++ b/clients/android/app/src/main/kotlin/io/unom/punktfunk/ProfileUi.kt @@ -8,7 +8,9 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons @@ -77,9 +79,19 @@ internal fun ProfileScopeChips( FilterChip( selected = selectedId == p.id, onClick = { onSelect(p.id) }, - label = { Text(p.name) }, - leadingIcon = p.accent?.let { accent -> - { AccentDot(accentColor(accent) ?: MaterialTheme.colorScheme.primary) } + // The accent dot rides INSIDE the label, not in the `leadingIcon` slot: that slot + // reserves an 18dp icon and shrinks the chip's leading padding to suit, so a + // profile with an accent would sit differently from one without and from + // "Default settings" beside it. In the label every chip keeps the same padding and + // the dot's spacing is ours to set. + label = { + Row(verticalAlignment = Alignment.CenterVertically) { + accentColor(p.accent)?.let { dot -> + AccentDot(dot, size = 8) + Spacer(Modifier.width(8.dp)) + } + Text(p.name) + } }, ) } @@ -127,33 +139,7 @@ internal fun ProfileNameDialog( AlertDialog( onDismissRequest = onDismiss, title = { Text(title) }, - text = { - Column { - OutlinedTextField( - value = name, - onValueChange = { name = it }, - label = { Text("Name") }, - placeholder = { Text("e.g. Game, Work, Travel") }, - singleLine = true, - isError = duplicate, - ) - Text( - if (duplicate) { - "A profile called “$trimmed” already exists." - } else { - "A profile starts out inheriting every default setting. Whatever you " + - "change while it's selected becomes an override; everything else " + - "keeps following the defaults." - }, - style = MaterialTheme.typography.bodySmall, - color = if (duplicate) { - MaterialTheme.colorScheme.error - } else { - MaterialTheme.colorScheme.onSurfaceVariant - }, - ) - } - }, + text = { ProfileNameFields(name, duplicate) { name = it } }, confirmButton = { TextButton( enabled = trimmed.isNotEmpty() && !duplicate, @@ -164,6 +150,41 @@ internal fun ProfileNameDialog( ) } +/** + * The name field and its caption. Extracted from [ProfileNameDialog] so the screenshot harness can + * render exactly these — a focused text field inside a Dialog window never reaches idle under + * Robolectric, so the dialog itself is uncapturable, and an eyeballed-only layout is how this + * shipped with the field and its caption touching. + */ +@Composable +internal fun ProfileNameFields(name: String, duplicate: Boolean, onNameChange: (String) -> Unit) { + Column(verticalArrangement = Arrangement.spacedBy(10.dp)) { + OutlinedTextField( + value = name, + onValueChange = onNameChange, + label = { Text("Name") }, + placeholder = { Text("e.g. Game, Work, Travel") }, + singleLine = true, + isError = duplicate, + ) + Text( + if (duplicate) { + "A profile called “${name.trim()}” already exists." + } else { + "A profile starts out inheriting every default setting. Whatever you " + + "change while it's selected becomes an override; everything else " + + "keeps following the defaults." + }, + style = MaterialTheme.typography.bodySmall, + color = if (duplicate) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + ) + } +} + /** * Deleting a profile is not destructive to anything but the profile — a host bound to it falls * back to the default settings and a card pinned to it disappears, neither of which is an error. @@ -181,7 +202,7 @@ internal fun DeleteProfileDialog( onDismissRequest = onDismiss, title = { Text("Delete “${profile.name}”?") }, text = { - Column { + Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { val consequences = buildList { if (boundHosts > 0) { add("$boundHosts ${plural(boundHosts, "host", "hosts")} will fall back to the default settings") diff --git a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ScreenshotTest.kt b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ScreenshotTest.kt index af300b58..3fe894ea 100644 --- a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ScreenshotTest.kt +++ b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ScreenshotTest.kt @@ -112,6 +112,9 @@ class ScreenshotTest { TrustDialog() } + @Test + fun newProfile() = shootRoot("new-profile") { NewProfileScene() } + @Test fun speedTest() = shootScreen("speed-test") { HostsScene() diff --git a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt index 822c9f3a..ec633a1e 100644 --- a/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt +++ b/clients/android/app/src/test/kotlin/io/unom/punktfunk/screenshots/ShotScenes.kt @@ -37,6 +37,7 @@ import io.unom.punktfunk.SettingsCategory import io.unom.punktfunk.SettingsScreen import io.unom.punktfunk.StatsOverlay import io.unom.punktfunk.StatsVerbosity +import io.unom.punktfunk.ProfileNameFields import io.unom.punktfunk.ProfileStore import io.unom.punktfunk.SettingsOverlay import io.unom.punktfunk.SpeedTestDialog @@ -244,6 +245,24 @@ internal fun SpeedTestScene() { ) } +/** + * Creating a profile. Small, but it is the first thing a user meets when they reach for this + * feature — and dialogs only get a shot each because a layout slip inside one is invisible from + * every other scene (this one shipped with the field and its caption touching). + */ +@Composable +internal fun NewProfileScene() { + Surface(Modifier.fillMaxSize(), color = MaterialTheme.colorScheme.background) { + Column(Modifier.padding(24.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + Text("New profile", style = MaterialTheme.typography.headlineSmall) + // The dialog's own body, not a rebuild of it — the spacing under test is the real one. + ProfileNameFields(name = "Travel", duplicate = false, onNameChange = {}) + Text("Duplicate name", style = MaterialTheme.typography.headlineSmall) + ProfileNameFields(name = "Game", duplicate = true, onNameChange = {}) + } + } +} + /** The real TOFU AlertDialog (mirrors ConnectScreen's PendingTrust.Kind.TRUST_NEW), shown over the host grid. */ @Composable internal fun TrustDialog() {