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-312] Pledge amount invalid input color #849

Merged
merged 14 commits into from
Sep 26, 2019
2 changes: 1 addition & 1 deletion Cartfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
### Internal

github "kickstarter/Kickstarter-Prelude" "d03f8831dcec9ad157db4f1c5f769ec17e1a3766"
github "kickstarter/Kickstarter-ReactiveExtensions" "aa994f0921dca1965ae537fe77c2edc798eef230"
github "kickstarter/Kickstarter-ReactiveExtensions" "99c3013e22fd6d4ea29d9d4c7a4a7b10bf80a99c"

### 3rd Party

Expand Down
2 changes: 1 addition & 1 deletion Cartfile.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ github "Alamofire/AlamofireImage" "c41f8b0acfbb3180fe045df73596e4332c338633"
github "ReactiveCocoa/ReactiveSwift" "6.0.0"
github "facebook/facebook-objc-sdk" "v5.0.2"
github "kickstarter/Kickstarter-Prelude" "d03f8831dcec9ad157db4f1c5f769ec17e1a3766"
github "kickstarter/Kickstarter-ReactiveExtensions" "aa994f0921dca1965ae537fe77c2edc798eef230"
github "kickstarter/Kickstarter-ReactiveExtensions" "99c3013e22fd6d4ea29d9d4c7a4a7b10bf80a99c"
Copy link
Contributor Author

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. πŸ€”

Copy link
Contributor

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.

github "stripe/stripe-ios" "v13.2.0"
github "thoughtbot/Argo" "39f06f089d25c111444e5a85eef64586e54756ac"
github "thoughtbot/Curry" "b6bf27ec9d711f607a8c7da9ca69ee9eaa201a22"
Expand Down
19 changes: 19 additions & 0 deletions Kickstarter-iOS/TestHelpers/TraitCollection.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import UIKit

extension UITraitCollection {
static let allCases: [UITraitCollection] = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
]
}

2 changes: 0 additions & 2 deletions Kickstarter-iOS/Views/AmountInputView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
24 changes: 12 additions & 12 deletions Kickstarter-iOS/Views/Controllers/PledgeAmountViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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) }()
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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)
Copy link
Contributor Author

@dusi dusi Sep 21, 2019

Choose a reason for hiding this comment

The 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 spacing on the stack view which should be much simpler and easier to reason about.


self.amountInputView.doneButton.addTarget(
self,
action: #selector(PledgeAmountViewController.doneButtonTapped(_:)),
Expand All @@ -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
Expand All @@ -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
Expand Down
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)")
}
}
}
33 changes: 33 additions & 0 deletions Kickstarter-iOS/Views/Controllers/PledgeViewControllerTests.swift
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 {
Expand All @@ -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() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)")
}
}
}
}
8 changes: 8 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
374CB94F22C17D4E00B84219 /* CharacterSet.swift in Sources */ = {isa = PBXBuildFile; fileRef = 374CB94E22C17D4E00B84219 /* CharacterSet.swift */; };
374CB95122C17D6700B84219 /* CharacterSetTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 374CB95022C17D6700B84219 /* CharacterSetTests.swift */; };
374F507922614A1000DE6746 /* PledgeFooterView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 374F507822614A1000DE6746 /* PledgeFooterView.swift */; };
3751E4292335871600047E9A /* PledgeAmountViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3751E4282335871600047E9A /* PledgeAmountViewControllerTests.swift */; };
3751E42C2335E7D400047E9A /* TraitCollection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3751E42A2335E7CE00047E9A /* TraitCollection.swift */; };
3757D0CE228E51F800241AE6 /* UIFont.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3757D0CD228E51F800241AE6 /* UIFont.swift */; };
3757D107228E521600241AE6 /* UIFontTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3757D106228E521600241AE6 /* UIFontTests.swift */; };
37637ED8224AA03D00777AD2 /* CreatePasswordViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37637ED7224AA03D00777AD2 /* CreatePasswordViewController.swift */; };
Expand Down Expand Up @@ -1445,6 +1447,8 @@
374CB94E22C17D4E00B84219 /* CharacterSet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CharacterSet.swift; sourceTree = "<group>"; };
374CB95022C17D6700B84219 /* CharacterSetTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CharacterSetTests.swift; sourceTree = "<group>"; };
374F507822614A1000DE6746 /* PledgeFooterView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgeFooterView.swift; sourceTree = "<group>"; };
3751E4282335871600047E9A /* PledgeAmountViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PledgeAmountViewControllerTests.swift; sourceTree = "<group>"; };
3751E42A2335E7CE00047E9A /* TraitCollection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TraitCollection.swift; sourceTree = "<group>"; };
3757D0CD228E51F800241AE6 /* UIFont.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIFont.swift; sourceTree = "<group>"; };
3757D106228E521600241AE6 /* UIFontTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIFontTests.swift; sourceTree = "<group>"; };
37637ED7224AA03D00777AD2 /* CreatePasswordViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreatePasswordViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2963,6 +2967,7 @@
D63BBCCF217E5460007E01F0 /* PaymentMethodsViewController.swift */,
D6F7416F21836C3D00C2DDA2 /* PaymentMethodsViewControllerTests.swift */,
370ACAFF225D337900C8745F /* PledgeAmountViewController.swift */,
3751E4282335871600047E9A /* PledgeAmountViewControllerTests.swift */,
7748438322D8DE8200508C9B /* PledgeContinueViewController.swift */,
D79A054A225E9B6B004BC6A8 /* PledgeDescriptionViewController.swift */,
8A8099FE22E21F9700373E66 /* PledgeDescriptionViewControllerTests.swift */,
Expand Down Expand Up @@ -3384,6 +3389,7 @@
A7ED20211E83237F00BFFA01 /* Combos.swift */,
37096C3122BC23AD003D1F40 /* MockAppEnvironment.swift */,
37096C2F22BC238C003D1F40 /* MockFeedbackGenerator.swift */,
3751E42A2335E7CE00047E9A /* TraitCollection.swift */,
A7ED20221E83237F00BFFA01 /* TraitController.swift */,
);
path = TestHelpers;
Expand Down Expand Up @@ -5237,6 +5243,7 @@
A7ED20581E8323E900BFFA01 /* SignupViewControllerTests.swift in Sources */,
D6B6875221923BCF005F5DA7 /* ChangeEmailViewControllerTests.swift in Sources */,
D00A376E225BDAF800F46F47 /* UIAlertControllerTests.swift in Sources */,
3751E42C2335E7D400047E9A /* TraitCollection.swift in Sources */,
3708DD45220A76FE00F8E569 /* CreatePasswordTableControllerTests.swift in Sources */,
D72370942119139D001EA4CA /* SettingsPrivacyViewControllerTests.swift in Sources */,
A7ED20651E83256700BFFA01 /* UpdatePreviewViewModelTests.swift in Sources */,
Expand All @@ -5258,6 +5265,7 @@
D033E2C322A05B7400464E43 /* MockApplication.swift in Sources */,
8A8099FF22E21F9700373E66 /* PledgeDescriptionViewControllerTests.swift in Sources */,
A7ED20541E8323E900BFFA01 /* ProjectPamphletContentViewControllerTests.swift in Sources */,
3751E4292335871600047E9A /* PledgeAmountViewControllerTests.swift in Sources */,
D6560C2A2182361800CD24BC /* PaymentMethodsDataSourceTests.swift in Sources */,
A7ED201E1E83231C00BFFA01 /* MockBundle.swift in Sources */,
A7ED20551E8323E900BFFA01 /* LoginToutViewControllerTests.swift in Sources */,
Expand Down
Loading