Skip to content
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 method section #851

Merged
merged 23 commits into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
68ef47f
Added ManagePledgePaymentMethodView, created PaymentSource type
Scollaco Sep 20, 2019
05cb1d2
Added ManagePledgePaymentMethodView, created PaymentSource type
Scollaco Sep 20, 2019
b00b1d6
Fixed CreditCard decoding bug / View styling / Added view to ManageVi…
Scollaco Sep 23, 2019
b124a24
Merge conflicts
Scollaco Sep 23, 2019
4786c87
Fixed / incremented tests
Scollaco Sep 23, 2019
786649e
Code cleanup / Added missing tests
Scollaco Sep 23, 2019
48ed87c
Swiftformat
Scollaco Sep 23, 2019
236fb92
Merge branch 'master' into manage-pledge-payment-method-section
Scollaco Sep 23, 2019
e2f6694
Alphabetization
Scollaco Sep 23, 2019
49c7f10
Merge branch 'manage-pledge-payment-method-section' of https://github…
Scollaco Sep 23, 2019
6a10fd5
Merge branch 'master' into manage-pledge-payment-method-section
Scollaco Sep 25, 2019
3d46aa8
Fixed margins of pledge summary view / Updated configureVIews function
Scollaco Sep 25, 2019
576ba5b
Merge remote-tracking branch 'oss/master' into manage-pledge-payment-…
Scollaco Sep 27, 2019
438100f
Set numberOfLines to 0 for title label
Scollaco Sep 27, 2019
ce8263a
Renamed stackView / Used PledgeCreditCardViewModel to bind card infos
Scollaco Sep 27, 2019
60d5a66
Added explicit object type while defining styles
Scollaco Sep 27, 2019
926eeaa
Added closure variable name / removed unecessary style wrapper
Scollaco Sep 27, 2019
aca4598
Removed line that broke VoiceOver
Scollaco Sep 27, 2019
41c1d57
Reverted commented out line
Scollaco Sep 27, 2019
90772ea
Swiftformat
Scollaco Sep 27, 2019
d1a0c1b
Merge branch 'master' into manage-pledge-payment-method-section
Scollaco Sep 27, 2019
dafbf54
[NT-204] Additional fixes (#861)
dusi Oct 1, 2019
3200e23
Merge branch 'master' into manage-pledge-payment-method-section
dusi Oct 1, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Kickstarter-iOS/Views/Cells/PledgeCreditCardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ final class PledgeCreditCardView: UIView {
fatalError("init(coder:) has not been implemented")
}

// MARK: - Configuration

private func configureSubviews() {
_ = self
|> \.accessibilityElements .~ self.subviews
Expand Down Expand Up @@ -101,6 +103,8 @@ final class PledgeCreditCardView: UIView {
|> blackButtonStyle
}

// MARK: - View model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add // MARK: - Configuration??


override func bindViewModel() {
super.bindViewModel()

Expand Down
141 changes: 141 additions & 0 deletions Kickstarter-iOS/Views/Controllers/ManagePledgePaymentMethodView.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import KsApi
import Library
import Prelude
import UIKit

final class ManagePledgePaymentMethodView: UIView {
// MARK: - Properties

private lazy var cardImageView: UIImageView = { UIImageView(frame: .zero) }()
private lazy var cardLabelsStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var expirationDateLabel: UILabel = { UILabel(frame: .zero) }()
private lazy var lastFourLabel: UILabel = { UILabel(frame: .zero) }()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lastFour what? Should we be more explicit? Also the signal & the UI element do not match (not sure if this is again due to copy-pasting stuff around)?

private lazy var paymentMethodAdaptableStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }()
private lazy var titleLabel: UILabel = { UILabel(frame: .zero) }()

private let viewModel: PledgeCreditCardViewModelType = PledgeCreditCardViewModel()

// MARK: - Lifecycle

override init(frame: CGRect) {
super.init(frame: frame)

self.configureViews()
self.setupConstraints()
self.bindViewModel()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any update on this @justinswart ? Thought we talked about swizzling some swizzlers to make this automatic :)

}

required init?(coder _: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

// MARK: - Configuration

public func configure(with card: GraphUserCreditCard.CreditCard) {
self.viewModel.inputs.configureWith(value: card)
}

private func configureViews() {
_ = ([self.lastFourLabel, self.expirationDateLabel], self.cardLabelsStackView)
|> ksr_addArrangedSubviewsToStackView()

_ = ([self.cardImageView, self.cardLabelsStackView], self.paymentMethodAdaptableStackView)
|> ksr_addArrangedSubviewsToStackView()

_ = ([self.titleLabel, self.paymentMethodAdaptableStackView], self.rootStackView)
|> ksr_addArrangedSubviewsToStackView()

_ = (self.rootStackView, self)
|> ksr_addSubviewToParent()
|> ksr_constrainViewToEdgesInParent()
}

// MARK: - Styles

override func bindStyles() {
super.bindStyles()

_ = self.cardImageView
|> cardImageViewStyle

_ = self.cardLabelsStackView
|> verticalStackViewStyle

_ = self.expirationDateLabel
|> cardExpirationDateLabelStyle

_ = self.lastFourLabel
|> cardLastFourLabelStyle

_ = self.paymentMethodAdaptableStackView
|> checkoutAdaptableStackViewStyle(
self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory
)
|> paymentMethodAdaptableStackViewStyle

_ = self.rootStackView
|> checkoutCardStackViewStyle

_ = self.titleLabel
|> titleLabelStyle
}

// MARK: - View model

override func bindViewModel() {
super.bindViewModel()

self.expirationDateLabel.rac.text = self.viewModel.outputs.expirationDateText
self.lastFourLabel.rac.text = self.viewModel.outputs.cardNumberTextShortStyle

self.viewModel.outputs.cardImage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add empty line here to separate rac and output signals?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second thought, would it be worth it adding reactive extension on the UIImageView to being able to bind image property? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I can create a ticket for that to be done in a different PR.

.observeForUI()
.observeValues { [weak self] image in
_ = self?.cardImageView
?|> \.image .~ image
}
}

// MARK: - Functions

private func setupConstraints() {
NSLayoutConstraint.activate([
self.cardImageView.widthAnchor.constraint(
equalToConstant: CheckoutConstants.PaymentSource.ImageView.width
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this being set by the image's intrinsicSize?

If we're setting this explicitly to prevent from a weird UI jump could we also set height or aspect ratio constraint (just to be safe)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constraint is being set here so the views are placed correctly in the stackView. Also, the imageView's contentMode is .scaleAspectFit, so the aspect ratio should be maintained by default.

)
])
}
}

// MARK: - Styles

private let cardExpirationDateLabelStyle: LabelStyle = { label in
label
|> checkoutTitleLabelStyle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using checkoutTitleLabelStyle means we're giving it a headline trait...which we probably only want to do for the title section.

  1. Currently, title is missing headline trait

Screen Shot 2019-09-27 at 2 43 06 PM

  1. While the two labels do have it (they probably shouldn't)

Screen Shot 2019-09-27 at 2 42 53 PM

Screen Shot 2019-09-27 at 2 42 44 PM

|> \.font .~ UIFont.ksr_caption1().bolded
|> \.adjustsFontForContentSizeCategory .~ true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of checkoutTitleLabelStyle

Suggested change
|> \.adjustsFontForContentSizeCategory .~ true

|> \.textColor .~ UIColor.ksr_text_dark_grey_500
}

private let cardLastFourLabelStyle: LabelStyle = { label in
label
|> checkoutTitleLabelStyle
|> \.font .~ UIFont.ksr_subhead().bolded
|> \.adjustsFontForContentSizeCategory .~ true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of checkoutTitleLabelStyle

Suggested change
|> \.adjustsFontForContentSizeCategory .~ true

|> \.textColor .~ UIColor.ksr_soft_black
}

private let paymentMethodAdaptableStackViewStyle: StackViewStyle = { stackView in
stackView
|> \.spacing .~ Styles.grid(2)
}

private let titleLabelStyle: LabelStyle = { label in
label
|> \.textColor .~ UIColor.black
|> \.font .~ UIFont.ksr_subhead().bolded
|> \.adjustsFontForContentSizeCategory .~ true
|> \.text %~ { _ in Strings.Payment_method() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll also need to use numberOfLines .~ 0 here to prevent from trimming the title

|> \.numberOfLines .~ 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ final class ManagePledgeSummaryView: UIView {
private func configureViews() {
_ = (self.rootStackView, self)
|> ksr_addSubviewToParent()
|> ksr_constrainViewToMarginsInParent()
|> ksr_constrainViewToEdgesInParent()

_ = ([self.backerNumberLabel, self.backingDateLabel], self.backerInfoStackView)
|> ksr_addArrangedSubviewsToStackView()
Expand Down
42 changes: 15 additions & 27 deletions Kickstarter-iOS/Views/Controllers/ManagePledgeViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ final class ManagePledgeViewController: UIViewController {
UIImage(in: CGRect(x: 0, y: 0, width: 1, height: 0.5), with: .ksr_dark_grey_400)
}()

private lazy var paymentMethodView: ManagePledgePaymentMethodView = {
ManagePledgePaymentMethodView(frame: .zero)
}()

private lazy var rewardReceivedViewController: ManageViewPledgeRewardReceivedViewController = {
ManageViewPledgeRewardReceivedViewController.instantiate()
}()
Expand All @@ -44,7 +48,7 @@ final class ManagePledgeViewController: UIViewController {
|> \.translatesAutoresizingMaskIntoConstraints .~ false
}()

private let viewModel = ManagePledgeViewModel()
private let viewModel: ManagePledgeViewModelType = ManagePledgeViewModel()

static func instantiate(with project: Project, reward: Reward) -> ManagePledgeViewController {
let manageViewPledgeVC = ManagePledgeViewController.instantiate()
Expand All @@ -62,16 +66,7 @@ final class ManagePledgeViewController: UIViewController {
?|> \.leftBarButtonItem .~ self.closeButton
?|> \.rightBarButtonItem .~ self.menuButton

_ = (self.rootScrollView, self.view)
|> ksr_addSubviewToParent()
|> ksr_constrainViewToEdgesInParent()

_ = (self.rootStackView, self.rootScrollView)
|> ksr_addSubviewToParent()
|> ksr_constrainViewToEdgesInParent()

self.configureViews()
self.configureChildViewControllers()
self.setupConstraints()

self.viewModel.inputs.viewDidLoad()
Expand Down Expand Up @@ -114,7 +109,9 @@ final class ManagePledgeViewController: UIViewController {

self.viewModel.outputs.configurePaymentMethodView
.observeForUI()
.observeValues { _ in }
.observeValues { [weak self] card in
self?.paymentMethodView.configure(with: card)
}

self.viewModel.outputs.configurePledgeSummaryView
.observeForUI()
Expand Down Expand Up @@ -165,21 +162,6 @@ final class ManagePledgeViewController: UIViewController {

// MARK: - Configuration

private func configureChildViewControllers() {
let childViewControllers = [
self.rewardReceivedViewController
]

childViewControllers.forEach { viewController in
self.addChild(viewController)

_ = ([viewController.view], self.rootStackView)
|> ksr_addArrangedSubviewsToStackView()

viewController.didMove(toParent: self)
}
}

func configureWith(project: Project, reward: Reward) {
self.viewModel.inputs.configureWith(project, reward: reward)
}
Expand All @@ -201,7 +183,13 @@ final class ManagePledgeViewController: UIViewController {
|> ksr_addSubviewToParent()
|> ksr_constrainViewToEdgesInParent()

_ = ([self.pledgeSummaryView], self.rootStackView)
let childViews: [UIView] = [
self.pledgeSummaryView,
self.paymentMethodView,
self.rewardReceivedViewController.view
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to add this not only to view hierarchy but to view-controller hierarchy as we do at other places (this is to ensure proper behavior fo the view-controller when being embedded in a parent VC.

self.addChild(viewController)
self.view.addSubview(viewController.view) // or using our functional helper
viewController.didMove(toParent: self)

]

_ = (childViews, self.rootStackView)
|> ksr_addArrangedSubviewsToStackView()
}

Expand Down
7 changes: 7 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,7 @@
D6534D3C22E7898B00E9D279 /* PledgePaymentMethodsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6534D3B22E7898B00E9D279 /* PledgePaymentMethodsViewModel.swift */; };
D6534D3E22E789B900E9D279 /* PledgePaymentMethodsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6534D3D22E789B900E9D279 /* PledgePaymentMethodsViewModelTests.swift */; };
D6560C2A2182361800CD24BC /* PaymentMethodsDataSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65E8F8821821EF500AB9412 /* PaymentMethodsDataSourceTests.swift */; };
D65BF3552334050100B15B25 /* ManagePledgePaymentMethodView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65BF3542334050100B15B25 /* ManagePledgePaymentMethodView.swift */; };
D65BF34F232C1A1C00B15B25 /* ManagePledgeSummaryView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65BF34E232C1A1C00B15B25 /* ManagePledgeSummaryView.swift */; };
D65BF351232FE88300B15B25 /* ManagePledgeSummaryViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65BF350232FE88300B15B25 /* ManagePledgeSummaryViewModel.swift */; };
D65BF353233023E500B15B25 /* ManagePledgeSummaryViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65BF352233023E400B15B25 /* ManagePledgeSummaryViewModelTests.swift */; };
Expand Down Expand Up @@ -2356,6 +2357,9 @@
D6534D3922E7878D00E9D279 /* CreditCard+Utils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CreditCard+Utils.swift"; sourceTree = "<group>"; };
D6534D3B22E7898B00E9D279 /* PledgePaymentMethodsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgePaymentMethodsViewModel.swift; sourceTree = "<group>"; };
D6534D3D22E789B900E9D279 /* PledgePaymentMethodsViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgePaymentMethodsViewModelTests.swift; sourceTree = "<group>"; };
D65BF3542334050100B15B25 /* M

anagePledgePaymentMethodView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagePledgePaymentMethodView.swift; sourceTree = "<group>"; };
D65BF34E232C1A1C00B15B25 /* ManagePledgeSummaryView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagePledgeSummaryView.swift; sourceTree = "<group>"; };
D65BF350232FE88300B15B25 /* ManagePledgeSummaryViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagePledgeSummaryViewModel.swift; sourceTree = "<group>"; };
D65BF352233023E400B15B25 /* ManagePledgeSummaryViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ManagePledgeSummaryViewModelTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2969,6 +2973,8 @@
D65BF34E232C1A1C00B15B25 /* ManagePledgeSummaryView.swift */,
37CA16AC23300376006044F9 /* ManageViewPledgeRewardReceivedViewController.swift */,
37CA16AE2330038E006044F9 /* ManageViewPledgeRewardReceivedViewControllerTests.swift */,
D65BF3542334050100B15B25 /* ManagePledgePaymentMethodView.swift */,
D6A45DB62319A7E1006DDB01 /* ManageViewPledgeViewController.swift */,
D6A45DB62319A7E1006DDB01 /* ManagePledgeViewController.swift */,
D61440FF23200FD3002A6507 /* ManageViewPledgeViewControllerTests.swift */,
D6E7DAF922089F3900689BD6 /* MessageBannerViewController.swift */,
Expand Down Expand Up @@ -5151,6 +5157,7 @@
778CCC5222822B5D00FB8D35 /* SheetOverlayTransitionAnimator.swift in Sources */,
D04F48D41E0313FB00EDC98A /* ActivityProjectStatusCell.swift in Sources */,
A773531F1D5E8AEF0017E239 /* MostPopularSearchProjectCell.swift in Sources */,
D65BF3552334050100B15B25 /* ManagePledgePaymentMethodView.swift in Sources */,
37E9E2A0225EABB000D29DD7 /* AmountInputView.swift in Sources */,
A77352ED1D5E70FC0017E239 /* MostPopularCell.swift in Sources */,
D79F0F3721028C2600D3B32C /* SettingsPrivacyRecommendationCell.swift in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions KsApi/models/Backing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public struct Backing {
public let id: Int
public let locationId: Int?
public let locationName: String?
public let paymentSource: GraphUserCreditCard.CreditCard?
public let pledgedAt: TimeInterval
public let projectCountry: String
public let projectId: Int
Expand Down Expand Up @@ -46,6 +47,7 @@ extension Backing: Argo.Decodable {
let tmp2 = tmp1
<*> json <|? "location_id"
<*> json <|? "location_name"
<*> json <|? "payment_source"
<*> json <| "pledged_at"
<*> json <| "project_country"
<*> json <| "project_id"
Expand Down
15 changes: 15 additions & 0 deletions KsApi/models/BackingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ final class BackingTests: XCTestCase {
"id": 1,
"location_id": 1,
"location_name": "United States",
"payment_source": [
"expiration_date": "2019-09-23",
"id": 20,
"last_four": "1234",
"payment_type": "CREDIT_CARD",
"state": "ACTIVE",
"type": "VISA"
],
"pledged_at": 1_000,
"project_country": "US",
"project_id": 1,
Expand All @@ -20,6 +28,13 @@ final class BackingTests: XCTestCase {
XCTAssertEqual(1.0, backing.value?.amount)
XCTAssertEqual(1, backing.value?.backerId)
XCTAssertEqual(1, backing.value?.id)
XCTAssertEqual("2019-09-23", backing.value?.paymentSource?.expirationDate)
// id is converted to a base64 encoded string to keep graphQL compatibility (used in other API calls).
XCTAssertEqual("VXNlci0yMA==", backing.value?.paymentSource?.id)
XCTAssertEqual("1234", backing.value?.paymentSource?.lastFour)
XCTAssertEqual("CREDIT_CARD", backing.value?.paymentSource?.paymentType)
XCTAssertEqual("ACTIVE", backing.value?.paymentSource?.state)
XCTAssertEqual(GraphUserCreditCard.CreditCardType.visa, backing.value?.paymentSource?.type)
XCTAssertEqual(1, backing.value?.locationId)
XCTAssertEqual("United States", backing.value?.locationName)
XCTAssertEqual(1_000, backing.value?.pledgedAt)
Expand Down
26 changes: 25 additions & 1 deletion KsApi/models/GraphUserCreditCard.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import Argo
import Curry
import Runes

public struct GraphUserCreditCard: Swift.Decodable {
public var storedCards: CreditCardConnection

public struct CreditCard: Swift.Decodable, Equatable {
public var expirationDate: String
public var id: String
public var lastFour: String
public let paymentType: String?
public let state: String?
public var type: CreditCardType?

public var formattedExpirationDate: String {
Expand All @@ -21,7 +27,7 @@ public struct GraphUserCreditCard: Swift.Decodable {
}
}

public enum CreditCardType: String, Decodable, CaseIterable {
public enum CreditCardType: String, Swift.Decodable, CaseIterable {
case amex = "AMEX"
case diners = "DINERS"
case discover = "DISCOVER"
Expand Down Expand Up @@ -55,3 +61,21 @@ public struct GraphUserCreditCard: Swift.Decodable {
public let nodes: [CreditCard]
}
}

extension GraphUserCreditCard.CreditCard: Argo.Decodable {
public static func decode(_ json: JSON) -> Decoded<GraphUserCreditCard.CreditCard> {
return curry(GraphUserCreditCard.CreditCard.init)
<^> json <| "expiration_date"
<*> (json <| "id" <|> (json <| "id" >>- intToString))
<*> json <| "last_four"
<*> json <|? "payment_type"
<*> json <| "state"
<*> json <|? "type"
}
}

extension GraphUserCreditCard.CreditCardType: Argo.Decodable {}

private func intToString(_ input: Int) -> Decoded<String> {
return .success(Data("User-\(input)".utf8).base64EncodedString())
}
Loading