Skip to content

Use NavigationManager in LinkActivity #10329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Mar 6, 2025

Summary

  • Uses NavigationManager to handle navigation ((flow based navigation communication via a DI shared instance).
  • Removes navController<> viewModel coupling
  • Replaces Compose signaling to VM to trigger navigation by providing the initial destination on the state, removing one navigation step from the stack
  • Simplifies back handling.

Motivation

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

)
)
}
navigationManager.tryNavigateBack()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goBack will go away in favor of specific screens using NavigationManager.

Comment on lines -226 to -225
fun linkScreenCreated() {
viewModelScope.launch {
val currentRoute = navController?.currentBackStackEntry?.destination?.route
if (currentRoute == null || currentRoute == LinkScreen.Loading.route) {
navigateToLinkScreen()
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed anymore:UI does not need to signal component rendering to trigger loading. Instead, we'll wait to render the component until we have the necessary info loaded.

Comment on lines 227 to 241
private suspend fun buildFullScreenState(): ScreenState.FullScreen {
val accountStatus = linkAccountManager.accountStatus.first()
val screen = when (accountStatus) {
AccountStatus.Verified -> {
LinkScreen.Wallet
}
AccountStatus.NeedsVerification, AccountStatus.VerificationStarted -> {
LinkScreen.Verification
}
AccountStatus.SignedOut, AccountStatus.Error -> {
LinkScreen.SignUp
return ScreenState.FullScreen(
initialDestination = when (accountStatus) {
AccountStatus.Verified -> {
LinkScreen.Wallet
}
AccountStatus.NeedsVerification, AccountStatus.VerificationStarted -> {
LinkScreen.Verification
}
AccountStatus.SignedOut, AccountStatus.Error -> {
LinkScreen.SignUp
}
}
}
navigate(screen, clearStack = true, launchSingleTop = true)
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial step to consolidate state build logic into ScreenState.

Copy link
Contributor

github-actions bot commented Mar 6, 2025

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   2.1 MiB │   2.1 MiB │  0 B │   4.3 MiB │   4.3 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 302.9 KiB │ 302.9 KiB │  0 B │   457 KiB │   457 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.7 KiB │   7.7 KiB │  0 B │   7.4 KiB │   7.4 KiB │  0 B 
    other │  95.7 KiB │  95.7 KiB │ -1 B │ 183.5 KiB │ 183.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.8 MiB │   9.8 MiB │ -1 B │  21.8 MiB │  21.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20668 │ 20668 │ 0 (+0 -0) 
   types │  6493 │  6493 │ 0 (+0 -0) 
 classes │  5259 │  5259 │ 0 (+0 -0) 
 methods │ 31489 │ 31489 │ 0 (+0 -0) 
  fields │ 18220 │ 18220 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3646 │ 3646 │  0
APK
   compressed    │  uncompressed   │                     
──────────┬──────┼──────────┬──────┤                     
 size     │ diff │ size     │ diff │ path                
──────────┼──────┼──────────┼──────┼─────────────────────
 29.2 KiB │ -3 B │ 64.6 KiB │  0 B │ ∆ META-INF/CERT.SF  
  1.2 KiB │ +2 B │  1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA 
──────────┼──────┼──────────┼──────┼─────────────────────
 30.4 KiB │ -1 B │ 65.8 KiB │  0 B │ (total)

@carlosmuvi-stripe carlosmuvi-stripe force-pushed the carlosmuvi/move-navigation-to-stripeui branch from 064d2dc to 1578ea5 Compare March 6, 2025 17:41
Base automatically changed from carlosmuvi/move-navigation-to-stripeui to master March 6, 2025 20:28
@carlosmuvi-stripe carlosmuvi-stripe force-pushed the carlosmuvi/use-navigation-manager branch from 36cda1b to b6aaa1f Compare March 6, 2025 22:11
Comment on lines -83 to -95
LaunchedEffect(Unit) {
onNavControllerCreated(navController)
onLinkScreenScreenCreated()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compose does not need to signal the VM to initiate rendering anymore - instead the state has the initialDestination bundled.

Comment on lines 86 to 88
PopUpToBehavior.Start -> popUpTo(
navHostController.graph.startDestinationId
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added clear stack navigation support!

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review March 7, 2025 18:49
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners March 7, 2025 18:49
toluo-stripe
toluo-stripe previously approved these changes Mar 10, 2025
…gation-manager

# Conflicts:
#	paymentsheet/src/main/java/com/stripe/android/link/LinkActivityViewModel.kt
#	paymentsheet/src/main/java/com/stripe/android/link/ui/FullScreenContent.kt
#	paymentsheet/src/main/java/com/stripe/android/link/ui/LinkContent.kt
…gation-manager

# Conflicts:
#	paymentsheet/src/main/java/com/stripe/android/link/LinkActivityViewModel.kt
@carlosmuvi-stripe carlosmuvi-stripe requested a review from a team as a code owner April 16, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants