From bd387bec86a668ff675238c9d0b98af8093d48d9 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 10 Jan 2025 18:19:10 +0100 Subject: [PATCH 1/8] Allow hiding totals table --- .../woopos/home/totals/WooPosTotalsScreen.kt | 96 ++++++++++--------- .../home/totals/WooPosTotalsViewModel.kt | 31 +++--- .../home/totals/WooPosTotalsViewState.kt | 17 +++- .../home/totals/WooPosTotalsViewModelTest.kt | 28 +++--- 4 files changed, 100 insertions(+), 72 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt index a0c3b906785..8a702b1dcd6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt @@ -74,8 +74,8 @@ private fun WooPosTotalsScreen( onUIEvent: (WooPosTotalsUIEvent) -> Unit, ) { Box(modifier = modifier) { - StateChangeAnimated(visible = state is WooPosTotalsViewState.Totals) { - if (state is WooPosTotalsViewState.Totals) { + StateChangeAnimated(visible = state is WooPosTotalsViewState.Checkout) { + if (state is WooPosTotalsViewState.Checkout) { TotalsLoaded( state = state, onUIEvent = onUIEvent, @@ -140,7 +140,7 @@ private fun StateChangeAnimated( @Composable private fun TotalsLoaded( - state: WooPosTotalsViewState.Totals, + state: WooPosTotalsViewState.Checkout, onUIEvent: (WooPosTotalsUIEvent) -> Unit, ) { Column( @@ -285,40 +285,44 @@ private fun ReaderDisconnected( } @Composable -private fun TotalsGrid(state: WooPosTotalsViewState.Totals) { - Column( - modifier = Modifier - .padding(24.dp.toAdaptivePadding()) - .width(382.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_subtotal_label), - textTwo = state.orderSubtotalText, - ) +private fun TotalsGrid(state: WooPosTotalsViewState.Checkout) { + when (state.totals) { + is WooPosTotalsViewState.Totals.Hidden -> Unit + is WooPosTotalsViewState.Totals.Visible -> + Column( + modifier = Modifier + .padding(24.dp.toAdaptivePadding()) + .width(382.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_subtotal_label), + textTwo = state.totals.orderSubtotalText, + ) - Spacer(modifier = Modifier.height(8.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(8.dp.toAdaptivePadding())) - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_tax_label), - textTwo = state.orderTaxText, - ) + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_tax_label), + textTwo = state.totals.orderTaxText, + ) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - Divider(color = WooPosTheme.colors.border, thickness = 1.dp) + Divider(color = WooPosTheme.colors.border, thickness = 1.dp) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_total_label), - textTwo = state.orderTotalText, - styleOne = MaterialTheme.typography.h4, - styleTwo = MaterialTheme.typography.h4, - fontWeightOne = FontWeight.Medium, - fontWeightTwo = FontWeight.Bold, - ) + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_total_label), + textTwo = state.totals.orderTotalText, + styleOne = MaterialTheme.typography.h4, + styleTwo = MaterialTheme.typography.h4, + fontWeightOne = FontWeight.Medium, + fontWeightTwo = FontWeight.Bold, + ) + } } } @@ -410,10 +414,12 @@ fun WooPosTotalsScreenPreview(modifier: Modifier = Modifier) { WooPosTheme { WooPosTotalsScreen( modifier = modifier, - state = WooPosTotalsViewState.Totals( - orderSubtotalText = "$420.00", - orderTotalText = "$462.00", - orderTaxText = "$42.00", + state = WooPosTotalsViewState.Checkout( + totals = WooPosTotalsViewState.Totals.Visible( + orderSubtotalText = "$420.00", + orderTotalText = "$462.00", + orderTaxText = "$42.00", + ), readerStatus = WooPosTotalsViewState.ReaderStatus.ReadyForPayment( title = "Ready for payment", subtitle = "Tap, swipe or insert card" @@ -430,10 +436,12 @@ fun WooPosTotalsScreenPreviewReaderNotConnected(modifier: Modifier = Modifier) { WooPosTheme { WooPosTotalsScreen( modifier = modifier, - state = WooPosTotalsViewState.Totals( - orderSubtotalText = "$420.00", - orderTotalText = "$462.00", - orderTaxText = "$42.00", + state = WooPosTotalsViewState.Checkout( + totals = WooPosTotalsViewState.Totals.Visible( + orderSubtotalText = "$420.00", + orderTotalText = "$462.00", + orderTaxText = "$42.00", + ), readerStatus = WooPosTotalsViewState.ReaderStatus.Disconnected( title = "Reader not connected", subtitle = "To process this payment, please connect your reader.", @@ -451,10 +459,12 @@ fun WooPosTotalsScreenPreviewWithCashPaymentAvailable() { WooPosTheme { WooPosTotalsScreen( modifier = Modifier, - state = WooPosTotalsViewState.Totals( - orderSubtotalText = "$420.00", - orderTotalText = "$462.00", - orderTaxText = "$42.00", + state = WooPosTotalsViewState.Checkout( + totals = WooPosTotalsViewState.Totals.Visible( + orderSubtotalText = "$420.00", + orderTotalText = "$462.00", + orderTaxText = "$42.00", + ), readerStatus = WooPosTotalsViewState.ReaderStatus.Disconnected( title = "Reader not connected", subtitle = "To process this payment, please connect your reader.", diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt index 40da8f0109e..37d66be1b55 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt @@ -30,6 +30,7 @@ import com.woocommerce.android.ui.woopos.home.WooPosParentToChildrenEventReceive import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsViewState.PaymentFailed import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsViewState.PaymentInProgress +import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsViewState.Totals import com.woocommerce.android.ui.woopos.util.WooPosNetworkStatus import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker @@ -43,6 +44,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine @@ -118,13 +120,13 @@ class WooPosTotalsViewModel @Inject constructor( when (status) { is NotConnected, is Connecting -> { val state = uiState.value - if (state !is WooPosTotalsViewState.Totals) return@collect + if (state !is WooPosTotalsViewState.Checkout) return@collect uiState.value = state.copy(readerStatus = buildTotalsReaderNotConnectedError()) cancelPaymentAction() } is Connected -> { val state = uiState.value - if (state !is WooPosTotalsViewState.Totals) return@collect + if (state !is WooPosTotalsViewState.Checkout) return@collect uiState.value = state.copy(readerStatus = buildPreparingReaderStatusState()) if (data.orderId != EMPTY_ORDER_ID) { collectPayment() @@ -255,8 +257,8 @@ class WooPosTotalsViewModel @Inject constructor( check(orderId != EMPTY_ORDER_ID) if (cardReaderFacade.readerStatus.value is Connected) { val state = uiState.value - check(state is WooPosTotalsViewState.Totals) - check(uiState.value is WooPosTotalsViewState.Totals) + check(state is WooPosTotalsViewState.Checkout) + check(uiState.value is WooPosTotalsViewState.Checkout) createCardReaderPaymentController(dataState.value.orderId) cardReaderPaymentController?.start() listenToPaymentState() @@ -298,6 +300,11 @@ class WooPosTotalsViewModel @Inject constructor( is CardReaderPaymentState.ProcessingPayment, is CardReaderPaymentState.PaymentCapturing -> { + val state = uiState.value + if (state is WooPosTotalsViewState.Checkout) { + uiState.value = state.copy(totals = Totals.Hidden) + delay(400) + } uiState.value = buildPaymentInProgressState(paymentState) childrenToParentEventSender.sendToParent(ChildToParentEvent.PaymentInProgress) } @@ -326,7 +333,7 @@ class WooPosTotalsViewModel @Inject constructor( private suspend fun handleCollectingPaymentState() { val totalsState = uiState.value - if (totalsState is WooPosTotalsViewState.Totals) { + if (totalsState is WooPosTotalsViewState.Checkout) { uiState.value = totalsState.copy( readerStatus = WooPosTotalsViewState.ReaderStatus.ReadyForPayment( title = resourceProvider.getString(R.string.woopos_totals_reader_ready_for_payment_title), @@ -343,7 +350,7 @@ class WooPosTotalsViewModel @Inject constructor( private suspend fun handleReaderLoadingPaymentState() { val totalsState = uiState.value - if (totalsState is WooPosTotalsViewState.Totals) { + if (totalsState is WooPosTotalsViewState.Checkout) { uiState.value = totalsState.copy( readerStatus = WooPosTotalsViewState.ReaderStatus.Preparing( @@ -445,7 +452,7 @@ class WooPosTotalsViewModel @Inject constructor( } } - private suspend fun buildWooPosTotalsViewState(order: Order): WooPosTotalsViewState.Totals { + private suspend fun buildWooPosTotalsViewState(order: Order): WooPosTotalsViewState.Checkout { val subtotalAmount = order.productsTotal val taxAmount = order.totalTax val totalAmount = order.total @@ -453,10 +460,12 @@ class WooPosTotalsViewModel @Inject constructor( is Connected -> buildPreparingReaderStatusState() else -> buildTotalsReaderNotConnectedError() } - return WooPosTotalsViewState.Totals( - orderSubtotalText = priceFormat(subtotalAmount), - orderTaxText = priceFormat(taxAmount), - orderTotalText = priceFormat(totalAmount), + return WooPosTotalsViewState.Checkout( + totals = Totals.Visible( + orderSubtotalText = priceFormat(subtotalAmount), + orderTaxText = priceFormat(taxAmount), + orderTotalText = priceFormat(totalAmount), + ), readerStatus = readerStatus, ) } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt index 9ccfe65b940..78af7a2a4fb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt @@ -7,13 +7,22 @@ import kotlinx.parcelize.Parcelize sealed class WooPosTotalsViewState : Parcelable { data object Loading : WooPosTotalsViewState() - data class Totals( - val orderSubtotalText: String, - val orderTaxText: String, - val orderTotalText: String, + data class Checkout( + val totals: Totals, val readerStatus: ReaderStatus, ) : WooPosTotalsViewState() + sealed class Totals : Parcelable { + @Parcelize + data object Hidden : Totals() + @Parcelize + data class Visible( + val orderSubtotalText: String, + val orderTaxText: String, + val orderTotalText: String, + ) : Totals() + } + data class PaymentSuccess(val orderTotalText: String) : WooPosTotalsViewState() sealed class ReaderStatus( diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt index 5a2071ed68d..81936c7debc 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt @@ -241,7 +241,7 @@ class WooPosTotalsViewModelTest { ) // THEN - val state = viewModel.state.value as WooPosTotalsViewState.Totals + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.orderTotalText).isEqualTo("$5.00") assertThat(state.orderTaxText).isEqualTo("$2.00") assertThat(state.orderSubtotalText).isEqualTo("$3.00") @@ -303,10 +303,10 @@ class WooPosTotalsViewModelTest { ) // THEN - val totals = viewModel.state.value as WooPosTotalsViewState.Totals - assertThat(totals.orderTotalText).isEqualTo("5.00$") - assertThat(totals.orderTaxText).isEqualTo("2.00$") - assertThat(totals.orderSubtotalText).isEqualTo("3.00$") + val checkout = viewModel.state.value as WooPosTotalsViewState.Checkout + assertThat(checkout.orderTotalText).isEqualTo("5.00$") + assertThat(checkout.orderTaxText).isEqualTo("2.00$") + assertThat(checkout.orderSubtotalText).isEqualTo("3.00$") } @Test @@ -440,7 +440,7 @@ class WooPosTotalsViewModelTest { viewModel.onUIEvent(WooPosTotalsUIEvent.RetryOrderCreationClicked) // Ensure the view model state transitions to the success state with correct totals - val state = viewModel.state.value as WooPosTotalsViewState.Totals + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.orderTotalText).isEqualTo("$5.00") assertThat(state.orderTaxText).isEqualTo("$2.00") assertThat(state.orderSubtotalText).isEqualTo("$3.00") @@ -502,7 +502,7 @@ class WooPosTotalsViewModelTest { ) // THEN - val state = viewModel.state.value as WooPosTotalsViewState.Totals + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.orderSubtotalText).isEqualTo("3.00$") assertThat(state.orderTaxText).isEqualTo("2.00$") assertThat(state.orderTotalText).isEqualTo("5.00$") @@ -578,7 +578,7 @@ class WooPosTotalsViewModelTest { ) // THEN - val state = viewModel.state.value as WooPosTotalsViewState.Totals + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.readerStatus).isInstanceOf(WooPosTotalsViewState.ReaderStatus.Preparing::class.java) } @@ -651,8 +651,8 @@ class WooPosTotalsViewModelTest { val viewModel = createViewModelAndSetupForSuccessfulOrderCreation() // THEN - assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Totals::class.java) - val state = viewModel.state.value as WooPosTotalsViewState.Totals + assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Checkout::class.java) + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.readerStatus).isNotNull() with(state.readerStatus as WooPosTotalsViewState.ReaderStatus.Disconnected) { assertThat(title).isEqualTo("Reader not connected") @@ -667,8 +667,8 @@ class WooPosTotalsViewModelTest { val viewModel = createViewModelAndSetupForSuccessfulOrderCreation() // THEN - assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Totals::class.java) - val state = viewModel.state.value as WooPosTotalsViewState.Totals + assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Checkout::class.java) + val state = viewModel.state.value as WooPosTotalsViewState.Checkout assertThat(state.readerStatus).isInstanceOf(WooPosTotalsViewState.ReaderStatus.Preparing::class.java) } @@ -682,7 +682,7 @@ class WooPosTotalsViewModelTest { // WHEN val viewModel = createViewModelAndSetupForSuccessfulOrderCreation() - assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Totals::class.java) + assertThat(viewModel.state.value).isInstanceOf(WooPosTotalsViewState.Checkout::class.java) viewModel.onUIEvent(WooPosTotalsUIEvent.ConnectReaderClicked) // THEN @@ -791,7 +791,7 @@ class WooPosTotalsViewModelTest { val vm = createViewModelAndSetupForSuccessfulOrderCreation(controllerFactory = factory) // THEN - val totalState = vm.state.value as WooPosTotalsViewState.Totals + val totalState = vm.state.value as WooPosTotalsViewState.Checkout assertThat(totalState.readerStatus).isInstanceOf( WooPosTotalsViewState.ReaderStatus.ReadyForPayment::class.java ) From 5c1fd6ce573eeb9ce6f7e41e40e7f2f584647794 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 10 Jan 2025 18:21:23 +0100 Subject: [PATCH 2/8] Update tests --- .../home/totals/WooPosTotalsViewModelTest.kt | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt index 81936c7debc..da23437a1fe 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt @@ -242,9 +242,11 @@ class WooPosTotalsViewModelTest { // THEN val state = viewModel.state.value as WooPosTotalsViewState.Checkout - assertThat(state.orderTotalText).isEqualTo("$5.00") - assertThat(state.orderTaxText).isEqualTo("$2.00") - assertThat(state.orderSubtotalText).isEqualTo("$3.00") + assert(state.totals is WooPosTotalsViewState.Totals.Visible) + state.totals as WooPosTotalsViewState.Totals.Visible + assertThat(state.totals.orderTotalText).isEqualTo("$5.00") + assertThat(state.totals.orderTaxText).isEqualTo("$2.00") + assertThat(state.totals.orderSubtotalText).isEqualTo("$3.00") verify(totalsRepository).createOrderWithProducts(itemClickedData) } @@ -303,10 +305,11 @@ class WooPosTotalsViewModelTest { ) // THEN - val checkout = viewModel.state.value as WooPosTotalsViewState.Checkout - assertThat(checkout.orderTotalText).isEqualTo("5.00$") - assertThat(checkout.orderTaxText).isEqualTo("2.00$") - assertThat(checkout.orderSubtotalText).isEqualTo("3.00$") + val state = viewModel.state.value as WooPosTotalsViewState.Checkout + state.totals as WooPosTotalsViewState.Totals.Visible + assertThat(state.totals.orderTotalText).isEqualTo("5.00$") + assertThat(state.totals.orderTaxText).isEqualTo("2.00$") + assertThat(state.totals.orderSubtotalText).isEqualTo("3.00$") } @Test @@ -441,9 +444,10 @@ class WooPosTotalsViewModelTest { // Ensure the view model state transitions to the success state with correct totals val state = viewModel.state.value as WooPosTotalsViewState.Checkout - assertThat(state.orderTotalText).isEqualTo("$5.00") - assertThat(state.orderTaxText).isEqualTo("$2.00") - assertThat(state.orderSubtotalText).isEqualTo("$3.00") + state.totals as WooPosTotalsViewState.Totals.Visible + assertThat(state.totals.orderTotalText).isEqualTo("$5.00") + assertThat(state.totals.orderTaxText).isEqualTo("$2.00") + assertThat(state.totals.orderSubtotalText).isEqualTo("$3.00") } @Test @@ -503,9 +507,10 @@ class WooPosTotalsViewModelTest { // THEN val state = viewModel.state.value as WooPosTotalsViewState.Checkout - assertThat(state.orderSubtotalText).isEqualTo("3.00$") - assertThat(state.orderTaxText).isEqualTo("2.00$") - assertThat(state.orderTotalText).isEqualTo("5.00$") + state.totals as WooPosTotalsViewState.Totals.Visible + assertThat(state.totals.orderSubtotalText).isEqualTo("3.00$") + assertThat(state.totals.orderTaxText).isEqualTo("2.00$") + assertThat(state.totals.orderTotalText).isEqualTo("5.00$") verify(totalsRepository).createOrderWithProducts(itemClickedData) } From 69c831bd97dba5082e15b48277de835f27d62bfd Mon Sep 17 00:00:00 2001 From: samiuelson Date: Mon, 13 Jan 2025 17:09:54 +0100 Subject: [PATCH 3/8] Animate totals grid to shrink on exit --- .../woopos/home/totals/WooPosTotalsScreen.kt | 116 ++++++++++-------- .../home/totals/WooPosTotalsViewModel.kt | 2 +- 2 files changed, 63 insertions(+), 55 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt index 8a702b1dcd6..72455b5434e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt @@ -1,5 +1,6 @@ package com.woocommerce.android.ui.woopos.home.totals +import androidx.compose.animation.AnimatedContent import androidx.compose.animation.AnimatedVisibility import androidx.compose.animation.AnimatedVisibilityScope import androidx.compose.animation.fadeIn @@ -52,6 +53,7 @@ import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosErrorS import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosOutlinedButton import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosShimmerBox import com.woocommerce.android.ui.woopos.common.composeui.toAdaptivePadding +import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsViewState.Totals import com.woocommerce.android.ui.woopos.home.totals.payment.failed.WooPosPaymentFailedScreen import com.woocommerce.android.ui.woopos.home.totals.payment.inprogress.WooPosPaymentInProgressScreen import com.woocommerce.android.ui.woopos.home.totals.payment.success.WooPosPaymentSuccessScreen @@ -174,26 +176,36 @@ private fun TotalsLoaded( } } } - Column( - modifier = Modifier - .fillMaxWidth() - .padding( - horizontal = 40.dp.toAdaptivePadding(), - vertical = 16.dp.toAdaptivePadding() - ) - .weight(.9f), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - TotalsGrid(state = state) - Column(horizontalAlignment = Alignment.CenterHorizontally) { - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - WooPosOutlinedButton( - text = stringResource(R.string.woopos_payment_take_cash_payment_label), - onClick = { onUIEvent(WooPosTotalsUIEvent.OnCashPaymentClicked) }, - ) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + AnimatedContent( + targetState = state.totals, + label = "totals_grid_animation", + ) { state -> + when (state) { + is Totals.Hidden -> Unit + is Totals.Visible -> { + Column( + modifier = Modifier + .fillMaxWidth() + .padding( + horizontal = 40.dp.toAdaptivePadding(), + vertical = 16.dp.toAdaptivePadding() + ) + .weight(.9f), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + TotalsGrid(totals = state) + Column(horizontalAlignment = Alignment.CenterHorizontally) { + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + WooPosOutlinedButton( + text = stringResource(R.string.woopos_payment_take_cash_payment_label), + onClick = { onUIEvent(WooPosTotalsUIEvent.OnCashPaymentClicked) }, + ) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + } + } + } } } } @@ -285,44 +297,40 @@ private fun ReaderDisconnected( } @Composable -private fun TotalsGrid(state: WooPosTotalsViewState.Checkout) { - when (state.totals) { - is WooPosTotalsViewState.Totals.Hidden -> Unit - is WooPosTotalsViewState.Totals.Visible -> - Column( - modifier = Modifier - .padding(24.dp.toAdaptivePadding()) - .width(382.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_subtotal_label), - textTwo = state.totals.orderSubtotalText, - ) +private fun TotalsGrid(totals: Totals.Visible) { + Column( + modifier = Modifier + .padding(24.dp.toAdaptivePadding()) + .width(382.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_subtotal_label), + textTwo = totals.orderSubtotalText, + ) - Spacer(modifier = Modifier.height(8.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(8.dp.toAdaptivePadding())) - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_tax_label), - textTwo = state.totals.orderTaxText, - ) + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_tax_label), + textTwo = totals.orderTaxText, + ) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - Divider(color = WooPosTheme.colors.border, thickness = 1.dp) + Divider(color = WooPosTheme.colors.border, thickness = 1.dp) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - TotalsGridRow( - textOne = stringResource(R.string.woopos_payment_total_label), - textTwo = state.totals.orderTotalText, - styleOne = MaterialTheme.typography.h4, - styleTwo = MaterialTheme.typography.h4, - fontWeightOne = FontWeight.Medium, - fontWeightTwo = FontWeight.Bold, - ) - } + TotalsGridRow( + textOne = stringResource(R.string.woopos_payment_total_label), + textTwo = totals.orderTotalText, + styleOne = MaterialTheme.typography.h4, + styleTwo = MaterialTheme.typography.h4, + fontWeightOne = FontWeight.Medium, + fontWeightTwo = FontWeight.Bold, + ) } } @@ -415,7 +423,7 @@ fun WooPosTotalsScreenPreview(modifier: Modifier = Modifier) { WooPosTotalsScreen( modifier = modifier, state = WooPosTotalsViewState.Checkout( - totals = WooPosTotalsViewState.Totals.Visible( + totals = Totals.Visible( orderSubtotalText = "$420.00", orderTotalText = "$462.00", orderTaxText = "$42.00", @@ -437,7 +445,7 @@ fun WooPosTotalsScreenPreviewReaderNotConnected(modifier: Modifier = Modifier) { WooPosTotalsScreen( modifier = modifier, state = WooPosTotalsViewState.Checkout( - totals = WooPosTotalsViewState.Totals.Visible( + totals = Totals.Visible( orderSubtotalText = "$420.00", orderTotalText = "$462.00", orderTaxText = "$42.00", @@ -460,7 +468,7 @@ fun WooPosTotalsScreenPreviewWithCashPaymentAvailable() { WooPosTotalsScreen( modifier = Modifier, state = WooPosTotalsViewState.Checkout( - totals = WooPosTotalsViewState.Totals.Visible( + totals = Totals.Visible( orderSubtotalText = "$420.00", orderTotalText = "$462.00", orderTaxText = "$42.00", diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt index 37d66be1b55..fa0aa95043a 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt @@ -303,7 +303,7 @@ class WooPosTotalsViewModel @Inject constructor( val state = uiState.value if (state is WooPosTotalsViewState.Checkout) { uiState.value = state.copy(totals = Totals.Hidden) - delay(400) + delay(300) } uiState.value = buildPaymentInProgressState(paymentState) childrenToParentEventSender.sendToParent(ChildToParentEvent.PaymentInProgress) From cf8b037d90f41f6f8b23af01afff4ce6fd5ecd7e Mon Sep 17 00:00:00 2001 From: samiuelson Date: Mon, 13 Jan 2025 17:10:29 +0100 Subject: [PATCH 4/8] Remove redundant animations from payment in progress screen --- .../WooPosTotalsPaymentInProgressScreen.kt | 49 ++++++------------- 1 file changed, 16 insertions(+), 33 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/payment/inprogress/WooPosTotalsPaymentInProgressScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/payment/inprogress/WooPosTotalsPaymentInProgressScreen.kt index cf28ea96991..402bf931d89 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/payment/inprogress/WooPosTotalsPaymentInProgressScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/payment/inprogress/WooPosTotalsPaymentInProgressScreen.kt @@ -1,8 +1,6 @@ package com.woocommerce.android.ui.woopos.home.totals.payment.inprogress import androidx.activity.compose.BackHandler -import androidx.compose.animation.AnimatedVisibility -import androidx.compose.animation.core.animateDpAsState import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -14,11 +12,7 @@ import androidx.compose.foundation.layout.size import androidx.compose.material.MaterialTheme import androidx.compose.material.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect 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.text.font.FontWeight @@ -34,7 +28,6 @@ import com.woocommerce.android.ui.woopos.common.composeui.WooPosTheme import com.woocommerce.android.ui.woopos.common.composeui.toAdaptivePadding import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsUIEvent import com.woocommerce.android.ui.woopos.home.totals.WooPosTotalsViewState -import kotlinx.coroutines.delay @Composable fun WooPosPaymentInProgressScreen( @@ -44,11 +37,6 @@ fun WooPosPaymentInProgressScreen( BackHandler { onUIEvent(WooPosTotalsUIEvent.OnBackClicked) } - var enterAnimationStarted by remember { mutableStateOf(false) } - LaunchedEffect(Unit) { - delay(50) - enterAnimationStarted = true - } Box( modifier = Modifier .background(color = WooPosTheme.colors.paymentProcessingBackground) @@ -69,29 +57,24 @@ fun WooPosPaymentInProgressScreen( clipToCompositionBounds = false, clipSpec = LottieClipSpec.Markers("payment_processing_start", "payment_processing_end") ) - val marginBetweenAnimatedIconAndText by animateDpAsState( - targetValue = if (enterAnimationStarted) 0.dp else 256.dp, - ) - Spacer(modifier = Modifier.height(marginBetweenAnimatedIconAndText.toAdaptivePadding())) - AnimatedVisibility(visible = enterAnimationStarted) { - Column(horizontalAlignment = Alignment.CenterHorizontally) { - Text( - text = state.title, - color = WooPosTheme.colors.paymentProcessingText, - style = MaterialTheme.typography.h6, - fontWeight = FontWeight.Normal, - ) - Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) - Text( - text = state.subtitle, - color = WooPosTheme.colors.paymentProcessingText, - style = MaterialTheme.typography.h4, - fontWeight = FontWeight.Bold, - ) - } - } Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Column(horizontalAlignment = Alignment.CenterHorizontally) { + Text( + text = state.title, + color = WooPosTheme.colors.paymentProcessingText, + style = MaterialTheme.typography.h6, + fontWeight = FontWeight.Normal, + ) + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) + Text( + text = state.subtitle, + color = WooPosTheme.colors.paymentProcessingText, + style = MaterialTheme.typography.h4, + fontWeight = FontWeight.Bold, + ) + } } + Spacer(modifier = Modifier.height(16.dp.toAdaptivePadding())) } } From 90a74c4a8cabe4e37804fb7ff90c87e638e0c8bb Mon Sep 17 00:00:00 2001 From: samiuelson Date: Mon, 13 Jan 2025 17:16:15 +0100 Subject: [PATCH 5/8] Satisfy detekt's complaints --- .../android/ui/woopos/home/totals/WooPosTotalsViewModel.kt | 5 ++++- .../android/ui/woopos/home/totals/WooPosTotalsViewState.kt | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt index fa0aa95043a..9f0abdd383f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt @@ -303,7 +303,10 @@ class WooPosTotalsViewModel @Inject constructor( val state = uiState.value if (state is WooPosTotalsViewState.Checkout) { uiState.value = state.copy(totals = Totals.Hidden) - delay(300) + // allow the UI to show "shrinking" exit animation of totals grid before showing + // the "payment in progress" state. + @Suppress("MagicNumber") + delay(384) } uiState.value = buildPaymentInProgressState(paymentState) childrenToParentEventSender.sendToParent(ChildToParentEvent.PaymentInProgress) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt index 78af7a2a4fb..c5f8cb87ad7 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewState.kt @@ -15,6 +15,7 @@ sealed class WooPosTotalsViewState : Parcelable { sealed class Totals : Parcelable { @Parcelize data object Hidden : Totals() + @Parcelize data class Visible( val orderSubtotalText: String, From 17ed1bfff5a36ee5432099d19e66410d88f65aeb Mon Sep 17 00:00:00 2001 From: samiuelson Date: Tue, 14 Jan 2025 11:40:45 +0100 Subject: [PATCH 6/8] Update tests --- .../ui/woopos/home/totals/WooPosTotalsViewModelTest.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt index da23437a1fe..7f514fd9fda 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt @@ -52,6 +52,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat import org.junit.Before @@ -761,6 +762,7 @@ class WooPosTotalsViewModelTest { // WHEN paymentState.value = CardReaderPaymentState.ProcessingPayment.ExternalReaderProcessingPayment("") {} + advanceUntilIdle() // THEN assertThat(vm.state.value).isInstanceOf(WooPosTotalsViewState.PaymentInProgress::class.java) @@ -831,6 +833,7 @@ class WooPosTotalsViewModelTest { // WHEN val vm = createViewModelAndSetupForSuccessfulOrderCreation(controllerFactory = factory) paymentState.value = CardReaderPaymentState.PaymentCapturing.ExternalReaderPaymentCapturing("") + advanceUntilIdle() // THEN val processingState = vm.state.value as WooPosTotalsViewState.PaymentInProgress @@ -860,6 +863,7 @@ class WooPosTotalsViewModelTest { // WHEN val vm = createViewModelAndSetupForSuccessfulOrderCreation(controllerFactory = factory) paymentState.value = CardReaderPaymentState.ProcessingPayment.ExternalReaderProcessingPayment("") {} + advanceUntilIdle() // THEN val processingState = vm.state.value as WooPosTotalsViewState.PaymentInProgress @@ -901,6 +905,7 @@ class WooPosTotalsViewModelTest { paymentState.value = CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment.NonCancelable( errorType = PaymentFlowError.NoNetwork, failedPaymentRetryAction ) + advanceUntilIdle() assertThat(vm.state.value).isInstanceOf(WooPosTotalsViewState.PaymentFailed::class.java) assertTrue( (paymentState.value as CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment).onRetry != null @@ -942,6 +947,7 @@ class WooPosTotalsViewModelTest { paymentState.value = CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment.Cancelable( errorType = PaymentFlowError.NoNetwork, onRetry = null, onCancel = {}, amountWithCurrencyLabel = "" ) + advanceUntilIdle() assertThat(vm.state.value).isInstanceOf(WooPosTotalsViewState.PaymentFailed::class.java) assertTrue( (paymentState.value as CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment).onRetry == null @@ -985,6 +991,8 @@ class WooPosTotalsViewModelTest { paymentState.value = CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment.Cancelable( errorType = PaymentFlowError.NoNetwork, onRetry = null, onCancel = {}, amountWithCurrencyLabel = "" ) + advanceUntilIdle() + assertThat(vm.state.value).isInstanceOf(WooPosTotalsViewState.PaymentFailed::class.java) assertTrue( (paymentState.value as CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment).onRetry == null @@ -992,6 +1000,7 @@ class WooPosTotalsViewModelTest { // WHEN vm.onUIEvent(WooPosTotalsUIEvent.RetryFailedTransactionClicked) + advanceUntilIdle() // THEN verify(childrenToParentEventSender).sendToParent(ChildToParentEvent.ReturnedFromCardReaderPaymentToCheckout) @@ -1023,6 +1032,7 @@ class WooPosTotalsViewModelTest { paymentState.value = CardReaderPaymentState.PaymentFailed.ExternalReaderFailedPayment.NonCancelable( errorType = PaymentFlowError.NoNetwork, {} ) + advanceUntilIdle() assertThat(vm.state.value).isInstanceOf(WooPosTotalsViewState.PaymentFailed::class.java) // WHEN From 057b1e759dfcca67e59c52a20a09b081502c40c9 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 17 Jan 2025 09:00:32 +0100 Subject: [PATCH 7/8] Clean up code after merge --- .../ui/woopos/home/totals/WooPosTotalsScreen.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt index e561ea6d8ae..37c35a5eae9 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt @@ -495,10 +495,12 @@ fun WooPosTotalsScreenPreviewForFreeOrders() { WooPosTheme { WooPosTotalsScreen( modifier = Modifier, - state = WooPosTotalsViewState.Totals( - orderSubtotalText = "$420.00", - orderTotalText = "$462.00", - orderTaxText = "$42.00", + state = WooPosTotalsViewState.Checkout( + totals = Totals.Visible( + orderSubtotalText = "$420.00", + orderTotalText = "$462.00", + orderTaxText = "$42.00", + ), readerStatus = WooPosTotalsViewState.ReaderStatus.Disconnected( title = "Reader not connected", subtitle = "To process this payment, please connect your reader.", From bf88936feb1b78fb53255ce92840e82f4fba5161 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 17 Jan 2025 09:39:29 +0100 Subject: [PATCH 8/8] Update unit tests --- .../ui/woopos/home/totals/WooPosTotalsViewModelTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt index 28131673e75..ebb6d9674b1 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModelTest.kt @@ -1194,8 +1194,8 @@ class WooPosTotalsViewModelTest { ) // THEN - val totals = viewModel.state.value as WooPosTotalsViewState.Totals - assertTrue(totals.isFreeOrder) + val checkout = viewModel.state.value as WooPosTotalsViewState.Checkout + assertTrue(checkout.isFreeOrder) } @Test @@ -1253,8 +1253,8 @@ class WooPosTotalsViewModelTest { ) // THEN - val totals = viewModel.state.value as WooPosTotalsViewState.Totals - assertFalse(totals.isFreeOrder) + val checkout = viewModel.state.value as WooPosTotalsViewState.Checkout + assertFalse(checkout.isFreeOrder) } private fun createNonEmptyOrder() = Order.getEmptyOrder(