From e231ca49fe3854422966ee171dd2bfdc2d24a007 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Thu, 29 Feb 2024 10:23:17 -0500 Subject: [PATCH 1/3] Don't count bottom sheets towards the back stack --- .../financialconnections/ui/components/TopAppBar.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt index 231f9f96509..ea4837a90b4 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt @@ -34,6 +34,7 @@ import androidx.compose.ui.unit.dp import androidx.navigation.NavController import androidx.navigation.NavHostController import com.stripe.android.financialconnections.R +import com.stripe.android.financialconnections.navigation.bottomsheet.BottomSheetNavigator import com.stripe.android.financialconnections.ui.FinancialConnectionsPreview import com.stripe.android.financialconnections.ui.LocalNavHostController import com.stripe.android.financialconnections.ui.LocalReducedBranding @@ -198,8 +199,12 @@ internal val LazyListState.elevation: Dp private fun NavHostController.collectCanGoBackAsState(): State { val canGoBack = remember { mutableStateOf(false) } DisposableEffect(Unit) { - val listener = NavController.OnDestinationChangedListener { controller, _, _ -> - canGoBack.value = controller.previousBackStackEntry != null + val listener = NavController.OnDestinationChangedListener { controller, destination, _ -> + if (destination.navigatorName == BottomSheetNavigator::class.java.simpleName) { + // We're looking at a bottom sheet, so don't change the back button + } else { + canGoBack.value = controller.previousBackStackEntry != null + } } addOnDestinationChangedListener(listener) onDispose { From 0cb4b35ea9820b2ab9ec9e19e11a04c91c15b5ac Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Thu, 29 Feb 2024 12:46:02 -0500 Subject: [PATCH 2/3] Add Maestro test for returning user flow The test checks the back button visibility --- .../ui/components/TopAppBar.kt | 10 +++- ...tInstitution-Networking-ReturningUser.yaml | 51 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 maestro/financial-connections/Testmode-PaymentIntent-TestInstitution-Networking-ReturningUser.yaml diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt index ea4837a90b4..3534acb7de7 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt @@ -24,10 +24,14 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment +import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawBehind import androidx.compose.ui.geometry.CornerRadius +import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.testTagsAsResourceId import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -100,6 +104,7 @@ internal fun FinancialConnectionsTopAppBar( ) } +@OptIn(ExperimentalComposeUiApi::class) @Composable private fun BackButton( scope: CoroutineScope, @@ -117,7 +122,10 @@ private fun BackButton( Icon( imageVector = Icons.Filled.ArrowBack, contentDescription = "Back icon", - tint = FinancialConnectionsTheme.colors.iconDefault + tint = FinancialConnectionsTheme.colors.iconDefault, + modifier = Modifier + .testTag("top-app-bar-back-button") + .semantics { testTagsAsResourceId = true } ) } } diff --git a/maestro/financial-connections/Testmode-PaymentIntent-TestInstitution-Networking-ReturningUser.yaml b/maestro/financial-connections/Testmode-PaymentIntent-TestInstitution-Networking-ReturningUser.yaml new file mode 100644 index 00000000000..856e07c0842 --- /dev/null +++ b/maestro/financial-connections/Testmode-PaymentIntent-TestInstitution-Networking-ReturningUser.yaml @@ -0,0 +1,51 @@ +appId: com.stripe.android.financialconnections.example +tags: + - all + - edge + - testmode-payments +--- +- startRecording: ${'/tmp/test_results/testmode-paymentintent-testinstitution-networking-returning-user-' + new Date().getTime()} +- clearState +- openLink: stripeconnectionsexample://playground?flow=PaymentIntent&financial_connections_override_native=native&merchant=networking_testmode&permissions=transactions,payment_method +- tapOn: + id: "Customer email setting" +- inputText: "email@email.com" +- hideKeyboard +- tapOn: + id: "connect_accounts" +# Wait until the consent button is visible +- extendedWaitUntil: + visible: + id: "consent_cta" + timeout: 30000 +- tapOn: + id: "consent_cta" +# LINK WARMUP PANE +- extendedWaitUntil: + visible: + id: "existing_email-button" + timeout: 5000 +- assertNotVisible: + id: "top-app-bar-back-button" +- tapOn: + id: "existing_email-button" +# OTP SCREEN +- extendedWaitUntil: + visible: "Save your account to Link" + timeout: 5000 +- assertVisible: + id: "top-app-bar-back-button" +- inputText: "000000" +# SELECT ACCOUNT +- extendedWaitUntil: + visible: "Select account" + timeout: 5000 +- tapOn: "Account Closed" +- tapOn: "Connect account" +# EMAIL OTP SCREEN +- waitForAnimationToEnd +- inputText: "000000" +# CONFIRM AND COMPLETE +- waitForAnimationToEnd +- assertVisible: "Success" +- stopRecording From 23fc3527a7391a17f282c441e78563807c697236 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Thu, 29 Feb 2024 14:37:09 -0500 Subject: [PATCH 3/3] Address code review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename `collectCanGoBackAsState` to `collectCanShowBackIconAsState`, and rename `showBack` to `allowBackNavigation`. The latter is meant to convey that it’s more about business logic rather than back stack state. --- .../features/accountpicker/AccountPickerScreen.kt | 2 +- .../features/attachpayment/AttachPaymentScreen.kt | 2 +- .../features/common/SharedPartnerAuth.kt | 2 +- .../features/error/ErrorScreen.kt | 2 +- .../linkaccountpicker/LinkAccountPickerScreen.kt | 2 +- .../LinkStepUpVerificationScreen.kt | 2 +- .../NetworkingLinkSignupScreen.kt | 2 +- .../NetworkingSaveToLinkVerificationScreen.kt | 1 - .../features/success/SuccessContent.kt | 2 +- .../ui/components/TopAppBar.kt | 14 +++++++------- 10 files changed, 15 insertions(+), 16 deletions(-) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/accountpicker/AccountPickerScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/accountpicker/AccountPickerScreen.kt index ef77f47f4f3..19bb34ee754 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/accountpicker/AccountPickerScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/accountpicker/AccountPickerScreen.kt @@ -166,7 +166,7 @@ private fun AccountPickerMainContent( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = false, + allowBackNavigation = false, onCloseClick = onCloseClick, elevation = lazyListState.elevation ) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/attachpayment/AttachPaymentScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/attachpayment/AttachPaymentScreen.kt index 26ba5f531d6..49acbbdf826 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/attachpayment/AttachPaymentScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/attachpayment/AttachPaymentScreen.kt @@ -47,7 +47,7 @@ private fun AttachPaymentContent( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = false, + allowBackNavigation = false, onCloseClick = onCloseClick ) } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/common/SharedPartnerAuth.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/common/SharedPartnerAuth.kt index ce4267f1097..b0e88ba9ffb 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/common/SharedPartnerAuth.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/common/SharedPartnerAuth.kt @@ -245,7 +245,7 @@ private fun SharedPartnerAuthContentWrapper( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = canNavigateBack, + allowBackNavigation = canNavigateBack, onCloseClick = onCloseClick ) } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/error/ErrorScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/error/ErrorScreen.kt index 7987c264a09..a7311756308 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/error/ErrorScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/error/ErrorScreen.kt @@ -135,7 +135,7 @@ private fun FullScreenError( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = showBack, + allowBackNavigation = showBack, onCloseClick = onCloseClick ) } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkaccountpicker/LinkAccountPickerScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkaccountpicker/LinkAccountPickerScreen.kt index 3e4574bc69d..a0fbada5b06 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkaccountpicker/LinkAccountPickerScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkaccountpicker/LinkAccountPickerScreen.kt @@ -184,7 +184,7 @@ private fun LinkAccountPickerMainContent( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = false, + allowBackNavigation = false, elevation = scrollState.elevation, onCloseClick = onCloseClick ) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkstepupverification/LinkStepUpVerificationScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkstepupverification/LinkStepUpVerificationScreen.kt index b1736cf7abf..ad6e60f8d2c 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkstepupverification/LinkStepUpVerificationScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/linkstepupverification/LinkStepUpVerificationScreen.kt @@ -75,7 +75,7 @@ private fun LinkStepUpVerificationContent( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = false, + allowBackNavigation = false, elevation = lazyListState.elevation, onCloseClick = onCloseClick ) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupScreen.kt index a20e0faa6ab..9e24c296be9 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkinglinksignup/NetworkingLinkSignupScreen.kt @@ -163,7 +163,7 @@ private fun NetworkingLinkSignupMainContent( topBar = { FinancialConnectionsTopAppBar( elevation = scrollState.elevation, - showBack = false, + allowBackNavigation = false, onCloseClick = onCloseClick, ) } diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkingsavetolinkverification/NetworkingSaveToLinkVerificationScreen.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkingsavetolinkverification/NetworkingSaveToLinkVerificationScreen.kt index 4a4dace94b7..1fd42a11e63 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkingsavetolinkverification/NetworkingSaveToLinkVerificationScreen.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/networkingsavetolinkverification/NetworkingSaveToLinkVerificationScreen.kt @@ -71,7 +71,6 @@ private fun NetworkingSaveToLinkVerificationContent( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = true, onCloseClick = onCloseClick, elevation = rememberLazyListState().elevation ) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/success/SuccessContent.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/success/SuccessContent.kt index b3e58dbe3b5..02f3bfbf397 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/features/success/SuccessContent.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/features/success/SuccessContent.kt @@ -96,7 +96,7 @@ private fun SuccessContentInternal( FinancialConnectionsScaffold( topBar = { FinancialConnectionsTopAppBar( - showBack = false, + allowBackNavigation = false, onCloseClick = onCloseClick, elevation = scrollState.elevation ) diff --git a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt index 3534acb7de7..52a2cdef1c2 100644 --- a/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt +++ b/financial-connections/src/main/java/com/stripe/android/financialconnections/ui/components/TopAppBar.kt @@ -61,14 +61,14 @@ internal fun FinancialConnectionsTopAppBar( hideStripeLogo: Boolean = LocalReducedBranding.current, testMode: Boolean = LocalTestMode.current, elevation: Dp = 0.dp, - showBack: Boolean = true, + allowBackNavigation: Boolean = true, onCloseClick: () -> Unit ) { val onBackPressedDispatcher = LocalOnBackPressedDispatcherOwner.current val localBackPressed = onBackPressedDispatcher?.onBackPressedDispatcher val navController = LocalNavHostController.current - val canGoBack by navController.collectCanGoBackAsState() + val canShowBackIcon by navController.collectCanShowBackIconAsState() val keyboardController = rememberKeyboardController() val scope = rememberCoroutineScope() @@ -81,7 +81,7 @@ internal fun FinancialConnectionsTopAppBar( ) }, elevation = elevation, - navigationIcon = if (canGoBack && showBack) { + navigationIcon = if (canShowBackIcon && allowBackNavigation) { { BackButton( scope = scope, @@ -204,14 +204,14 @@ internal val LazyListState.elevation: Dp } @Composable -private fun NavHostController.collectCanGoBackAsState(): State { - val canGoBack = remember { mutableStateOf(false) } +private fun NavHostController.collectCanShowBackIconAsState(): State { + val canShowBackIcon = remember { mutableStateOf(false) } DisposableEffect(Unit) { val listener = NavController.OnDestinationChangedListener { controller, destination, _ -> if (destination.navigatorName == BottomSheetNavigator::class.java.simpleName) { // We're looking at a bottom sheet, so don't change the back button } else { - canGoBack.value = controller.previousBackStackEntry != null + canShowBackIcon.value = controller.previousBackStackEntry != null } } addOnDestinationChangedListener(listener) @@ -219,7 +219,7 @@ private fun NavHostController.collectCanGoBackAsState(): State { removeOnDestinationChangedListener(listener) } } - return canGoBack + return canShowBackIcon } @Preview(group = "Components", name = "TopAppBar")