-
Notifications
You must be signed in to change notification settings - Fork 44
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
PI-1738: Billing Form component: Standard #144
Changes from 52 commits
a3ddaae
1d5aae9
02641fb
8bb68d2
0201e01
0f918de
4373e90
e2efe8f
86fed89
77e1998
3551409
311a63a
7a61071
88bc829
d0f46cf
f94e42a
5b742d9
9e5b7a2
ca3e7a9
42a7f4e
a7e0c31
d56979d
1674596
ba10ffe
31c2a02
44fe199
7b6b139
64559f6
fc01d9c
96ee26c
f1dbfe5
347c106
e77207e
52267b1
4045c05
d554c26
3fa1922
db069fa
d1df920
ecea7cd
b366298
0634474
dbd2ef5
dd4a5b2
0c01976
5e74377
61b42c0
3b53468
07a18fd
bad5997
1dedcc8
a57c2d7
b538fde
abd3e24
bbcdcba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
github "marmelroy/PhoneNumberKit" "3.3.3" | ||
github "marmelroy/PhoneNumberKit" "3.3.7" | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import UIKit | ||
|
||
extension UIColor { | ||
public static let brandeisBlue = UIColor(hex: "#0B5FF0") | ||
public static let doveGray = UIColor(hex: "#636363") | ||
public static let codGray = UIColor(hex: "#141414") | ||
public static let mediumGray = UIColor(hex: "#8A8A8A") | ||
public static let tallPoppyRed = UIColor(hex: "#AD283E") | ||
} | ||
|
||
extension UIColor { | ||
convenience init(hex: String, alpha: CGFloat = 1.0) { | ||
var cString = hex | ||
.trimmingCharacters(in: .whitespacesAndNewlines) | ||
.replacingOccurrences(of: " ", with: "") | ||
.uppercased() | ||
|
||
if cString.hasPrefix("#") { cString.removeFirst() } | ||
|
||
guard cString.count == 6 else { | ||
self.init(hex: "ff0000") // gray color | ||
return | ||
} | ||
|
||
var rgbValue: UInt64 = 0 | ||
Scanner(string: cString).scanHexInt64(&rgbValue) | ||
|
||
self.init ( | ||
red: CGFloat((rgbValue & 0xFF0000) >> 16) / 255.0, | ||
green: CGFloat((rgbValue & 0x00FF00) >> 8) / 255.0, | ||
blue: CGFloat(rgbValue & 0x0000FF) / 255.0, | ||
alpha: CGFloat(1.0) | ||
) | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import UIKit | ||
|
||
extension UIFont { | ||
|
||
// load framework font in application | ||
public static let loadAllFonts: () = { | ||
for style in GraphikStyle.allCases { | ||
style.fontName.register(for: CheckoutTheme.self, withExtension: "otf") | ||
} | ||
}() | ||
|
||
} | ||
|
||
extension UIFont { | ||
|
||
enum GraphikStyle: String, CaseIterable { | ||
case regularIt | ||
case thin | ||
case thinIt | ||
case extrabldIt | ||
case lightIt | ||
case black | ||
case medium | ||
case extrabld | ||
case boldIt | ||
case regular | ||
case blackIt | ||
case mediumIt | ||
case bold | ||
case light | ||
case semiboldIt | ||
case semibold | ||
var fontName: String { | ||
return "GraphikLCG-\(self.rawValue.capitalized)" | ||
} | ||
} | ||
|
||
convenience init(graphikStyle: GraphikStyle, size: CGFloat) { | ||
self.init(name: graphikStyle.fontName, size: size)! | ||
} | ||
|
||
} | ||
|
||
|
||
extension String { | ||
//MARK: - Make custom font bundle register to framework | ||
|
||
func register(for type: AnyClass, withExtension: String) { | ||
let bundle = getBundle(forClass: type) | ||
|
||
if let pathForResourceString = bundle.url(forResource: self, withExtension: withExtension), | ||
let fontData = NSData(contentsOf: pathForResourceString), let dataProvider = CGDataProvider.init(data: fontData) { | ||
let fontRef = CGFont.init(dataProvider) | ||
var errorRef: Unmanaged<CFError>? = nil | ||
if CTFontManagerRegisterGraphicsFont(fontRef!, &errorRef) == false { | ||
print("Failed to register font - register graphics font failed - this font may have already been registered in the main bundle.") | ||
} | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,24 +2,45 @@ import Foundation | |
import UIKit | ||
|
||
extension String { | ||
|
||
private func getBundle(forClass: AnyClass) -> Foundation.Bundle { | ||
#if SWIFT_PACKAGE | ||
func getBundle(forClass: Swift.AnyClass) -> Foundation.Bundle { | ||
#if SWIFT_PACKAGE | ||
let baseBundle = Bundle.module | ||
#else | ||
#else | ||
let baseBundle = Foundation.Bundle(for: forClass) | ||
#endif | ||
#endif | ||
let path = baseBundle.path(forResource: "Frames", ofType: "bundle") | ||
return path == nil ? baseBundle : Foundation.Bundle(path: path!)! | ||
} | ||
|
||
func localized(forClass: AnyClass, comment: String = "") -> String { | ||
func localized(forClass: Swift.AnyClass, comment: String = "") -> String { | ||
let bundle = getBundle(forClass: forClass) | ||
return NSLocalizedString(self, bundle: bundle, comment: "") | ||
} | ||
|
||
func image(forClass: AnyClass) -> UIImage { | ||
let bundle = getBundle(forClass: forClass) | ||
return UIImage(named: self, in: bundle, compatibleWith: nil) ?? UIImage() | ||
} | ||
|
||
//TODO: migrate to assets | ||
//https://www.hackingwithswift.com/example-code/core-graphics/how-to-render-a-pdf-to-an-image | ||
func vectorPDFImage(forClass: AnyClass) -> UIImage? { | ||
let bundle = getBundle(forClass: forClass) | ||
guard let urlPath = bundle.url(forResource: self, withExtension: "pdf") else { return nil } | ||
guard let document = CGPDFDocument(urlPath as CFURL) else { return nil } | ||
guard let page = document.page(at: 1) else { return nil } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As I was documenting this comment, I realised using assets in SPM is more what you need and all of this could be avoided to begin with? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: a new ticket to refactor and test this change |
||
|
||
let pageRect = page.getBoxRect(.mediaBox) | ||
let renderer = UIGraphicsImageRenderer(size: pageRect.size) | ||
|
||
return renderer.image { | ||
UIColor.white.set() | ||
$0.fill(pageRect) | ||
$0.cgContext.translateBy(x: 0.0, y: pageRect.size.height) | ||
$0.cgContext.scaleBy(x: 1.0, y: -1.0) | ||
$0.cgContext.drawPDFPage(page) | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,16 @@ | ||
|
||
"missingBillingFormFullName" = "Missing Name"; | ||
"missingBillingFormAddressLine1" = "Missing Address Line 1"; | ||
"missingBillingFormAddressLine2" = "Missing Address Line 2"; | ||
"missingBillingFormCity" = "Missing City"; | ||
"missingBillingFormState" = "Missing State"; | ||
"missingBillingFormPostcode" = "Missing Postcode"; | ||
"missingBillingFormPhoneNumber" = "Missing Phone Number"; | ||
"missingBillingFormCountry" = "Missing Country"; | ||
"billingFormPhoneNumberHint" = "Phone Number"; | ||
"billingAddressTitle" = "Billing address"; | ||
"city" = "City"; | ||
"cancel" = "Cancel"; | ||
"done" = "Done"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a tracking option to ensure the localisation is being followed after this PR is merged? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be solved by @patrick-hoban-cko and @deepesh-vasthimal-cko |
||
"cardNumber" = "Card Number*"; | ||
"cardholderName" = "Cardholder's name"; | ||
"cardholderNameRequired" = "Cardholder's name*"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,13 +43,17 @@ public class CardViewController: UIViewController, | |
var topConstraint: NSLayoutConstraint? | ||
|
||
private var loggedForCurrentCorrelationID = false | ||
|
||
public var isNewUI = false | ||
// TODO: [Will updated in the next ticket]. | ||
private var countryCode = 0 | ||
ehab-al-cko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// MARK: - Initialization | ||
|
||
|
||
|
||
/// Returns a newly initialized view controller with the cardholder's name and billing details | ||
/// state specified. You can specified the region using the Iso2 region code ("UK" for "United Kingdom") | ||
public init(checkoutApiClient: CheckoutAPIClient, cardHolderNameState: InputState, | ||
billingDetailsState: InputState, defaultRegionCode: String? = nil) { | ||
UIFont.loadAllFonts | ||
self.checkoutApiClient = checkoutApiClient | ||
self.cardHolderNameState = cardHolderNameState | ||
self.billingDetailsState = billingDetailsState | ||
|
@@ -160,8 +164,12 @@ public class CardViewController: UIViewController, | |
} | ||
|
||
@objc func onTapAddressView() { | ||
navigationController?.pushViewController(addressViewController, animated: true) | ||
checkoutApiClient?.logger.log(.billingFormPresented) | ||
guard isNewUI, let viewController = BillingFormFactory.getBillingFormViewController(delegate: self).1 else { | ||
navigationController?.pushViewController(addressViewController, animated: true) | ||
return | ||
} | ||
navigationController?.present(viewController, animated: true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Outside of the scope of this PR, but @harry-brown-cko , @deepesh-vasthimal-cko , I think the animated value should follow the decisions of the user of the SDK. I would say we should offer it as a customisable option, that defaults from user device There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feedback from @patrick-hoban-cko |
||
loggedForCurrentCorrelationID = true | ||
} | ||
|
||
|
@@ -339,3 +347,20 @@ public class CardViewController: UIViewController, | |
} | ||
} | ||
} | ||
|
||
|
||
extension CardViewController: BillingFormViewModelDelegate { | ||
func updateCountryCode(code: Int) { | ||
countryCode = code | ||
} | ||
|
||
func onTapDoneButton(address: CkoAddress, phone: CkoPhoneNumber) { | ||
billingDetailsAddress = address | ||
billingDetailsPhone = phone | ||
let value = "\(address.addressLine1 ?? ""), \(address.city ?? "")" | ||
melting-snowman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cardView.billingDetailsInputView.value.text = value | ||
validateFieldsValues() | ||
// return to CardViewController | ||
self.topConstraint?.isActive = false | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import UIKit | ||
|
||
struct BillingFormFactory { | ||
// in order | ||
static let styles: [BillingFormTextFieldCellStyle] = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could these styles just be an enum case? Lets imagine:
My initial thought is it would reduce a lot of the syntactic sugar you currently have and may increase configuration options as well as extensibility. Additionally, you could completely drop this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, before resolving can we discuss on it? Or at least open a discussion forum somewhere (Git, Slack, anything), so we can try and not forget about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ehab-al-cko to Create Tech investment ticket to address this in new PR |
||
DefaultBillingFormFullNameCellStyle(), | ||
DefaultBillingFormAddressLine1CellStyle(), | ||
DefaultBillingFormAddressLine2CellStyle(), | ||
DefaultBillingFormCityCellStyle(), | ||
DefaultBillingFormStateCellStyle(), | ||
DefaultBillingFormPostcodeCellStyle(), | ||
DefaultBillingFormCountryCellStyle(), | ||
DefaultBillingFormPhoneNumberCellStyle() | ||
] | ||
|
||
static func getBillingFormViewController(delegate: BillingFormViewModelDelegate) -> (BillingFormViewModelDelegate?, UIViewController?) { | ||
let style = DefaultBillingFormStyle() | ||
guard !style.fields.isEmpty else { return (nil, nil) } | ||
let viewModel = DefaultBillingFormViewModel(style: style, initialCountry: "", delegate: delegate) | ||
return (viewModel.delegate, BillingFormViewController(viewModel: viewModel)) | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import Foundation | ||
import UIKit | ||
|
||
public protocol BillingFormHeaderCellStyle { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not certain if the comment above the protocol provides any value? The type naming should clearly convey it, and if we want the user of the syntax to flag some helper comment, we should at least use the description marker ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not going to lie, going ahead with these comments above the class/protocol declaration makes our publicly exposed code look unfinished. It would be nice if our exposed code would make CKO appear knowledgeable and professional, not showing useless comments. This occurs in a few places, happy to flag them if we decide to do something about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
var backgroundColor: UIColor { get } | ||
var headerLabel: InputLabelStyle { get } | ||
var cancelButton: FormButtonStyle { get } | ||
var doneButton: FormButtonStyle { get set} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import UIKit | ||
|
||
public protocol BillingFormTextFieldCellStyle { | ||
var type: BillingFormCellType { get } | ||
var backgroundColor: UIColor { get } | ||
var title: InputLabelStyle? { get } | ||
var hint: InputLabelStyle? { get } | ||
var textfield: TextFieldStyle { get set } | ||
var error: ErrorInputLabelStyle { get set } | ||
} | ||
ehab-al-cko marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import UIKit | ||
|
||
struct DefaultBillingFormAddressLine1CellStyle : BillingFormTextFieldCellStyle { | ||
|
||
var type: BillingFormCellType | ||
var backgroundColor: UIColor | ||
var title: InputLabelStyle? | ||
var hint: InputLabelStyle? | ||
var textfield: TextFieldStyle | ||
var error: ErrorInputLabelStyle | ||
|
||
init(type: BillingFormCellType = .addressLine1, | ||
backgroundColor: UIColor = .white, | ||
header: InputLabelStyle = DefaultTitleLabelStyle(text: "addressLine1".localized(forClass: CheckoutTheme.self)), | ||
hint: InputLabelStyle? = nil, | ||
textfield: TextFieldStyle = DefaultTextField(), | ||
error: ErrorInputLabelStyle = DefaultErrorInputLabelStyle(text: "missingBillingFormAddressLine1".localized(forClass: CheckoutTheme.self))) { | ||
self.backgroundColor = backgroundColor | ||
self.title = header | ||
self.hint = hint | ||
self.textfield = textfield | ||
self.error = error | ||
self.type = type | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import UIKit | ||
|
||
struct DefaultBillingFormAddressLine2CellStyle : BillingFormTextFieldCellStyle { | ||
|
||
var type: BillingFormCellType | ||
var backgroundColor: UIColor | ||
var title: InputLabelStyle? | ||
var hint: InputLabelStyle? | ||
var textfield: TextFieldStyle | ||
var error: ErrorInputLabelStyle | ||
|
||
init(type: BillingFormCellType = .addressLine2, | ||
backgroundColor: UIColor = .white, | ||
header: InputLabelStyle = DefaultTitleLabelStyle(text: "addressLine2".localized(forClass: CheckoutTheme.self)), | ||
hint: InputLabelStyle? = nil, | ||
textfield: TextFieldStyle = DefaultTextField(), | ||
error: ErrorInputLabelStyle = DefaultErrorInputLabelStyle(text: "missingBillingFormAddressLine2".localized(forClass: CheckoutTheme.self))) { | ||
self.backgroundColor = backgroundColor | ||
self.title = header | ||
self.hint = hint | ||
self.textfield = textfield | ||
self.error = error | ||
self.type = type | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import UIKit | ||
|
||
struct DefaultBillingFormCityCellStyle : BillingFormTextFieldCellStyle { | ||
|
||
var type: BillingFormCellType | ||
var backgroundColor: UIColor | ||
var title: InputLabelStyle? | ||
var hint: InputLabelStyle? | ||
var textfield: TextFieldStyle | ||
var error: ErrorInputLabelStyle | ||
|
||
init(type: BillingFormCellType = .city, | ||
backgroundColor: UIColor = .white, | ||
header: InputLabelStyle = DefaultTitleLabelStyle(text: "city".localized(forClass: CheckoutTheme.self)), | ||
hint: InputLabelStyle? = nil, | ||
textfield: TextFieldStyle = DefaultTextField(), | ||
error: ErrorInputLabelStyle = DefaultErrorInputLabelStyle(text: "missingBillingFormCity".localized(forClass: CheckoutTheme.self))) { | ||
self.backgroundColor = backgroundColor | ||
self.title = header | ||
self.hint = hint | ||
self.textfield = textfield | ||
self.error = error | ||
self.type = type | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import UIKit | ||
|
||
struct DefaultBillingFormCountryCellStyle : BillingFormTextFieldCellStyle { | ||
|
||
var type: BillingFormCellType | ||
var backgroundColor: UIColor | ||
var title: InputLabelStyle? | ||
var hint: InputLabelStyle? | ||
var textfield: TextFieldStyle | ||
var error: ErrorInputLabelStyle | ||
|
||
init(type: BillingFormCellType = .country, | ||
backgroundColor: UIColor = .white, | ||
header: InputLabelStyle = DefaultTitleLabelStyle(text: "country".localized(forClass: CheckoutTheme.self)), | ||
hint: InputLabelStyle? = nil, | ||
textfield: TextFieldStyle = DefaultTextField(), | ||
error: ErrorInputLabelStyle = DefaultErrorInputLabelStyle(text: "missingBillingFormCountry".localized(forClass: CheckoutTheme.self))) { | ||
self.backgroundColor = backgroundColor | ||
self.title = header | ||
self.hint = hint | ||
self.textfield = textfield | ||
self.error = error | ||
self.type = type | ||
} | ||
|
||
} |
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 review dragged longer than it was expected, but for visibility 3.4.0 was released just few hours ago 🙈 . It has added encoding and decoding strategies, so if that is something of interest we may create some separate maintenance task to review that functionality.
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.
-> Revert to 3.3.3
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.
Should be reverted to 3.3.3
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.
Done