-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[NT-204] Manage pledge payment section apple pay #888
Conversation
…section-apple-pay
…section-apple-pay
…section-apple-pay
…hub.com/kickstarter/ios-oss into manage-pledge-payment-section-apple-pay
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 noticed the screenshots for the Settings PaymentMethodsViewController
were updated. Any idea why? They shouldn't have changed.
"images" : [ | ||
{ | ||
"idiom" : "universal", | ||
"filename" : "icon--apple_pay.pdf" |
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 think we've typically preferred dashes over underscores for asset names, so this should be icon--apple-pay
instead of icon--apple_pay
KsApi/models/BackingTests.swift
Outdated
@@ -14,7 +14,7 @@ final class BackingTests: XCTestCase { | |||
"expiration_date": "2019-09-23", | |||
"id": 20, | |||
"last_four": "1234", | |||
"payment_type": "CREDIT_CARD", | |||
"payment_type": "APPLE_PAY", |
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.
Is it necessary to change this?
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 really, I had changed it just to make sure everything was working. I will revert it 👍
@@ -27,6 +27,11 @@ public struct GraphUserCreditCard: Swift.Decodable { | |||
} | |||
} | |||
|
|||
public enum PaymentType: String, Swift.Decodable, CaseIterable { | |||
case applePay = "APPLE_PAY" | |||
case card = "CREDIT_CARD" |
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 wonder if creditCard
would be more explicit here so we can differentiate between credit cards and debit cards in the future.
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 noticed the screenshots for the Settings
PaymentMethodsViewController
were updated. Any idea why? They shouldn't have changed.
I paired on that with @justinswart on this and it seems that this change is made by XCode when we add a new asset, but visually there's no difference between images.
📲 What
🤔 Why
🛠 How
PaymentType
enum👀 See
📖 Story
✅ Acceptance criteria
Choose a project backed using ApplePay and go to the ManagePledge screen :