fix: UX improvements from audit

- Remove dead-end back arrow from dashboard; sites and instances are accessible via globe and swap icons in the top bar - Replace gear icon with swap icon (SwapHoriz) for switching instances - Require successful connection test before enabling Save - Add API key visibility toggle on setup form - Add delete confirmation dialog on site list - Fix chart double-modifier bug (height applied twice) - Add empty states for chart ("No visitor data") and dimension sections ("No data for this period") instead of silently hiding Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-18 17:34:08 +01:00 · 2026-03-18 17:34:08 +01:00 · 8d92720d63
commit 8d92720d63
parent 3e0ad6c350
6 changed files with 109 additions and 92 deletions
--- a/app/src/main/java/no/naiv/implausibly/ui/dashboard/DashboardScreen.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/dashboard/DashboardScreen.kt
@ -8,9 +8,8 @@ import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.material.icons.Icons
-import androidx.compose.material.icons.automirrored.filled.ArrowBack
 import androidx.compose.material.icons.filled.Language
-import androidx.compose.material.icons.filled.Settings
+import androidx.compose.material.icons.filled.SwapHoriz
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.Icon
 import androidx.compose.material3.IconButton
@ -42,9 +41,9 @@ import no.naiv.implausibly.ui.dashboard.sections.TopSourcesSection
@OptIn(ExperimentalMaterial3Api::class)
@Composable
 fun DashboardScreen(
-    onBack: () -> Unit,
    onNavigateToSetup: () -> Unit,
    onNavigateToSites: (instanceId: String) -> Unit,
+    onBack: () -> Unit = {},
    viewModel: DashboardViewModel = hiltViewModel()
 ) {
    val uiState by viewModel.uiState.collectAsState()
@ -53,17 +52,12 @@ fun DashboardScreen(
        topBar = {
            TopAppBar(
                title = { Text(uiState.siteId) },
-                navigationIcon = {
-                    IconButton(onClick = onBack) {
-                        Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back")
-                    }
-                },
                actions = {
                    IconButton(onClick = { onNavigateToSites(uiState.instanceId) }) {
-                        Icon(Icons.Default.Language, contentDescription = "Sites")
+                        Icon(Icons.Default.Language, contentDescription = "Switch site")
                    }
                    IconButton(onClick = onNavigateToSetup) {
-                        Icon(Icons.Default.Settings, contentDescription = "Instances")
+                        Icon(Icons.Default.SwapHoriz, contentDescription = "Switch instance")
                    }
                }
            )
@ -126,32 +120,13 @@ private fun DashboardContent(
            )
        }

-        item {
-            TopSourcesSection(entries = data.topSources)
-        }
+        item { TopSourcesSection(entries = data.topSources) }
+        item { TopPagesSection(entries = data.topPages) }
+        item { CountriesSection(entries = data.countries) }
+        item { DevicesSection(entries = data.devices) }
+        item { BrowsersSection(entries = data.browsers) }
+        item { OperatingSystemsSection(entries = data.operatingSystems) }

-        item {
-            TopPagesSection(entries = data.topPages)
-        }
-
-        item {
-            CountriesSection(entries = data.countries)
-        }
-
-        item {
-            DevicesSection(entries = data.devices)
-        }
-
-        item {
-            BrowsersSection(entries = data.browsers)
-        }
-
-        item {
-            OperatingSystemsSection(entries = data.operatingSystems)
-        }
-
-        item {
-            Spacer(modifier = Modifier.height(8.dp))
-        }
+        item { Spacer(modifier = Modifier.height(8.dp)) }
    }
 }
--- a/app/src/main/java/no/naiv/implausibly/ui/dashboard/components/VisitorChart.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/dashboard/components/VisitorChart.kt
@ -2,11 +2,15 @@
 package no.naiv.implausibly.ui.dashboard.components

 import androidx.compose.foundation.Canvas
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.material3.Card
 import androidx.compose.material3.MaterialTheme
+import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.graphics.Path
@ -14,12 +18,6 @@ import androidx.compose.ui.graphics.drawscope.Stroke
 import androidx.compose.ui.unit.dp
 import no.naiv.implausibly.domain.model.TimeSeriesPoint

-/**
- * Custom Canvas line chart for visitor time series.
- *
- * Draws a filled area chart with a line on top. No external charting
- * library needed — this is ~80 lines of Canvas drawing code.
- */
@Composable
 fun VisitorChart(
    points: List<TimeSeriesPoint>,
@ -29,13 +27,24 @@ fun VisitorChart(
    val fillColor = MaterialTheme.colorScheme.primary.copy(alpha = 0.15f)

    Card(modifier = modifier.fillMaxWidth()) {
-        if (points.isEmpty()) return@Card
+        if (points.isEmpty()) {
+            Box(
+                modifier = Modifier.fillMaxSize().padding(16.dp),
+                contentAlignment = Alignment.Center
+            ) {
+                Text(
+                    text = "No visitor data for this period",
+                    style = MaterialTheme.typography.bodyMedium,
+                    color = MaterialTheme.colorScheme.onSurfaceVariant
+                )
+            }
+            return@Card
+        }

        Canvas(
            modifier = Modifier
-                .fillMaxWidth()
+                .fillMaxSize()
                .padding(16.dp)
-                .then(modifier)
        ) {
            val maxVisitors = points.maxOf { it.visitors }.coerceAtLeast(1)
            val width = size.width
@ -47,7 +56,6 @@ fun VisitorChart(

            val stepX = if (points.size > 1) chartWidth / (points.size - 1) else chartWidth

-            // Build points
            val chartPoints = points.mapIndexed { index, point ->
                val x = padding + index * stepX
                val y = padding + chartHeight - (point.visitors.toFloat() / maxVisitors * chartHeight)
@ -72,13 +80,9 @@ fun VisitorChart(
            }
            drawPath(linePath, lineColor, style = Stroke(width = 2.dp.toPx()))

-            // Draw dots at each data point
+            // Draw dots
            chartPoints.forEach { point ->
-                drawCircle(
-                    color = lineColor,
-                    radius = 3.dp.toPx(),
-                    center = point
-                )
+                drawCircle(color = lineColor, radius = 3.dp.toPx(), center = point)
            }
        }
    }
--- a/app/src/main/java/no/naiv/implausibly/ui/dashboard/sections/DimensionSection.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/dashboard/sections/DimensionSection.kt
@ -28,10 +28,6 @@ fun DimensionSection(
    entries: List<DimensionEntry>,
    modifier: Modifier = Modifier
 ) {
-    if (entries.isEmpty()) return
-
-    val maxVisitors = entries.maxOf { it.visitors }.coerceAtLeast(1)
-
    Card(modifier = modifier.fillMaxWidth()) {
        Column(modifier = Modifier.padding(16.dp)) {
            Text(
@ -40,15 +36,21 @@ fun DimensionSection(
            )
            Spacer(modifier = Modifier.height(12.dp))

-            entries.forEach { entry ->
-                DimensionRow(
-                    entry = entry,
-                    maxVisitors = maxVisitors
+            if (entries.isEmpty()) {
+                Text(
+                    text = "No data for this period",
+                    style = MaterialTheme.typography.bodyMedium,
+                    color = MaterialTheme.colorScheme.onSurfaceVariant
                )
+            } else {
+                val maxVisitors = entries.maxOf { it.visitors }.coerceAtLeast(1)
+                entries.forEach { entry ->
+                    DimensionRow(entry = entry, maxVisitors = maxVisitors)
                    Spacer(modifier = Modifier.height(8.dp))
                }
            }
        }
+    }
 }

@Composable
--- a/app/src/main/java/no/naiv/implausibly/ui/navigation/AppNavHost.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/navigation/AppNavHost.kt
@ -103,7 +103,6 @@ fun AppNavHost(
            )
        ) {
            DashboardScreen(
-                onBack = { navController.popBackStack() },
                onNavigateToSetup = {
                    navController.navigate(Routes.setup()) {
                        popUpTo(0) { inclusive = true }
--- a/app/src/main/java/no/naiv/implausibly/ui/setup/SetupScreen.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/setup/SetupScreen.kt
@ -10,10 +10,15 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.verticalScroll
+import androidx.compose.material.icons.Icons
+import androidx.compose.material.icons.filled.Visibility
+import androidx.compose.material.icons.filled.VisibilityOff
 import androidx.compose.material3.Button
 import androidx.compose.material3.Card
 import androidx.compose.material3.CircularProgressIndicator
 import androidx.compose.material3.HorizontalDivider
+import androidx.compose.material3.Icon
+import androidx.compose.material3.IconButton
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.OutlinedButton
 import androidx.compose.material3.OutlinedTextField
@ -23,8 +28,12 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 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.text.input.PasswordVisualTransformation
+import androidx.compose.ui.text.input.VisualTransformation
 import androidx.compose.ui.unit.dp
 import androidx.hilt.navigation.compose.hiltViewModel

@ -34,8 +43,8 @@ fun SetupScreen(
    viewModel: SetupViewModel = hiltViewModel()
 ) {
    val uiState by viewModel.uiState.collectAsState()
+    var apiKeyVisible by remember { mutableStateOf(false) }

-    // Navigate when instance is saved
    LaunchedEffect(uiState.savedInstanceId) {
        uiState.savedInstanceId?.let { onInstanceAdded(it) }
    }
@ -116,14 +125,27 @@ fun SetupScreen(

            Spacer(modifier = Modifier.height(12.dp))

-            // API Key
+            // API Key with visibility toggle
            OutlinedTextField(
                value = uiState.apiKey,
                onValueChange = viewModel::onApiKeyChanged,
                label = { Text("API Key (optional)") },
                placeholder = { Text("Leave blank for public dashboards") },
                singleLine = true,
-                visualTransformation = PasswordVisualTransformation(),
+                visualTransformation = if (apiKeyVisible) {
+                    VisualTransformation.None
+                } else {
+                    PasswordVisualTransformation()
+                },
+                trailingIcon = {
+                    IconButton(onClick = { apiKeyVisible = !apiKeyVisible }) {
+                        Icon(
+                            if (apiKeyVisible) Icons.Default.VisibilityOff
+                            else Icons.Default.Visibility,
+                            contentDescription = if (apiKeyVisible) "Hide API key" else "Show API key"
+                        )
+                    }
+                },
                modifier = Modifier.fillMaxWidth()
            )

@ -183,9 +205,7 @@ fun SetupScreen(
                modifier = Modifier.fillMaxWidth()
            ) {
                if (uiState.isTesting) {
-                    CircularProgressIndicator(
-                        modifier = Modifier.height(20.dp)
-                    )
+                    CircularProgressIndicator(modifier = Modifier.height(20.dp))
                } else {
                    Text("Test Connection")
                }
@ -193,18 +213,14 @@ fun SetupScreen(

            Spacer(modifier = Modifier.height(8.dp))

-            // Save button
+            // Save button — requires successful test first
            Button(
                onClick = viewModel::saveInstance,
-                enabled = uiState.baseUrl.isNotBlank()
-                    && uiState.siteId.isNotBlank()
-                    && !uiState.isSaving,
+                enabled = uiState.testResult is TestResult.Success && !uiState.isSaving,
                modifier = Modifier.fillMaxWidth()
            ) {
                if (uiState.isSaving) {
-                    CircularProgressIndicator(
-                        modifier = Modifier.height(20.dp)
-                    )
+                    CircularProgressIndicator(modifier = Modifier.height(20.dp))
                } else {
                    Text("Save & Continue")
                }
--- a/app/src/main/java/no/naiv/implausibly/ui/sites/SiteListScreen.kt
+++ b/app/src/main/java/no/naiv/implausibly/ui/sites/SiteListScreen.kt
@ -21,6 +21,7 @@ import androidx.compose.material.icons.filled.ContentCopy
 import androidx.compose.material.icons.filled.Delete
 import androidx.compose.material.icons.filled.Edit
 import androidx.compose.material.icons.filled.Language
+import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.Card
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.Icon
@ -29,10 +30,14 @@ import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.OutlinedTextField
 import androidx.compose.material3.Scaffold
 import androidx.compose.material3.Text
+import androidx.compose.material3.TextButton
 import androidx.compose.material3.TopAppBar
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.unit.dp
@ -47,11 +52,38 @@ fun SiteListScreen(
    viewModel: SiteListViewModel = hiltViewModel()
 ) {
    val uiState by viewModel.uiState.collectAsState()
+    var siteToDelete by remember { mutableStateOf<String?>(null) }
+
+    // Delete confirmation dialog
+    siteToDelete?.let { siteId ->
+        AlertDialog(
+            onDismissRequest = { siteToDelete = null },
+            title = { Text("Remove site") },
+            text = { Text("Remove $siteId from this instance?") },
+            confirmButton = {
+                TextButton(onClick = {
+                    viewModel.removeSite(siteId)
+                    siteToDelete = null
+                }) {
+                    Text("Remove", color = MaterialTheme.colorScheme.error)
+                }
+            },
+            dismissButton = {
+                TextButton(onClick = { siteToDelete = null }) {
+                    Text("Cancel")
+                }
+            }
+        )
+    }

    Scaffold(
        topBar = {
            TopAppBar(
-                title = { Text(uiState.instanceName.ifEmpty { "Sites" }) },
+                title = {
+                    Column {
+                        Text(uiState.instanceName.ifEmpty { "Sites" })
+                    }
+                },
                navigationIcon = {
                    IconButton(onClick = onBack) {
                        Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back")
@ -74,7 +106,7 @@ fun SiteListScreen(
                OutlinedTextField(
                    value = uiState.newSiteId,
                    onValueChange = viewModel::onNewSiteIdChanged,
-                    label = { Text("Site ID") },
+                    label = { Text("Add site ID") },
                    placeholder = { Text("example.com") },
                    singleLine = true,
                    modifier = Modifier.weight(1f)
@ -114,7 +146,6 @@ fun SiteListScreen(
                            )
                    ) {
                        if (isEditing) {
-                            // Editing mode: inline text field
                            Row(
                                modifier = Modifier
                                    .fillMaxWidth()
@ -135,14 +166,10 @@ fun SiteListScreen(
                                    )
                                }
                                IconButton(onClick = viewModel::cancelEdit) {
-                                    Icon(
-                                        Icons.Default.Close,
-                                        contentDescription = "Cancel"
-                                    )
+                                    Icon(Icons.Default.Close, contentDescription = "Cancel")
                                }
                            }
                        } else {
-                            // Normal mode
                            Row(
                                modifier = Modifier
                                    .fillMaxWidth()
@ -161,18 +188,12 @@ fun SiteListScreen(
                                    modifier = Modifier.weight(1f)
                                )
                                IconButton(onClick = { onCloneSite(uiState.instanceId, site.id) }) {
-                                    Icon(
-                                        Icons.Default.ContentCopy,
-                                        contentDescription = "Clone site"
-                                    )
+                                    Icon(Icons.Default.ContentCopy, contentDescription = "Clone as new instance")
                                }
                                IconButton(onClick = { viewModel.startEditing(site.id) }) {
-                                    Icon(
-                                        Icons.Default.Edit,
-                                        contentDescription = "Edit site"
-                                    )
+                                    Icon(Icons.Default.Edit, contentDescription = "Edit site ID")
                                }
-                                IconButton(onClick = { viewModel.removeSite(site.id) }) {
+                                IconButton(onClick = { siteToDelete = site.id }) {
                                    Icon(
                                        Icons.Default.Delete,
                                        contentDescription = "Remove site",