Skip to content

Commit

Permalink
[NT-327] Disable pledge button for invalid inputs (#859)
Browse files Browse the repository at this point in the history 
* Round and format amount input based on some rules

* Rerecord snapshots for the new maximum value of stepper

* Add snapshot tests for the minimum value

* Format code

* Notify delegate when the initial credit card is selected in order for the pledge button to properly reflect enabled/disabled state

* Update pledge button's enabled/disabled state based on the input amount and its validity

* Remove unused function after missing this during merge conflict resolution step

* Format code

* Rename input signal and delegate function

* Fix failing test
  • Loading branch information
@dusi @justinswart
dusi authored and justinswart committed Sep 30, 2019
1 parent c7f302a commit 32de6d1
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 60 deletions.
8 changes: 4 additions & 4 deletions Kickstarter-iOS/Views/Controllers/PledgeAmountViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import UIKit
protocol PledgeAmountViewControllerDelegate: AnyObject {
func pledgeAmountViewController(
_ viewController: PledgeAmountViewController,
didUpdateAmount amount: Double
didUpdateWith data: PledgeAmountData
)
}

Expand Down Expand Up @@ -117,12 +117,12 @@ final class PledgeAmountViewController: UIViewController {
.observeForUI()
.observeValues { generateNotificationWarningFeedback() }

self.viewModel.outputs.amountPrimitive
self.viewModel.outputs.amount
.observeForUI()
.observeValues { [weak self] amount in
.observeValues { [weak self] data in
guard let self = self else { return }

self.delegate?.pledgeAmountViewController(self, didUpdateAmount: amount)
self.delegate?.pledgeAmountViewController(self, didUpdateWith: data)
}
}

Expand Down
4 changes: 2 additions & 2 deletions Kickstarter-iOS/Views/Controllers/PledgeViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,9 @@ extension PledgeViewController: PKPaymentAuthorizationViewControllerDelegate {
extension PledgeViewController: PledgeAmountViewControllerDelegate {
func pledgeAmountViewController(
_: PledgeAmountViewController,
didUpdateAmount amount: Double
didUpdateWith data: PledgeAmountData
) {
self.viewModel.inputs.pledgeAmountDidUpdate(to: amount)
self.viewModel.inputs.pledgeAmountViewControllerDidUpdate(with: data)
}
}

Expand Down
20 changes: 9 additions & 11 deletions Library/ViewModels/PledgeAmountViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public protocol PledgeAmountViewModelInputs {
}

public protocol PledgeAmountViewModelOutputs {
var amountPrimitive: Signal<Double, Never> { get }
var amount: Signal<(Double, Bool), Never> { get }
var currency: Signal<String, Never> { get }
var doneButtonIsEnabled: Signal<Bool, Never> { get }
var generateSelectionFeedback: Signal<Void, Never> { get }
Expand Down Expand Up @@ -122,15 +122,13 @@ public final class PledgeAmountViewModel: PledgeAmountViewModelType,
)
)

self.amountPrimitive = updatedValue
.map(third)
.map(rounded)
.skipRepeats()

let isValueValid = updatedValue
.map { (min: Double, max: Double, doubleValue: Double) -> Bool in
min <= doubleValue && doubleValue <= max
self.amount = updatedValue
.map { min, max, value in
(rounded(value), min <= value && value <= max)
}

let isValueValid = self.amount
.map(second)
.skipRepeats()

self.doneButtonIsEnabled = isValueValid
Expand All @@ -144,7 +142,7 @@ public final class PledgeAmountViewModel: PledgeAmountViewModelType,

self.stepperValue = Signal.merge(
minValue,
self.amountPrimitive
self.amount.map(first)
)
.skipRepeats()

Expand Down Expand Up @@ -180,7 +178,7 @@ public final class PledgeAmountViewModel: PledgeAmountViewModelType,
self.textFieldValueProperty.value = value
}

public let amountPrimitive: Signal<Double, Never>
public let amount: Signal<(Double, Bool), Never>
public let currency: Signal<String, Never>
public let doneButtonIsEnabled: Signal<Bool, Never>
public let generateSelectionFeedback: Signal<Void, Never>
Expand Down
99 changes: 68 additions & 31 deletions Library/ViewModels/PledgeAmountViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import XCTest
internal final class PledgeAmountViewModelTests: TestCase {
private let vm: PledgeAmountViewModelType = PledgeAmountViewModel()

private let amountPrimitive = TestObserver<Double, Never>()
private let amountIsValid = TestObserver<Bool, Never>()
private let amountValue = TestObserver<Double, Never>()
private let currency = TestObserver<String, Never>()
private let doneButtonIsEnabled = TestObserver<Bool, Never>()
private let generateSelectionFeedback = TestObserver<Void, Never>()
Expand All @@ -24,7 +25,8 @@ internal final class PledgeAmountViewModelTests: TestCase {
override func setUp() {
super.setUp()

self.vm.outputs.amountPrimitive.observe(self.amountPrimitive.observer)
self.vm.outputs.amount.map(second).observe(self.amountIsValid.observer)
self.vm.outputs.amount.map(first).observe(self.amountValue.observer)
self.vm.outputs.currency.observe(self.currency.observer)
self.vm.outputs.doneButtonIsEnabled.observe(self.doneButtonIsEnabled.observer)
self.vm.outputs.generateSelectionFeedback.observe(self.generateSelectionFeedback.observer)
Expand All @@ -44,7 +46,8 @@ internal final class PledgeAmountViewModelTests: TestCase {
func testAmountCurrencyAndStepper_NoReward() {
self.vm.inputs.configureWith(project: .template, reward: Reward.noReward)

self.amountPrimitive.assertValues([1])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([1])
self.currency.assertValues(["$"])
self.stepperMinValue.assertValue(PledgeAmountStepperConstants.min)
self.stepperMaxValue.assertValue(PledgeAmountStepperConstants.max)
Expand All @@ -59,7 +62,8 @@ internal final class PledgeAmountViewModelTests: TestCase {

self.vm.inputs.configureWith(project: project, reward: Reward.noReward)

self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([10])
self.currency.assertValues(["MX$"])
self.stepperMinValue.assertValue(PledgeAmountStepperConstants.min)
self.stepperMaxValue.assertValue(PledgeAmountStepperConstants.max)
Expand All @@ -77,7 +81,8 @@ internal final class PledgeAmountViewModelTests: TestCase {

self.vm.inputs.configureWith(project: project, reward: Reward.noReward)

self.amountPrimitive.assertValues([1])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([1])
self.currency.assertValues(["$"])
self.stepperMinValue.assertValue(PledgeAmountStepperConstants.min)
self.stepperMaxValue.assertValue(PledgeAmountStepperConstants.max)
Expand All @@ -89,7 +94,8 @@ internal final class PledgeAmountViewModelTests: TestCase {
func testAmountCurrencyAndStepper_Reward_Minimum_Template() {
self.vm.inputs.configureWith(project: .template, reward: .template)

self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([10])
self.currency.assertValues(["$"])
self.stepperMinValue.assertValue(PledgeAmountStepperConstants.min)
self.stepperMaxValue.assertValue(PledgeAmountStepperConstants.max)
Expand All @@ -107,7 +113,8 @@ internal final class PledgeAmountViewModelTests: TestCase {

self.vm.inputs.configureWith(project: project, reward: reward)

self.amountPrimitive.assertValues([200])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([200])
self.currency.assertValues(["¥"])
self.stepperMinValue.assertValue(PledgeAmountStepperConstants.min)
self.stepperMaxValue.assertValue(PledgeAmountStepperConstants.max)
Expand Down Expand Up @@ -742,51 +749,63 @@ internal final class PledgeAmountViewModelTests: TestCase {
func testNilInputReturnsZero() {
self.vm.inputs.configureWith(project: .template, reward: .template)

self.amountPrimitive.assertValue(10)
self.amountIsValid.assertValues([true])
self.amountValue.assertValue(10)

self.vm.inputs.textFieldValueChanged("11")
self.amountPrimitive.assertValues([10, 11])
self.amountIsValid.assertValues([true, true])
self.amountValue.assertValues([10, 11])

self.vm.inputs.textFieldValueChanged("")
self.amountPrimitive.assertValues([10, 11, 0])
self.amountIsValid.assertValues([true, true, false])
self.amountValue.assertValues([10, 11, 0])

self.vm.inputs.textFieldValueChanged("5")
self.amountPrimitive.assertValues([10, 11, 0, 5])
self.amountIsValid.assertValues([true, true, false, false])
self.amountValue.assertValues([10, 11, 0, 5])

self.vm.inputs.textFieldValueChanged(nil)
self.amountPrimitive.assertValues([10, 11, 0, 5, 0])
self.amountIsValid.assertValues([true, true, false, false, false])
self.amountValue.assertValues([10, 11, 0, 5, 0])
}

func testTextFieldDidEndEditing() {
let maxValue = PledgeAmountStepperConstants.max
let maxValueFormatted = String(format: "%.0f", maxValue)

self.vm.inputs.configureWith(project: .template, reward: .template)
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([10])
self.textFieldValue.assertValues(["10"])

self.vm.inputs.textFieldDidEndEditing(nil)
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true])
self.amountValue.assertValues([10])
self.textFieldValue.assertValues(["10"])

self.vm.inputs.textFieldDidEndEditing("16")
self.amountPrimitive.assertValues([10, 16])
self.amountIsValid.assertValues([true, true])
self.amountValue.assertValues([10, 16])
self.textFieldValue.assertValues(["10", "16"])

self.vm.inputs.textFieldDidEndEditing(String(maxValue))
self.amountPrimitive.assertValues([10, 16, maxValue])
self.amountIsValid.assertValues([true, true, false])
self.amountValue.assertValues([10, 16, maxValue])
self.textFieldValue.assertValues(["10", "16", maxValueFormatted])

self.vm.inputs.textFieldDidEndEditing("0")
self.amountPrimitive.assertValues([10, 16, maxValue, 0])
self.amountIsValid.assertValues([true, true, false, false])
self.amountValue.assertValues([10, 16, maxValue, 0])
self.textFieldValue.assertValues(["10", "16", maxValueFormatted, "0"])

self.vm.inputs.textFieldDidEndEditing("17")
self.amountPrimitive.assertValues([10, 16, maxValue, 0, 17])
self.amountIsValid.assertValues([true, true, false, false, true])
self.amountValue.assertValues([10, 16, maxValue, 0, 17])
self.textFieldValue.assertValues(["10", "16", maxValueFormatted, "0", "17"])

self.vm.inputs.textFieldDidEndEditing("")
self.amountPrimitive.assertValues([10, 16, maxValue, 0, 17])
self.amountIsValid.assertValues([true, true, false, false, true])
self.amountValue.assertValues([10, 16, maxValue, 0, 17])
self.textFieldValue.assertValues(["10", "16", maxValueFormatted, "0", "17"])
}

Expand Down Expand Up @@ -814,91 +833,109 @@ internal final class PledgeAmountViewModelTests: TestCase {
self.textFieldTextColor.assertValues([green, red, green, red, green])
}

// swiftlint:disable line_length
func testTextFieldValueChangedRounding() {
let green = UIColor.ksr_green_500
let red = UIColor.ksr_red_400

self.vm.inputs.configureWith(project: .template, reward: .template)

self.amountIsValid.assertValues([true])
self.amountValue.assertValues([10])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10")
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true, true])
self.amountValue.assertValues([10, 10])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.")
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true, true, true])
self.amountValue.assertValues([10, 10, 10])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.0")
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.00")
self.amountPrimitive.assertValues([10])
self.amountIsValid.assertValues([true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.01")
self.amountPrimitive.assertValues([10, 10.01])
self.amountIsValid.assertValues([true, true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10, 10.01])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.010")
self.amountPrimitive.assertValues([10, 10.01])
self.amountIsValid.assertValues([true, true, true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10, 10.01])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.0100")
self.amountPrimitive.assertValues([10, 10.01])
self.amountIsValid.assertValues([true, true, true, true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01, 10.01])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10, 10.01])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.019")
self.amountPrimitive.assertValues([10, 10.01, 10.02])
self.amountIsValid.assertValues([true, true, true, true, true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01, 10.01, 10.02])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10, 10.01, 10.02])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("10.0194444444")
self.amountPrimitive.assertValues([10, 10.01, 10.02])
self.amountIsValid.assertValues([true, true, true, true, true, true, true, true, true, true])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01, 10.01, 10.02, 10.02])
self.doneButtonIsEnabled.assertValues([true])
self.labelTextColor.assertValues([green])
self.stepperValue.assertValues([10, 10.01, 10.02])
self.textFieldTextColor.assertValues([green])

self.vm.inputs.textFieldValueChanged("9.999")
self.amountPrimitive.assertValues([10, 10.01, 10.02, 10])
self.amountIsValid.assertValues([true, true, true, true, true, true, true, true, true, true, false])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01, 10.01, 10.02, 10.02, 10])
self.doneButtonIsEnabled.assertValues([true, false])
self.labelTextColor.assertValues([green, red])
self.stepperValue.assertValues([10, 10.01, 10.02, 10])
self.textFieldTextColor.assertValues([green, red])

self.vm.inputs.textFieldValueChanged("9.99")
self.amountPrimitive.assertValues([10, 10.01, 10.02, 10, 9.99])
self.amountIsValid.assertValues([true, true, true, true, true, true, true, true, true, true, false, false])
self.amountValue.assertValues([10, 10, 10, 10, 10, 10.01, 10.01, 10.01, 10.02, 10.02, 10, 9.99])
self.doneButtonIsEnabled.assertValues([true, false])
self.labelTextColor.assertValues([green, red])
self.stepperValue.assertValues([10, 10.01, 10.02, 10, 9.99])
self.textFieldTextColor.assertValues([green, red])
}

// swiftlint:disable line_length
func testTextFieldDidEndEditingRoundingAndTruncation() {
self.vm.inputs.configureWith(project: .template, reward: .template)

Expand Down
1 change: 0 additions & 1 deletion Library/ViewModels/PledgeCreditCardViewModelTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ internal final class PledgeCreditCardViewModelTests: TestCase {
self.notifyDelegateOfCardSelected.assertDidNotEmitValue()

self.vm.inputs.selectButtonTapped()

self.notifyDelegateOfCardSelected.assertValues(["123"])
}
}
Loading

0 comments on commit 32de6d1

Please sign in to comment.