From fddfcac279865e78547b05384a1dc7cd53470633 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Thu, 15 Oct 2026 15:38:23 +0000 Subject: [PATCH 1/5] Expense screen UI improvements and animations - Embed shimmer loading item inside LazyColumn to eliminate layout popping - Add Modifier.animateItem() on expense cards for smooth item insertions and removals - Add animated total spent counter with animateFloatAsState - Add receipt scanning shortcuts (top app bar action & recent expenses header pill) - Expand expense category matching to recognize common travel, dining, and lodging synonyms --- .../feature/expenses/ManageExpensesScreen.kt | 114 ++++++++++++------ .../expenses/ManageExpensesViewModel.kt | 7 +- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt index 6f681ed6..bff6ff7a 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt @@ -16,7 +16,6 @@ package com.example.jetpacker.feature.expenses -import com.example.jetpacker.core.ui.components.JetPackerFabConfig import android.util.Log import android.widget.Toast import androidx.activity.compose.rememberLauncherForActivityResult @@ -24,6 +23,7 @@ import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.RepeatMode import androidx.compose.animation.core.animateFloat +import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.infiniteRepeatable import androidx.compose.animation.core.rememberInfiniteTransition import androidx.compose.animation.core.tween @@ -55,7 +55,6 @@ import androidx.compose.material.icons.rounded.DocumentScanner import androidx.compose.material.icons.rounded.Hotel import androidx.compose.material.icons.rounded.ModeOfTravel import androidx.compose.material.icons.rounded.Restaurant -import androidx.compose.material.icons.rounded.Search import androidx.compose.material.icons.rounded.ShoppingCart import androidx.compose.material.icons.rounded.Star import androidx.compose.material3.Button @@ -115,6 +114,7 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.example.jetpacker.core.ui.EventColors import com.example.jetpacker.core.ui.SekuyaFontFamily +import com.example.jetpacker.core.ui.components.JetPackerFabConfig import com.example.jetpacker.data.itinerary.Expense import com.example.jetpacker.data.trips.DummyData import kotlinx.coroutines.launch @@ -166,6 +166,7 @@ fun ManageExpensesScreen( contentPadding: PaddingValues, onBack: () -> Unit = {}, onFabConfigChange: (JetPackerFabConfig?) -> Unit = {}, + isAssistantProcessingExpense: Boolean = false, viewModel: ManageExpensesViewModel = hiltViewModel(), ) { val scrollBehavior = TopAppBarDefaults.enterAlwaysScrollBehavior() @@ -190,6 +191,12 @@ fun ManageExpensesScreen( var expenseToDelete by remember { mutableStateOf(null) } val totalSpent = expenses.sumOf { CurrencyConverter.convertToUsd(it.amount, it.currency) } + val animatedTotalSpent by + animateFloatAsState( + targetValue = totalSpent.toFloat(), + animationSpec = tween(durationMillis = 600), + label = "total_spent_anim", + ) val isProcessing = remember { mutableStateOf(false) } @@ -231,19 +238,21 @@ fun ManageExpensesScreen( } } + val onScanReceiptClick: () -> Unit = { + coroutineScope.launch { + if (viewModel.isSupported()) { + cameraLauncher.launch(null) + } else { + Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() + } + } + } + LaunchedEffect(Unit) { onFabConfigChange( JetPackerFabConfig( icon = Icons.Rounded.DocumentScanner, - onClick = { - coroutineScope.launch { - if (viewModel.isSupported()) { - cameraLauncher.launch(null) - } else { - Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() - } - } - }, + onClick = onScanReceiptClick, contentDescription = "Add Expense", ) ) @@ -272,10 +281,10 @@ fun ManageExpensesScreen( } }, actions = { - IconButton(onClick = {}) { + IconButton(onClick = onScanReceiptClick) { Icon( - Icons.Rounded.Search, - contentDescription = "Search", + Icons.Rounded.DocumentScanner, + contentDescription = "Scan Receipt", tint = MaterialTheme.colorScheme.onSurface, ) } @@ -297,7 +306,7 @@ fun ManageExpensesScreen( modifier = Modifier.fillMaxWidth().padding(vertical = 16.dp), horizontalAlignment = Alignment.CenterHorizontally, ) { - val totalText = "$${"%.2f".format(totalSpent)}" + val totalText = "$${"%.2f".format(animatedTotalSpent)}" Surface(shape = RoundedCornerShape(30.dp), color = MaterialTheme.colorScheme.surface) { Text( @@ -342,19 +351,44 @@ fun ManageExpensesScreen( } } - Text( - text = "Recent expenses", - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.Medium, - color = MaterialTheme.colorScheme.onSurface, - modifier = Modifier.padding(vertical = 16.dp), - ) - - if (isProcessing.value) { - ShimmerExpenseItem() - Spacer(modifier = Modifier.height(12.dp)) + Row( + modifier = Modifier.fillMaxWidth().padding(vertical = 12.dp), + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = "Recent expenses", + style = MaterialTheme.typography.titleMedium, + fontWeight = FontWeight.Medium, + color = MaterialTheme.colorScheme.onSurface, + ) + Surface( + shape = RoundedCornerShape(24.dp), + color = MaterialTheme.colorScheme.secondary, + contentColor = MaterialTheme.colorScheme.onSecondary, + modifier = Modifier.clickable { onScanReceiptClick() }, + ) { + Row( + modifier = Modifier.padding(horizontal = 14.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(6.dp), + ) { + Icon( + imageVector = Icons.Rounded.DocumentScanner, + contentDescription = "Scan receipt", + modifier = Modifier.size(18.dp), + ) + Text( + text = "Scan receipt", + style = MaterialTheme.typography.labelLarge, + fontWeight = FontWeight.Bold, + ) + } + } } + val showShimmer = isProcessing.value || isAssistantProcessingExpense + LazyColumn( modifier = Modifier.graphicsLayer(compositingStrategy = CompositingStrategy.Offscreen) @@ -373,6 +407,9 @@ fun ManageExpensesScreen( verticalArrangement = Arrangement.spacedBy(12.dp), contentPadding = PaddingValues(bottom = 100.dp), ) { + if (showShimmer) { + item(key = "shimmer_placeholder") { ShimmerExpenseItem() } + } items(expenses, key = { it.id }) { expense -> ExpenseItem( expense = expense, @@ -383,6 +420,7 @@ fun ManageExpensesScreen( isDetailSheetVisible = true }, onDelete = { expenseToDelete = expense }, + modifier = Modifier.animateItem(), ) } } @@ -505,11 +543,20 @@ fun ExpenseItem( ) { val (icon, color) = when (expense.category.lowercase()) { - "travel" -> Icons.Rounded.ModeOfTravel to EventColors.Flight - "food" -> Icons.Rounded.Restaurant to EventColors.Food + "travel", + "transport", + "transportation", + "flight", + "train", + "transit" -> Icons.Rounded.ModeOfTravel to EventColors.Flight + "food", + "dining", + "restaurant" -> Icons.Rounded.Restaurant to EventColors.Food "shopping" -> Icons.Rounded.ShoppingCart to EventColors.Shopping "entertainment" -> Icons.Rounded.Star to EventColors.Entertainment - "hotel" -> Icons.Rounded.Hotel to EventColors.Hotel + "hotel", + "accommodation", + "lodging" -> Icons.Rounded.Hotel to EventColors.Hotel else -> Icons.Rounded.ConfirmationNumber to EventColors.Activity } @@ -672,11 +719,8 @@ fun DeleteExpenseSheet(expense: Expense, onConfirm: () -> Unit, onDismiss: () -> .padding(horizontal = 8.dp, vertical = 12.dp), verticalAlignment = Alignment.CenterVertically, ) { - Checkbox( - checked = isConfirmChecked, + Checkbox(checked = isConfirmChecked, onCheckedChange = { isConfirmChecked = it }) - onCheckedChange = { isConfirmChecked = it } - ) Text( "I am sure I want to delete this expense.", style = MaterialTheme.typography.labelMedium, @@ -688,9 +732,7 @@ fun DeleteExpenseSheet(expense: Expense, onConfirm: () -> Unit, onDismiss: () -> Row(modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(12.dp)) { TextButton( colors = - ButtonDefaults.textButtonColors( - contentColor = MaterialTheme.colorScheme.onSurface - ), + ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.onSurface), onClick = onDismiss, modifier = Modifier.weight(1f), ) { diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModel.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModel.kt index 6a5c312f..5e9343fa 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModel.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModel.kt @@ -40,7 +40,8 @@ constructor( ) : ViewModel() { private val _tripIdFlow = MutableStateFlow(savedStateHandle.get("tripId") ?: "") - internal val tripId: String get() = _tripIdFlow.value + internal val tripId: String + get() = _tripIdFlow.value fun loadForTrip(id: String) { if (id.isNotEmpty() && id != _tripIdFlow.value) { @@ -49,7 +50,9 @@ constructor( } @OptIn(ExperimentalCoroutinesApi::class) - val expenses: Flow> = _tripIdFlow.flatMapLatest { id -> expenseDao.getExpensesForTrip(id) } + val expenses: Flow> = _tripIdFlow.flatMapLatest { id -> + expenseDao.getExpensesForTrip(id) + } suspend fun isSupported(): Boolean { return receiptParser.isSupported() From ff8e59b7cdeefcf7d4f404adfcd6abe90252a922 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Fri, 16 Oct 2026 08:53:20 +0000 Subject: [PATCH 2/5] Address PR review comments: performance optimizations, button states, and animation polish - Isolate animated total spent counter into TotalSpentDisplay to prevent full-screen recompositions - Memoize totalSpent calculation with remember(expenses) - Remember onScanReceiptClick callback and guard against isProcessing state - Disable TopAppBar scan action and header scan pill while processing is active - Use Surface(onClick) on header scan pill for bounded ripples and accessibility - Add Modifier.animateItem() to ShimmerExpenseItem for smooth entry and exit transitions - Trim category string during normalization --- .../feature/expenses/ManageExpensesScreen.kt | 162 ++++++++++-------- 1 file changed, 93 insertions(+), 69 deletions(-) diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt index bff6ff7a..5c87ed3c 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt @@ -190,13 +190,10 @@ fun ManageExpensesScreen( var expenseToDelete by remember { mutableStateOf(null) } - val totalSpent = expenses.sumOf { CurrencyConverter.convertToUsd(it.amount, it.currency) } - val animatedTotalSpent by - animateFloatAsState( - targetValue = totalSpent.toFloat(), - animationSpec = tween(durationMillis = 600), - label = "total_spent_anim", - ) + val totalSpent = + remember(expenses) { + expenses.sumOf { CurrencyConverter.convertToUsd(it.amount, it.currency) } + } val isProcessing = remember { mutableStateOf(false) } @@ -238,12 +235,16 @@ fun ManageExpensesScreen( } } - val onScanReceiptClick: () -> Unit = { - coroutineScope.launch { - if (viewModel.isSupported()) { - cameraLauncher.launch(null) - } else { - Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() + val onScanReceiptClick: () -> Unit = remember { + { + if (!isProcessing.value) { + coroutineScope.launch { + if (viewModel.isSupported()) { + cameraLauncher.launch(null) + } else { + Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() + } + } } } } @@ -281,7 +282,7 @@ fun ManageExpensesScreen( } }, actions = { - IconButton(onClick = onScanReceiptClick) { + IconButton(onClick = onScanReceiptClick, enabled = !isProcessing.value) { Icon( Icons.Rounded.DocumentScanner, contentDescription = "Scan Receipt", @@ -302,54 +303,11 @@ fun ManageExpensesScreen( Column(modifier = Modifier.fillMaxSize().padding(innerPadding).padding(horizontal = 16.dp)) { // Total spent and Budget display - Column( - modifier = Modifier.fillMaxWidth().padding(vertical = 16.dp), - horizontalAlignment = Alignment.CenterHorizontally, - ) { - val totalText = "$${"%.2f".format(animatedTotalSpent)}" - - Surface(shape = RoundedCornerShape(30.dp), color = MaterialTheme.colorScheme.surface) { - Text( - text = totalText, - style = - MaterialTheme.typography.displayLarge.copy( - fontSize = 84.sp, - fontWeight = FontWeight.Black, - ), - color = MaterialTheme.colorScheme.onSurface, - maxLines = 1, - autoSize = TextAutoSize.StepBased(minFontSize = 24.sp, maxFontSize = 84.sp), - modifier = Modifier.padding(horizontal = 24.dp, vertical = 12.dp), - ) - } - - Row( - modifier = Modifier.fillMaxWidth().padding(top = 12.dp, end = 16.dp), - horizontalArrangement = Arrangement.End, - verticalAlignment = Alignment.CenterVertically, - ) { - Text( - text = "/", - style = MaterialTheme.typography.titleLarge, - fontWeight = FontWeight.Black, - color = MaterialTheme.colorScheme.secondary, - ) - Spacer(modifier = Modifier.width(8.dp)) - Surface( - shape = RoundedCornerShape(30.dp), - color = MaterialTheme.colorScheme.onSurface, - modifier = Modifier.clickable { isBudgetSheetVisible = true }, - ) { - Text( - text = "$${budget.toInt()}", - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.Black, - color = MaterialTheme.colorScheme.surface, - modifier = Modifier.padding(horizontal = 16.dp, vertical = 12.dp), - ) - } - } - } + TotalSpentDisplay( + totalSpent = totalSpent, + budget = budget, + onBudgetClick = { isBudgetSheetVisible = true }, + ) Row( modifier = Modifier.fillMaxWidth().padding(vertical = 12.dp), @@ -363,10 +321,11 @@ fun ManageExpensesScreen( color = MaterialTheme.colorScheme.onSurface, ) Surface( + onClick = onScanReceiptClick, shape = RoundedCornerShape(24.dp), color = MaterialTheme.colorScheme.secondary, contentColor = MaterialTheme.colorScheme.onSecondary, - modifier = Modifier.clickable { onScanReceiptClick() }, + enabled = !isProcessing.value, ) { Row( modifier = Modifier.padding(horizontal = 14.dp, vertical = 8.dp), @@ -375,7 +334,7 @@ fun ManageExpensesScreen( ) { Icon( imageVector = Icons.Rounded.DocumentScanner, - contentDescription = "Scan receipt", + contentDescription = null, modifier = Modifier.size(18.dp), ) Text( @@ -408,7 +367,9 @@ fun ManageExpensesScreen( contentPadding = PaddingValues(bottom = 100.dp), ) { if (showShimmer) { - item(key = "shimmer_placeholder") { ShimmerExpenseItem() } + item(key = "shimmer_placeholder") { + ShimmerExpenseItem(modifier = Modifier.animateItem()) + } } items(expenses, key = { it.id }) { expense -> ExpenseItem( @@ -461,7 +422,70 @@ fun ManageExpensesScreen( } @Composable -fun ShimmerExpenseItem() { +private fun TotalSpentDisplay( + totalSpent: Double, + budget: Double, + onBudgetClick: () -> Unit, + modifier: Modifier = Modifier, +) { + val animatedTotalSpent by + animateFloatAsState( + targetValue = totalSpent.toFloat(), + animationSpec = tween(durationMillis = 600), + label = "total_spent_anim", + ) + val totalText = "$${"%.2f".format(animatedTotalSpent)}" + + Column( + modifier = modifier.fillMaxWidth().padding(vertical = 16.dp), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Surface(shape = RoundedCornerShape(30.dp), color = MaterialTheme.colorScheme.surface) { + Text( + text = totalText, + style = + MaterialTheme.typography.displayLarge.copy( + fontSize = 84.sp, + fontWeight = FontWeight.Black, + ), + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + autoSize = TextAutoSize.StepBased(minFontSize = 24.sp, maxFontSize = 84.sp), + modifier = Modifier.padding(horizontal = 24.dp, vertical = 12.dp), + ) + } + + Row( + modifier = Modifier.fillMaxWidth().padding(top = 12.dp, end = 16.dp), + horizontalArrangement = Arrangement.End, + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = "/", + style = MaterialTheme.typography.titleLarge, + fontWeight = FontWeight.Black, + color = MaterialTheme.colorScheme.secondary, + ) + Spacer(modifier = Modifier.width(8.dp)) + Surface( + shape = RoundedCornerShape(30.dp), + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.clickable { onBudgetClick() }, + ) { + Text( + text = "$${budget.toInt()}", + style = MaterialTheme.typography.titleMedium, + fontWeight = FontWeight.Black, + color = MaterialTheme.colorScheme.surface, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 12.dp), + ) + } + } + } +} + +@Composable +fun ShimmerExpenseItem(modifier: Modifier = Modifier) { val shimmerColors = listOf( Color.LightGray.copy(alpha = 0.6f), @@ -490,7 +514,7 @@ fun ShimmerExpenseItem() { ) Card( - modifier = Modifier.fillMaxWidth(), + modifier = modifier.fillMaxWidth(), shape = RoundedCornerShape(12.dp), colors = CardDefaults.cardColors(containerColor = MaterialTheme.colorScheme.surfaceContainer), ) { @@ -542,7 +566,7 @@ fun ExpenseItem( modifier: Modifier = Modifier, ) { val (icon, color) = - when (expense.category.lowercase()) { + when (expense.category.trim().lowercase()) { "travel", "transport", "transportation", @@ -824,7 +848,7 @@ fun ExpenseDetailSheet( Button( onClick = { val newAmount = amountText.toDoubleOrNull() ?: expense.amount - onSave(expense.copy(title = title, amount = newAmount, category = category.lowercase())) + onSave(expense.copy(title = title, amount = newAmount, category = category.trim().lowercase())) }, modifier = Modifier.fillMaxWidth().height(56.dp), ) { From 044cd0d50f4258f86324986e048b0f73bc4e5b49 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Wed, 21 Oct 2026 10:04:36 +0000 Subject: [PATCH 3/5] Address PR review feedback: fix scan button disabled state, camera launch guard, and add unit tests - Remove explicit tint on TopAppBar scan icon to let IconButton disabled state manage opacity - Add in-flight camera launch guard to prevent race condition on rapid double-tap - Add disabled container and content color handling to header scan Surface pill - Cache LazyColumn fading edge gradient with drawWithCache to avoid allocations per frame - Migrate deprecated centerAlignedTopAppBarColors to topAppBarColors - Add unit tests for CurrencyConverter (conversions, fallback, case-insensitivity, currency symbols) - Add unit tests for ManageExpensesViewModel (filtering, insert, delete, capability delegation) --- .../feature/expenses/ManageExpensesScreen.kt | 58 ++-- .../feature/expenses/CurrencyConverterTest.kt | 73 +++++ .../expenses/ManageExpensesViewModelTest.kt | 255 ++++++++++++++++++ 3 files changed, 364 insertions(+), 22 deletions(-) create mode 100644 jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt create mode 100644 jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModelTest.kt diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt index 5c87ed3c..b262578a 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt @@ -235,14 +235,22 @@ fun ManageExpensesScreen( } } + var isLaunchingCamera by remember { mutableStateOf(false) } + val isScanDisabled = isProcessing.value || isLaunchingCamera + val onScanReceiptClick: () -> Unit = remember { { - if (!isProcessing.value) { + if (!isProcessing.value && !isLaunchingCamera) { + isLaunchingCamera = true coroutineScope.launch { - if (viewModel.isSupported()) { - cameraLauncher.launch(null) - } else { - Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() + try { + if (viewModel.isSupported()) { + cameraLauncher.launch(null) + } else { + Toast.makeText(context, "Feature not available", Toast.LENGTH_SHORT).show() + } + } finally { + isLaunchingCamera = false } } } @@ -282,16 +290,15 @@ fun ManageExpensesScreen( } }, actions = { - IconButton(onClick = onScanReceiptClick, enabled = !isProcessing.value) { + IconButton(onClick = onScanReceiptClick, enabled = !isScanDisabled) { Icon( Icons.Rounded.DocumentScanner, contentDescription = "Scan Receipt", - tint = MaterialTheme.colorScheme.onSurface, ) } }, colors = - TopAppBarDefaults.centerAlignedTopAppBarColors( + TopAppBarDefaults.topAppBarColors( containerColor = Color.Transparent, scrolledContainerColor = Color.Transparent, ), @@ -323,9 +330,13 @@ fun ManageExpensesScreen( Surface( onClick = onScanReceiptClick, shape = RoundedCornerShape(24.dp), - color = MaterialTheme.colorScheme.secondary, - contentColor = MaterialTheme.colorScheme.onSecondary, - enabled = !isProcessing.value, + color = + if (!isScanDisabled) MaterialTheme.colorScheme.secondary + else MaterialTheme.colorScheme.secondary.copy(alpha = 0.38f), + contentColor = + if (!isScanDisabled) MaterialTheme.colorScheme.onSecondary + else MaterialTheme.colorScheme.onSecondary.copy(alpha = 0.38f), + enabled = !isScanDisabled, ) { Row( modifier = Modifier.padding(horizontal = 14.dp, vertical = 8.dp), @@ -351,17 +362,20 @@ fun ManageExpensesScreen( LazyColumn( modifier = Modifier.graphicsLayer(compositingStrategy = CompositingStrategy.Offscreen) - .drawWithContent { - drawContent() - drawRect( - brush = - Brush.verticalGradient( - 0f to Color.Black, - 0.8f to Color.Black, - 1f to Color.Transparent, - ), - blendMode = BlendMode.DstIn, - ) + .drawWithCache { + val brush = + Brush.verticalGradient( + 0f to Color.Black, + 0.8f to Color.Black, + 1f to Color.Transparent, + ) + onDrawWithContent { + drawContent() + drawRect( + brush = brush, + blendMode = BlendMode.DstIn, + ) + } }, verticalArrangement = Arrangement.spacedBy(12.dp), contentPadding = PaddingValues(bottom = 100.dp), diff --git a/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt new file mode 100644 index 00000000..6d6c0204 --- /dev/null +++ b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt @@ -0,0 +1,73 @@ +/* + * Copyright 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.example.jetpacker.feature.expenses + +import org.junit.Assert.assertEquals +import org.junit.Test + +class CurrencyConverterTest { + + @Test + fun convertToUsd_knownCurrencies_calculatesExpectedAmount() { + assertEquals(100.0, CurrencyConverter.convertToUsd(100.0, "USD"), 0.0001) + assertEquals(108.0, CurrencyConverter.convertToUsd(100.0, "EUR"), 0.0001) + assertEquals(127.0, CurrencyConverter.convertToUsd(100.0, "GBP"), 0.0001) + assertEquals(6.7, CurrencyConverter.convertToUsd(1000.0, "JPY"), 0.0001) + assertEquals(1.2, CurrencyConverter.convertToUsd(100.0, "INR"), 0.0001) + assertEquals(0.75, CurrencyConverter.convertToUsd(1000.0, "KRW"), 0.0001) + assertEquals(75.0, CurrencyConverter.convertToUsd(100.0, "SGD"), 0.0001) + assertEquals(14.0, CurrencyConverter.convertToUsd(100.0, "CNY"), 0.0001) + assertEquals(2.9, CurrencyConverter.convertToUsd(100.0, "THB"), 0.0001) + assertEquals(0.4, CurrencyConverter.convertToUsd(10000.0, "VND"), 0.0001) + } + + @Test + fun convertToUsd_isCaseInsensitive() { + assertEquals(108.0, CurrencyConverter.convertToUsd(100.0, "eur"), 0.0001) + assertEquals(127.0, CurrencyConverter.convertToUsd(100.0, "Gbp"), 0.0001) + } + + @Test + fun convertToUsd_unknownCurrency_defaultsToOneToOneRate() { + assertEquals(42.0, CurrencyConverter.convertToUsd(42.0, "XYZ"), 0.0001) + } + + @Test + fun getSymbol_knownCurrencies_returnsExpectedSymbol() { + assertEquals("$", CurrencyConverter.getSymbol("USD")) + assertEquals("€", CurrencyConverter.getSymbol("EUR")) + assertEquals("£", CurrencyConverter.getSymbol("GBP")) + assertEquals("¥", CurrencyConverter.getSymbol("JPY")) + assertEquals("₹", CurrencyConverter.getSymbol("INR")) + assertEquals("₩", CurrencyConverter.getSymbol("KRW")) + assertEquals("S$", CurrencyConverter.getSymbol("SGD")) + assertEquals("¥", CurrencyConverter.getSymbol("CNY")) + assertEquals("฿", CurrencyConverter.getSymbol("THB")) + assertEquals("₫", CurrencyConverter.getSymbol("VND")) + } + + @Test + fun getSymbol_isCaseInsensitive() { + assertEquals("€", CurrencyConverter.getSymbol("eur")) + assertEquals("£", CurrencyConverter.getSymbol("Gbp")) + } + + @Test + fun getSymbol_unknownCurrency_defaultsToDollarSign() { + assertEquals("$", CurrencyConverter.getSymbol("UNKNOWN")) + } +} diff --git a/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModelTest.kt b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModelTest.kt new file mode 100644 index 00000000..862919db --- /dev/null +++ b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesViewModelTest.kt @@ -0,0 +1,255 @@ +/* + * Copyright 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.example.jetpacker.feature.expenses + +import android.graphics.Bitmap +import androidx.lifecycle.SavedStateHandle +import com.example.jetpacker.data.itinerary.Expense +import com.example.jetpacker.data.itinerary.ExpenseDao +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import sun.misc.Unsafe + +@OptIn(ExperimentalCoroutinesApi::class) +class ManageExpensesViewModelTest { + + private val testDispatcher = StandardTestDispatcher() + private lateinit var fakeExpenseDao: FakeExpenseDao + private lateinit var fakeReceiptParser: FakeReceiptParser + + @Before + fun setup() { + Dispatchers.setMain(testDispatcher) + fakeExpenseDao = FakeExpenseDao() + fakeReceiptParser = FakeReceiptParser() + } + + @Test + fun initialTripId_fromSavedStateHandle_loadsMatchingExpenses() = runTest { + val initialExpense = + Expense( + id = "exp-1", + title = "Dinner", + amount = 54.0, + currency = "USD", + category = "dining", + tripId = "trip-initial", + ) + fakeExpenseDao.insertExpense(initialExpense) + + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-initial")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + advanceUntilIdle() + + assertEquals("trip-initial", viewModel.tripId) + val expenses = viewModel.expenses.first() + assertEquals(1, expenses.size) + assertEquals("exp-1", expenses.first().id) + } + + @Test + fun loadForTrip_updatesTripIdAndFiltersExpenses() = runTest { + val expenseTripA = + Expense( + id = "exp-a", + title = "Taxi", + amount = 25.0, + currency = "USD", + category = "transportation", + tripId = "trip-a", + ) + val expenseTripB = + Expense( + id = "exp-b", + title = "Museum", + amount = 18.0, + currency = "EUR", + category = "activity", + tripId = "trip-b", + ) + fakeExpenseDao.insertExpense(expenseTripA) + fakeExpenseDao.insertExpense(expenseTripB) + + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-a")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + advanceUntilIdle() + assertEquals("trip-a", viewModel.tripId) + assertEquals(listOf("exp-a"), viewModel.expenses.first().map { it.id }) + + viewModel.loadForTrip("trip-b") + advanceUntilIdle() + + assertEquals("trip-b", viewModel.tripId) + assertEquals(listOf("exp-b"), viewModel.expenses.first().map { it.id }) + } + + @Test + fun addExpense_withEmptyTripId_assignsCurrentTripId() = runTest { + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-active")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + advanceUntilIdle() + + val newExpense = + Expense( + id = "exp-new", + title = "Lunch", + amount = 12.5, + currency = "USD", + category = "dining", + tripId = "", + ) + viewModel.addExpense(newExpense) + advanceUntilIdle() + + assertEquals(1, fakeExpenseDao.expenses.size) + val saved = fakeExpenseDao.expenses.first() + assertEquals("trip-active", saved.tripId) + assertEquals("exp-new", saved.id) + } + + @Test + fun addExpense_withExistingTripId_preservesTripId() = runTest { + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-active")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + advanceUntilIdle() + + val customExpense = + Expense( + id = "exp-custom", + title = "Flight", + amount = 350.0, + currency = "USD", + category = "transportation", + tripId = "trip-custom", + ) + viewModel.addExpense(customExpense) + advanceUntilIdle() + + assertEquals(1, fakeExpenseDao.expenses.size) + assertEquals("trip-custom", fakeExpenseDao.expenses.first().tripId) + } + + @Test + fun deleteExpense_removesExpenseFromDao() = runTest { + val expense = + Expense( + id = "exp-delete", + title = "Souvenir", + amount = 30.0, + currency = "USD", + category = "shopping", + tripId = "trip-1", + ) + fakeExpenseDao.insertExpense(expense) + + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-1")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + advanceUntilIdle() + assertEquals(1, fakeExpenseDao.expenses.size) + + viewModel.deleteExpense(expense) + advanceUntilIdle() + + assertTrue(fakeExpenseDao.expenses.isEmpty()) + } + + @Test + fun isSupported_delegatesToReceiptParser() = runTest { + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-1")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + fakeReceiptParser.supported = true + assertTrue(viewModel.isSupported()) + + fakeReceiptParser.supported = false + assertFalse(viewModel.isSupported()) + } + + @Test + fun parseReceipt_delegatesToReceiptParser() = runTest { + val savedStateHandle = SavedStateHandle(mapOf("tripId" to "trip-1")) + val viewModel = ManageExpensesViewModel(savedStateHandle, fakeReceiptParser, fakeExpenseDao) + + val unsafeField = Unsafe::class.java.getDeclaredField("theUnsafe") + unsafeField.isAccessible = true + val unsafe = unsafeField.get(null) as Unsafe + val dummyBitmap = unsafe.allocateInstance(Bitmap::class.java) as Bitmap + + val expectedJson = """{"title":"Coffee","amount":4.50,"currency":"USD","category":"dining"}""" + fakeReceiptParser.parsedResponse = expectedJson + + val response = viewModel.parseReceipt(dummyBitmap) + assertEquals(expectedJson, response) + } + + private class FakeExpenseDao : ExpenseDao { + val expenses = mutableListOf() + private val _expensesFlow = MutableStateFlow>(emptyList()) + + private fun emit() { + _expensesFlow.value = expenses.toList() + } + + override fun getAllExpenses(): Flow> = _expensesFlow + + override suspend fun insertExpense(expense: Expense) { + expenses.removeAll { it.id == expense.id } + expenses.add(expense) + emit() + } + + override suspend fun deleteExpense(expense: Expense) { + expenses.removeAll { it.id == expense.id } + emit() + } + + override suspend fun deleteAllExpenses() { + expenses.clear() + emit() + } + + override fun getExpensesForTrip(tripId: String): Flow> = + _expensesFlow.map { list -> list.filter { it.tripId == tripId } } + } + + private class FakeReceiptParser( + var supported: Boolean = true, + var parsedResponse: String = "", + ) : ReceiptParser { + override suspend fun isSupported(): Boolean = supported + + override suspend fun parseReceipt(bitmap: Bitmap): String = parsedResponse + } +} From c1a5d9fb3fbb9000a7c8b519ef2f726ae2ccd00f Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Wed, 21 Oct 2026 10:45:28 +0000 Subject: [PATCH 4/5] Address follow-up review feedback: whitespace resilience, remember keys, and assistant scan guard - Trim currency code in CurrencyConverter.convertToUsd and getSymbol - Disable scan buttons while isAssistantProcessingExpense is active - Key mutableStateOf in ExpenseDetailSheet by expense.id to prevent stale fields on updates - Add unit tests for CurrencyConverter whitespace handling --- .../feature/expenses/ManageExpensesScreen.kt | 14 +++++++------- .../feature/expenses/CurrencyConverterTest.kt | 12 ++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt index b262578a..954eaa5a 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt @@ -137,12 +137,12 @@ object CurrencyConverter { ) fun convertToUsd(amount: Double, currency: String): Double { - val rate = rates[currency.uppercase()] ?: 1.0 + val rate = rates[currency.trim().uppercase()] ?: 1.0 return amount * rate } fun getSymbol(currency: String): String { - return when (currency.uppercase()) { + return when (currency.trim().uppercase()) { "USD" -> "$" "INR" -> "₹" "EUR" -> "€" @@ -236,11 +236,11 @@ fun ManageExpensesScreen( } var isLaunchingCamera by remember { mutableStateOf(false) } - val isScanDisabled = isProcessing.value || isLaunchingCamera + val isScanDisabled = isProcessing.value || isLaunchingCamera || isAssistantProcessingExpense val onScanReceiptClick: () -> Unit = remember { { - if (!isProcessing.value && !isLaunchingCamera) { + if (!isScanDisabled) { isLaunchingCamera = true coroutineScope.launch { try { @@ -802,11 +802,11 @@ fun ExpenseDetailSheet( onDismiss: () -> Unit, onSave: (Expense) -> Unit, ) { - var title by remember { mutableStateOf(expense.title) } - var amountText by remember { + var title by remember(expense.id) { mutableStateOf(expense.title) } + var amountText by remember(expense.id) { mutableStateOf(if (expense.amount == 0.0) "" else expense.amount.toString()) } - var category by remember { mutableStateOf(expense.category) } + var category by remember(expense.id) { mutableStateOf(expense.category) } val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) diff --git a/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt index 6d6c0204..893fbbe7 100644 --- a/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt +++ b/jetpacker/android/feature/trip/expenses/src/test/kotlin/com/example/jetpacker/feature/expenses/CurrencyConverterTest.kt @@ -41,6 +41,12 @@ class CurrencyConverterTest { assertEquals(127.0, CurrencyConverter.convertToUsd(100.0, "Gbp"), 0.0001) } + @Test + fun convertToUsd_trimsWhitespace() { + assertEquals(108.0, CurrencyConverter.convertToUsd(100.0, " EUR "), 0.0001) + assertEquals(127.0, CurrencyConverter.convertToUsd(100.0, "\tGBP\n"), 0.0001) + } + @Test fun convertToUsd_unknownCurrency_defaultsToOneToOneRate() { assertEquals(42.0, CurrencyConverter.convertToUsd(42.0, "XYZ"), 0.0001) @@ -66,6 +72,12 @@ class CurrencyConverterTest { assertEquals("£", CurrencyConverter.getSymbol("Gbp")) } + @Test + fun getSymbol_trimsWhitespace() { + assertEquals("€", CurrencyConverter.getSymbol(" EUR ")) + assertEquals("£", CurrencyConverter.getSymbol("\tGBP\n")) + } + @Test fun getSymbol_unknownCurrency_defaultsToDollarSign() { assertEquals("$", CurrencyConverter.getSymbol("UNKNOWN")) From 9362a9c2293993083701dbebf2516365bc61b40c Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Mon, 26 Oct 2026 13:54:18 +0000 Subject: [PATCH 5/5] fix(expenses): remove duplicate scanner FAB to allow Trip Assistant FAB fallback --- .../jetpacker/feature/expenses/ManageExpensesScreen.kt | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt index 954eaa5a..47c84c76 100644 --- a/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt +++ b/jetpacker/android/feature/trip/expenses/src/main/kotlin/com/example/jetpacker/feature/expenses/ManageExpensesScreen.kt @@ -257,15 +257,7 @@ fun ManageExpensesScreen( } } - LaunchedEffect(Unit) { - onFabConfigChange( - JetPackerFabConfig( - icon = Icons.Rounded.DocumentScanner, - onClick = onScanReceiptClick, - contentDescription = "Add Expense", - ) - ) - } + LaunchedEffect(Unit) { onFabConfigChange(null) } Scaffold( modifier = modifier.nestedScroll(scrollBehavior.nestedScrollConnection),