-
Notifications
You must be signed in to change notification settings - Fork 294
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
Add Shopper Insights Demo feature #1158
Conversation
Co-authored-by: sshropshire <[email protected]>
import BraintreeVenmo | ||
import BraintreeShopperInsights | ||
|
||
class ShopperInsightsViewController: PaymentButtonBaseViewController { |
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.
Not sure if we want to 🏈 on this based on the PR description but subclassing PaymentButtonBaseViewController
should allow us to get a lot of this for "free". Specially the createButton
and ability to remove the override of the auth init.
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 know this is in the Demo app, so not as merchant impacting, but I personally think it's acceptable to take a little time and do this work now. If folks push back we can lean on the company's leadership principles.
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 is a great callout! Addressed in 92676b6
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.
overall looks good, added my two cents on cleanup work now vs later
import BraintreeVenmo | ||
import BraintreeShopperInsights | ||
|
||
class ShopperInsightsViewController: PaymentButtonBaseViewController { |
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 know this is in the Demo app, so not as merchant impacting, but I personally think it's acceptable to take a little time and do this work now. If folks push back we can lean on the company's leadership principles.
JIRA DTBTSDK-3174
Summary of changes
Checklist
Added a changelog entryAuthors
@scannillo