-
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-312] Pledge amount invalid input color #849
Changes from 8 commits
5f5363d
35e92d6
ca45fd5
c083ccc
4b0b48b
f656dd0
b0df1b3
5a0f585
509dfeb
0538c2b
8364de9
116fb02
7d5e9b2
841e916
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 |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import UIKit | ||
|
||
extension UITraitCollection { | ||
static let allCases: [UITraitCollection] = [ | ||
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. Just a test helper for allowing us testing large text better. |
||
UITraitCollection(preferredContentSizeCategory: .extraSmall), | ||
UITraitCollection(preferredContentSizeCategory: .small), | ||
UITraitCollection(preferredContentSizeCategory: .medium), | ||
UITraitCollection(preferredContentSizeCategory: .large), | ||
UITraitCollection(preferredContentSizeCategory: .extraLarge), | ||
UITraitCollection(preferredContentSizeCategory: .extraExtraLarge), | ||
UITraitCollection(preferredContentSizeCategory: .extraExtraExtraLarge), | ||
UITraitCollection(preferredContentSizeCategory: .accessibilityMedium), | ||
UITraitCollection(preferredContentSizeCategory: .accessibilityLarge), | ||
UITraitCollection(preferredContentSizeCategory: .accessibilityExtraLarge), | ||
UITraitCollection(preferredContentSizeCategory: .accessibilityExtraExtraLarge), | ||
UITraitCollection(preferredContentSizeCategory: .accessibilityExtraExtraExtraLarge) | ||
] | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,15 +112,13 @@ private let labelStyle: LabelStyle = { (label: UILabel) in | |
|> \.font .~ UIFont.ksr_body() | ||
|> \.adjustsFontForContentSizeCategory .~ true | ||
|> \.textAlignment .~ NSTextAlignment.right | ||
|> \.textColor .~ UIColor.ksr_green_500 | ||
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. These are now set using VM's outputs |
||
} | ||
|
||
private let textFieldStyle: TextFieldStyle = { (textField: UITextField) in | ||
textField | ||
|> \.adjustsFontForContentSizeCategory .~ true | ||
|> \.font .~ UIFont.ksr_title1() | ||
|> \.keyboardType .~ UIKeyboardType.decimalPad | ||
|> \.textColor .~ UIColor.ksr_green_500 | ||
} | ||
|
||
private let stackViewStyle: StackViewStyle = { (stackView: UIStackView) in | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,11 +21,7 @@ final class PledgeAmountViewController: UIViewController { | |
private lazy var amountInputView: AmountInputView = { AmountInputView(frame: .zero) }() | ||
private lazy var titleLabel: UILabel = { UILabel(frame: .zero) }() | ||
private lazy var rootStackView: UIStackView = { UIStackView(frame: .zero) }() | ||
private lazy var spacer: UIView = { | ||
UIView(frame: .zero) | ||
|> \.translatesAutoresizingMaskIntoConstraints .~ false | ||
}() | ||
|
||
private lazy var horizontalSpacer: UIView = { UIView(frame: .zero) }() | ||
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. No real need for auto layout anymore...we're using spacing (where we have previously used auto layout)...this should be simpler and more readable. |
||
private lazy var stepper: UIStepper = { UIStepper(frame: .zero) }() | ||
|
||
// MARK: - Lifecycle | ||
|
@@ -40,16 +36,14 @@ final class PledgeAmountViewController: UIViewController { | |
|> ksr_addSubviewToParent() | ||
|> ksr_constrainViewToEdgesInParent() | ||
|
||
_ = ([self.titleLabel, self.adaptableStackView], self.rootStackView) | ||
_ = ([self.titleLabel, self.adaptableStackView, UIView(frame: .zero)], self.rootStackView) | ||
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 makes sure that the vertical stack view will always align its subviews vertically to the top |
||
|> ksr_addArrangedSubviewsToStackView() | ||
|
||
_ = ([self.stepper, self.spacer, self.amountInputView], self.adaptableStackView) | ||
_ = ([self.stepper, self.horizontalSpacer, self.amountInputView], self.adaptableStackView) | ||
|> ksr_addArrangedSubviewsToStackView() | ||
|
||
self.amountInputView.textField.delegate = self | ||
|
||
self.spacer.widthAnchor.constraint(greaterThanOrEqualToConstant: Styles.grid(3)).isActive = 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. We have previously used this width as the minimum space between the stepper and the input field (otherwise when the text field grows it would almost touch the stepper)...now this is done using |
||
|
||
self.amountInputView.doneButton.addTarget( | ||
self, | ||
action: #selector(PledgeAmountViewController.doneButtonTapped(_:)), | ||
|
@@ -74,13 +68,17 @@ final class PledgeAmountViewController: UIViewController { | |
override func bindStyles() { | ||
super.bindStyles() | ||
|
||
let isAccessibilityCategory = self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory | ||
|
||
_ = self.view | ||
|> checkoutBackgroundStyle | ||
|
||
_ = self.adaptableStackView | ||
|> checkoutAdaptableStackViewStyle( | ||
self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory | ||
) | ||
|> checkoutAdaptableStackViewStyle(isAccessibilityCategory) | ||
|> \.spacing .~ Styles.grid(3) | ||
|
||
_ = self.horizontalSpacer | ||
|> \.isHidden .~ isAccessibilityCategory | ||
|
||
_ = self.titleLabel | ||
|> checkoutBackgroundStyle | ||
|
@@ -102,8 +100,10 @@ final class PledgeAmountViewController: UIViewController { | |
|
||
self.amountInputView.doneButton.rac.enabled = self.viewModel.outputs.doneButtonIsEnabled | ||
self.amountInputView.label.rac.text = self.viewModel.outputs.currency | ||
self.amountInputView.label.rac.textColor = self.viewModel.outputs.labelTextColor | ||
self.amountInputView.textField.rac.isFirstResponder = self.viewModel.outputs.textFieldIsFirstResponder | ||
self.amountInputView.textField.rac.text = self.viewModel.outputs.textFieldValue | ||
self.amountInputView.textField.rac.textColor = self.viewModel.outputs.textFieldTextColor | ||
self.stepper.rac.maximumValue = self.viewModel.outputs.stepperMaxValue | ||
self.stepper.rac.minimumValue = self.viewModel.outputs.stepperMinValue | ||
self.stepper.rac.stepValue = self.viewModel.outputs.stepperStepValue | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
@testable import Kickstarter_Framework | ||
@testable import Library | ||
@testable import KsApi | ||
import Prelude | ||
import UIKit | ||
|
||
final class PledgeAmountViewControllerTests: TestCase { | ||
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. I felt like it was a perfect opportunity to add testing...not only can we test large text but also invalid input values, currency symbol, stepper increment/decrement button etc. |
||
override func setUp() { | ||
super.setUp() | ||
|
||
AppEnvironment.pushEnvironment(mainBundle: Bundle.framework) | ||
UIView.setAnimationsEnabled(false) | ||
} | ||
|
||
override func tearDown() { | ||
AppEnvironment.popEnvironment() | ||
UIView.setAnimationsEnabled(true) | ||
|
||
super.tearDown() | ||
} | ||
|
||
func testView() { | ||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: .template, reward: .template)) | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
|
||
func testView_LargerText() { | ||
UITraitCollection.allCases.forEach { additionalTraits in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(child: controller, additionalTraits: additionalTraits) | ||
parent.view.frame.size.height = 300 | ||
|
||
controller.configureWith(value: (project: .template, reward: .template)) | ||
|
||
FBSnapshotVerifyView( | ||
parent.view, identifier: "trait_\(additionalTraits.preferredContentSizeCategory.rawValue)" | ||
) | ||
} | ||
} | ||
|
||
func testView_ShowsCurrencySymbol() { | ||
let project = Project.template | ||
|> Project.lens.country .~ Project.Country.ca | ||
|
||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: project, reward: .template)) | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
|
||
func testView_StepperDecrementButtonDisabled_WhenStepperValueSetToMinimum() { | ||
let reward = Reward.template | ||
|> Reward.lens.minimum .~ 0 | ||
|
||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: .template, reward: reward)) | ||
controller.stepperValueChanged(UIStepper(frame: .zero) |> \.value .~ 0) | ||
controller.textFieldDidChange(UITextField(frame: .zero) |> \.text .~ "0") | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
|
||
func testView_StepperIncrementButtonDisabled_WhenStepperValueSetToMaximum() { | ||
let stepper = UIStepper(frame: .zero) | ||
|> \.maximumValue .~ Double.greatestFiniteMagnitude | ||
|> \.value .~ Double.greatestFiniteMagnitude | ||
|
||
let textField = UITextField(frame: .zero) | ||
|> \.text .~ String(format: "%.0f", Double.greatestFiniteMagnitude) | ||
|
||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: .template, reward: .template)) | ||
controller.stepperValueChanged(stepper) | ||
controller.textFieldDidChange(textField) | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
|
||
func testView_TextColorIsRedWhenBellowMinimum() { | ||
let reward = Reward.template | ||
|> Reward.lens.minimum .~ 10 | ||
|
||
let stepper = UIStepper(frame: .zero) | ||
|> \.value .~ 5 | ||
|
||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: .template, reward: reward)) | ||
controller.stepperValueChanged(stepper) | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
|
||
func testView_TextColorIsRedWhenAboveMaximum() { | ||
let project = Project.template | ||
|> (Project.lens.country .. Project.Country.lens.maxPledge) .~ 10_000 | ||
|
||
let stepper = UIStepper(frame: .zero) | ||
|> \.maximumValue .~ Double.greatestFiniteMagnitude | ||
|> \.value .~ Double.greatestFiniteMagnitude | ||
|
||
[Device.phone4_7inch, Device.pad].forEach { device in | ||
let controller = PledgeAmountViewController.instantiate() | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
parent.view.frame.size.height = 100 | ||
|
||
controller.configureWith(value: (project: project, reward: .template)) | ||
controller.stepperValueChanged(stepper) | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "device_\(device)") | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
@testable import Kickstarter_Framework | ||
@testable import Library | ||
@testable import KsApi | ||
import Prelude | ||
import UIKit | ||
|
||
final class PledgeViewControllerTests: TestCase { | ||
|
@@ -16,4 +18,35 @@ final class PledgeViewControllerTests: TestCase { | |
|
||
super.tearDown() | ||
} | ||
|
||
func testView() { | ||
combos(Language.allLanguages, [Device.phone4_7inch, Device.pad]).forEach { language, device in | ||
withEnvironment(language: language) { | ||
let controller = PledgeViewController.instantiate() | ||
controller.configureWith(project: .template, reward: .template, refTag: nil) | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
|
||
self.scheduler.run() | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "lang_\(language)_device_\(device)") | ||
} | ||
} | ||
} | ||
|
||
func testView_WithShippingLocation() { | ||
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: Rename to something more meaningful |
||
let reward = Reward.template | ||
|> (Reward.lens.shipping .. Reward.Shipping.lens.enabled) .~ true | ||
|
||
combos(Language.allLanguages, [Device.phone4_7inch, Device.pad]).forEach { language, device in | ||
withEnvironment(language: language) { | ||
let controller = PledgeViewController.instantiate() | ||
controller.configureWith(project: .template, reward: reward, refTag: nil) | ||
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller) | ||
|
||
self.scheduler.run() | ||
|
||
FBSnapshotVerifyView(parent.view, identifier: "lang_\(language)_device_\(device)") | ||
} | ||
} | ||
} | ||
} |
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.
It's weird that it builds on Circle and not on my machine. I wonder what's the real problem here. π€
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.
The hash in this file should match the one in the
Cartfile
.