-
Notifications
You must be signed in to change notification settings - Fork 629
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
Clear error message when new payment method is selected #3978
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also make sense if there is a network and other forms of error?
I think so, as long as it stays on screen until the user takes an action, which is the case with this change. What do you think? |
591ed8c
to
034e7e0
Compare
...s-core/src/main/java/com/stripe/android/paymentsheet/PaymentSheetAddPaymentMethodFragment.kt
Outdated
Show resolved
Hide resolved
payments-core/src/main/java/com/stripe/android/paymentsheet/PaymentSheetViewModel.kt
Outdated
Show resolved
Hide resolved
payments-core/src/main/java/com/stripe/android/paymentsheet/BaseAddPaymentMethodFragment.kt
Show resolved
Hide resolved
@@ -177,12 +177,12 @@ internal class PaymentSheetActivity : BaseSheetActivity<PaymentSheetResult>() { | |||
|
|||
override fun onBackPressed() { | |||
if (supportFragmentManager.backStackEntryCount > 0) { | |||
updateErrorMessage(null) | |||
updateErrorMessage() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a stylistic thing here, I realize you can use default arguments, but without the null parameter passed in here, it is hard to realize that the side-effect of this function is that the error message will be cleared.
payments-core/src/main/java/com/stripe/android/paymentsheet/PaymentSheetActivity.kt
Show resolved
Hide resolved
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If order is important here, a comment would be helpful.
_viewState.value = PaymentSheetViewState.Reset(null) | ||
if (this.checkoutIdentifier != checkoutIdentifier) { | ||
// Clear out any previous errors before setting the new button to get updates. | ||
_viewState.value = PaymentSheetViewState.Reset() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comments says it clears previous errors, but it seems like ti doesn't because in the activities we are also clearing the error message?
Summary
Motivation
Previously the error message was shown even after the user selects a different payment method.
Testing