Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Create empty manage PMs screen #8523

Merged
merged 11 commits into from
May 23, 2024
Merged

Create empty manage PMs screen #8523

merged 11 commits into from
May 23, 2024

Conversation

amk-stripe
Copy link
Contributor

Summary

Create empty manage PMs screen

Motivation

Vertical mode! https://jira.corp.stripe.com/browse/MOBILESDK-2031

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screen recording

empty.manage.screen.mp4

amk-stripe
amk-stripe commented May 22, 2024
View reviewed changes
paymentsheet/src/main/java/com/stripe/android/paymentsheet/navigation/PaymentSheetScreen.kt

@Composable
override fun Content(viewModel: BaseSheetViewModel, modifier: Modifier) {
Text("Manage your saved PMs here")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a follow up, I will make this a column of SavedPaymentMethodRowButtons

amk-stripe
amk-stripe commented May 22, 2024
View reviewed changes
paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/HeaderTextFactory.kt
null
}
}
createForCompleteFlow(screen, isWalletEnabled)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ran into a linter issue about this function being too complex, so separated some of it out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only actually new thing in this file is the PaymentSheetScreen.ManageSavedPaymentMethods branch

amk-stripe
amk-stripe commented May 22, 2024
View reviewed changes
...ntsheet/src/main/java/com/stripe/android/paymentsheet/ui/NewPaymentMethodVerticalLayoutUI.kt Outdated
onItemSelectedListener: (SupportedPaymentMethod) -> Unit,
imageLoader: StripeImageLoader,
modifier: Modifier = Modifier,
) {
Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(12.dp)) {
TextButton(onClick = { onViewMorePaymentMethods() }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

once we display a saved PM on this screen, that saved PM will have a button "View more" which should have this onClick behavior

@amk-stripe amk-stripe marked this pull request as ready for review May 22, 2024 22:32
@amk-stripe amk-stripe requested review from a team as code owners May 22, 2024 22:32
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 22, 2024 •
edited
Loading

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 MiB │     2 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.1 KiB │   8.1 KiB │  0 B 
      res │ 301.5 KiB │ 301.5 KiB │  0 B │   455 KiB │   455 KiB │  0 B 
   native │   7.3 MiB │   7.3 MiB │  0 B │  18.4 MiB │  18.4 MiB │  0 B 
    asset │   1.5 MiB │   1.5 MiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
    other │    87 KiB │    87 KiB │ -1 B │ 161.5 KiB │ 161.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.2 MiB │  12.2 MiB │ -1 B │  25.8 MiB │  25.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21688 │ 21688 │ 0 (+0 -0) 
   types │  6869 │  6869 │ 0 (+0 -0) 
 classes │  5634 │  5634 │ 0 (+0 -0) 
 methods │ 31448 │ 31448 │ 0 (+0 -0) 
  fields │ 18315 │ 18315 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3404 │ 3404 │  0
APK 
   compressed    │   uncompressed   │                                           
──────────┬──────┼───────────┬──────┤                                           
 size     │ diff │ size      │ diff │ path                                      
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 29.1 KiB │ +3 B │    64 KiB │  0 B │ ∆ META-INF/CERT.SF                        
  1.2 KiB │ -2 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
    271 B │ -1 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
 25.9 KiB │ -1 B │  63.9 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 56.4 KiB │ -1 B │ 129.2 KiB │  0 B │ (total)

@amk-stripe amk-stripe marked this pull request as draft May 22, 2024 22:41
@amk-stripe amk-stripe marked this pull request as ready for review May 22, 2024 22:54
@amk-stripe amk-stripe marked this pull request as draft May 22, 2024 23:50
amk-stripe
amk-stripe commented May 23, 2024
View reviewed changes
paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/PaymentMethodVerticalLayoutUI.kt
import com.stripe.android.uicore.image.StripeImageLoader

@Composable
internal fun PaymentMethodVerticalLayoutUI(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created this new layout for the list of both saved PMs + new PMs (using the existing NewPaymentMethodVerticalLayoutUI). Once there is a saved PM displayed in this layout, it will have a "View more" button and clicking that button should call the onViewMorePaymentMethods function that is included here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, we will probably keep doing this for a while. I'm hoping to refactor some stuff in this realm too. But I'm on board with this as iterative progress!

@amk-stripe amk-stripe marked this pull request as ready for review May 23, 2024 16:09
jaynewstrom-stripe
jaynewstrom-stripe reviewed May 23, 2024
View reviewed changes
paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/HeaderTextFactory.kt
StripeR.string.stripe_title_update_card
}
is PaymentSheetScreen.ManageSavedPaymentMethods -> {
R.string.stripe_paymentsheet_select_payment_method
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a test for this (if it's the final variant).

I've been skipping tests for all the stuff I've added, since it isn't done yet.

@amk-stripe amk-stripe enabled auto-merge (squash) May 23, 2024 16:38
@amk-stripe amk-stripe merged commit b487d88 into master May 23, 2024
12 checks passed
@amk-stripe amk-stripe deleted the show-empty-manage-screen branch May 23, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaynewstrom-stripe jaynewstrom-stripe jaynewstrom-stripe approved these changes

Assignees
No one assigned
Labels
vertical-mode
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@amk-stripe @jaynewstrom-stripe