-
Notifications
You must be signed in to change notification settings - Fork 658
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
Make CustomerSheet.Configuration non nullable. #7627
Make CustomerSheet.Configuration non nullable. #7627
Conversation
@@ -19,7 +19,7 @@ import com.stripe.android.utils.rememberActivityOrNull | |||
@ExperimentalCustomerSheetApi | |||
@Composable | |||
fun rememberCustomerSheet( | |||
configuration: CustomerSheet.Configuration = CustomerSheet.Configuration(), | |||
configuration: CustomerSheet.Configuration, |
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.
I’m not sure what to think about this. Should we relax this to a nullable config and create a default configuration internally, with the application label as the merchant name?
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.
Totally possible. This is what we do for PaymentSheet.
If everything else in this sounds good to you, I think we should definitely do that and ship it.
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.
Let’s do it!
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.
Chatted offline. Slightly more inclined to leave it as not optional since the non compose way to instantiate CustomerSheet already requires a config.
b7e0545
to
f8444b5
Compare
Summary
Make customer sheet configuration non null. This is to ensure defaults live in one place. Similar PR done in #7625