Link Wallet payment method list UI #9691
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Diffuse output: APKDEXARSC |
toluo-stripe
force-pushed
the
tolu/link/wallet_list_ui
branch
3 times, most recently
from
a0a90d5
to
b59f90a
Compare
toluo-stripe
changed the title
Add screenshots Minor fixes clean up fix test theme Fix screenshots Remove strings remove unused payment details remove alpha Delete screenshots Update strings.xml Update screenshots Fix screenshots
toluo-stripe
force-pushed
the
tolu/link/wallet_list_ui
branch
from
b59f90a
to
df61258
Compare
amk-stripe
reviewed
Nov 26, 2024
|
|
||
| onCollapsedWalletRow().performClick() | ||
|
|
||
| onWalletAddPaymentMethodRow().assertIsDisplayed().assertHasClickAction() |
There was a problem hiding this comment.
Could you run these tests with shampoo rule and make sure they're not flaky? I've seen test setups like this flake because the compose component is not immediately visible. We could also fix this pre-emptively by adding a composeTestRule.waitUntil call to ensure that the node is visible before asserting anything. You can add these to the helpers themselves (e.g. to the implementation of onWalletAddPaymentMethodRow)
| val paymentDetailsList: List<ConsumerPaymentDetails.PaymentDetails>, | ||
| val selectedItem: ConsumerPaymentDetails.PaymentDetails?, | ||
| val isProcessing: Boolean, | ||
| val isExpanded: Boolean |
There was a problem hiding this comment.
Does isExpanded need to be part of the state that the VM knows about?
Since this seems very specific to the UI and not actually impacted by anything in the VM, I think we could just include it within the composable itself. Just thinking that would be nice for simplicity + division of responsibilities (ie if the VM doesn't need to know about it, we don't need to worry about setting it properly in the VM, etc)
There was a problem hiding this comment.
You're right, I'll take it out
| .animateContentSize() | ||
| ) { | ||
| val selectedItem = state.selectedItem | ||
| if (state.isExpanded || selectedItem == null) { |
There was a problem hiding this comment.
Does this mean that if there is no selected item, then the user won't be able to click the collapse/expand chevron to collapse the PMs list?
That experience sounds potentially annoying -- could we instead just select a PM by default to ensure that the user always has a PM selected to be used in the CollapsedPaymentDetails composable? There are probably other solutions to this too. I feel like we should be able to show the collapsed PM details even if there isn't a selected PM
There was a problem hiding this comment.
The selectedItem can never be null unless there is no payment method available. In the case where there is no payment method, the user will be redirected the CardCreation screen, but that has not been built yet.
samer-stripe
approved these changes
Nov 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Payment methods list on wallet screen
Motivation
JIRA
Testing
Screenshots
Screen.Recording.2024-11-26.at.12.38.15.PM.mov
Changelog