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-280] Manage pledge screen from CTA container #831

Merged
merged 13 commits into from
Sep 12, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ internal final class DeprecatedRewardPledgeViewController: UIViewController {
)
)

let closeButton = UIBarButtonItem(
image: image(named: "icon--cross", tintColor: .ksr_navy_600),
style: .plain,
target: self,
action: #selector(DeprecatedRewardPledgeViewController.closeButtonTapped)
)

_ = self.navigationItem
|> \.leftBarButtonItem .~ closeButton

self.disclaimerTextView.delegate = self

self.applePayButtonContainerView.addArrangedSubview(self.applePayButton)
Expand Down Expand Up @@ -632,7 +642,7 @@ internal final class DeprecatedRewardPledgeViewController: UIViewController {
self.viewModel.inputs.expandDescriptionTapped()
}

@IBAction internal func closeButtonTapped() {
@objc fileprivate func closeButtonTapped() {
self.viewModel.inputs.closeButtonTapped()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ public final class ProjectPamphletContentViewController: UITableViewController {
applePayCapable: applePayCapable
)

vc.navigationItem.leftBarButtonItem = UIBarButtonItem(
image: image(named: "icon--cross", tintColor: .ksr_navy_600),
style: .plain,
target: vc,
action: #selector(DeprecatedRewardPledgeViewController.closeButtonTapped)
)

let nav = UINavigationController(rootViewController: vc)
if AppEnvironment.current.device.userInterfaceIdiom == .pad {
_ = nav
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,26 @@ public final class ProjectPamphletViewController: UIViewController {
self?.goToRewards(project: project, refTag: refTag)
}

self.viewModel.outputs.goToManageViewPledge
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing a lot of inconsistencies between the naming of VMs output signal and the private action on the controller...do you think it would be worth it cleaning this up..

So for example:

self.viewModel.outputs.goToManageViewPledge would trigger self?.goToManageViewPledge, etc...

For the deprecated ones I'd suggest we simply add it as a suffix:

self.viewModel.outputs.goToViewBackingDeprecated links to self?.goToViewBacking...

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 makes sense to change the private actions to match the name of the output. Will change that.

Regarding the deprecated naming, I'm following the naming convention that we adopted for deprecated classes (DeprecatedCheckoutViewModel, DeprecatedRewardPledgeViewController, DeprecatedRewardCell etc)

.observeForControllerAction()
.observeValues { [weak self] params in
let (project, reward, refTag) = params

self?.goToManageViewPledge(project: project, reward: reward, refTag: refTag)
}

self.viewModel.outputs.goToDeprecatedViewBacking
.observeForControllerAction()
.observeValues { [weak self] project, user in
self?.goToDeprecatedViewBacking(project: project, user: user)
}

self.viewModel.outputs.goToDeprecatedManagePledge
.observeForControllerAction()
.observeValues { [weak self] project, reward, refTag in
self?.goToDeprecatedManagePledge(project: project, reward: reward, refTag: refTag)
}

self.viewModel.outputs.configureChildViewControllersWithProject
.observeForUI()
.observeValues { [weak self] project, refTag in
Expand Down Expand Up @@ -186,6 +206,44 @@ public final class ProjectPamphletViewController: UIViewController {
self.present(vc, animated: true)
}

private func goToManageViewPledge(project: Project, reward: Reward, refTag _: RefTag?) {
let managePledgeViewController = ManagePledgeViewController.instantiate()
managePledgeViewController.configureWith(project: project, reward: reward)

let nav = UINavigationController(rootViewController: managePledgeViewController)
if AppEnvironment.current.device.userInterfaceIdiom == .pad {
_ = nav
|> \.modalPresentationStyle .~ .formSheet
}
self.present(nav, animated: true)
}

private func goToDeprecatedManagePledge(project: Project, reward: Reward, refTag _: RefTag?) {
let pledgeViewController = DeprecatedRewardPledgeViewController
.configuredWith(
project: project, reward: reward
)

let nav = UINavigationController(rootViewController: pledgeViewController)
if AppEnvironment.current.device.userInterfaceIdiom == .pad {
_ = nav
|> \.modalPresentationStyle .~ .formSheet
}
self.present(nav, animated: true)
}

private func goToDeprecatedViewBacking(project: Project, user _: User?) {
let backingViewController = BackingViewController.configuredWith(project: project, backer: nil)

if AppEnvironment.current.device.userInterfaceIdiom == .pad {
let nav = UINavigationController(rootViewController: backingViewController)
|> \.modalPresentationStyle .~ .formSheet
self.present(nav, animated: true)
} else {
self.navigationController?.pushViewController(backingViewController, animated: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

So, I know we've previously discussed the fact that we're presenting manage screen over the previous context using .formSheet...but do we really want to push on non-iPad devices? Just trying to understand the decision behind this?

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 from BackingViewController, we can access Messages (the vc's navigation controller pushes the MessagesViewController). If we present the BackingViewController modally instead, we are no longer able to see the messages, since it requires a navigation controller to work

}
}

private func updateContentInsets() {
let buttonSize = self.pledgeCTAContainerView.pledgeCTAButton.systemLayoutSizeFitting(
UIView.layoutFittingCompressedSize
Expand All @@ -203,8 +261,8 @@ public final class ProjectPamphletViewController: UIViewController {
}

extension ProjectPamphletViewController: PledgeCTAContainerViewDelegate {
func pledgeCTAButtonTapped() {
self.viewModel.inputs.backThisProjectTapped()
func pledgeCTAButtonTapped(with state: PledgeStateCTAType) {
self.viewModel.inputs.pledgeCTAButtonTapped(with: state)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,6 @@ final class RewardsCollectionViewController: UICollectionViewController {
self?.goToDeprecatedPledge(project: project, reward: reward, refTag: refTag)
}

self.viewModel.outputs.goToManagePledge
.observeForControllerAction()
.observeValues { [weak self] project, reward, refTag in
self?.goToManagePledge(project: project, reward: reward, refTag: refTag)
}

self.viewModel.outputs.goToViewBacking
.observeForControllerAction()
.observeValues { [weak self] project, user in
self?.goToViewBacking(project: project, user: user)
}

self.viewModel.outputs.rewardsCollectionViewFooterIsHidden
.observeForUI()
.observeValues { [weak self] isHidden in
Expand Down Expand Up @@ -240,31 +228,13 @@ final class RewardsCollectionViewController: UICollectionViewController {
?|> \.isActive .~ !isHidden
}

private func goToManagePledge(project: Project, reward: Reward, refTag _: RefTag?) {
let managePledgeViewController = ManagePledgeViewController.instantiate()
managePledgeViewController.configureWith(project: project, reward: reward)

let nav = UINavigationController(rootViewController: managePledgeViewController)
if AppEnvironment.current.device.userInterfaceIdiom == .pad {
_ = nav
|> \.modalPresentationStyle .~ .formSheet
}
self.present(nav, animated: true)
}

private func goToPledge(project: Project, reward: Reward, refTag _: RefTag?) {
let pledgeViewController = PledgeViewController.instantiate()
pledgeViewController.configureWith(project: project, reward: reward)

self.navigationController?.pushViewController(pledgeViewController, animated: true)
}

private func goToViewBacking(project: Project, user: User?) {
let backingViewController = BackingViewController.configuredWith(project: project, backer: user)

self.navigationController?.pushViewController(backingViewController, animated: true)
}

private func goToDeprecatedPledge(project: Project, reward: Reward, refTag _: RefTag?) {
let pledgeViewController = DeprecatedRewardPledgeViewController
.configuredWith(
Expand Down
6 changes: 3 additions & 3 deletions Kickstarter-iOS/Views/PledgeCTAContainerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Prelude
import UIKit

protocol PledgeCTAContainerViewDelegate: AnyObject {
func pledgeCTAButtonTapped()
func pledgeCTAButtonTapped(with state: PledgeStateCTAType)
}

private enum Layout {
Expand Down Expand Up @@ -113,8 +113,8 @@ final class PledgeCTAContainerView: UIView {

self.viewModel.outputs.notifyDelegateCTATapped
.observeForUI()
.observeValues { [weak self] in
self?.delegate?.pledgeCTAButtonTapped()
.observeValues { [weak self] state in
self?.delegate?.pledgeCTAButtonTapped(with: state)
}

self.viewModel.outputs.buttonStyleType
Expand Down
2 changes: 1 addition & 1 deletion Library/ViewModels/BackingViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public final class BackingViewModel: BackingViewModelType, BackingViewModelInput
self.messageCreatorTappedProperty.value = ()
}

fileprivate let projectAndBackerProperty = MutableProperty<(Project, User?)?>(nil)
fileprivate let projectAndBackerProperty = MutableProperty<BackingData?>(nil)
public func configureWith(project: Project, backer: User?) {
self.projectAndBackerProperty.value = (project, backer)
}
Expand Down
7 changes: 4 additions & 3 deletions Library/ViewModels/PledgeCTAContainerViewViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public protocol PledgeCTAContainerViewViewModelOutputs {
var activityIndicatorIsHidden: Signal<Bool, Never> { get }
var buttonStyleType: Signal<ButtonStyleType, Never> { get }
var buttonTitleText: Signal<String, Never> { get }
var notifyDelegateCTATapped: Signal<(), Never> { get }
var notifyDelegateCTATapped: Signal<PledgeStateCTAType, Never> { get }
var pledgeCTAButtonIsHidden: Signal<Bool, Never> { get }
var pledgeRetryButtonIsHidden: Signal<Bool, Never> { get }
var spacerIsHidden: Signal<Bool, Never> { get }
Expand Down Expand Up @@ -63,7 +63,8 @@ public final class PledgeCTAContainerViewViewModel: PledgeCTAContainerViewViewMo
isLoading.filter(isFalse).ignoreValues()
)

self.notifyDelegateCTATapped = self.pledgeCTAButtonTappedProperty.signal
self.notifyDelegateCTATapped = pledgeState
.takeWhen(self.pledgeCTAButtonTappedProperty.signal)

self.pledgeRetryButtonIsHidden = inError
.map(isFalse)
Expand Down Expand Up @@ -117,7 +118,7 @@ public final class PledgeCTAContainerViewViewModel: PledgeCTAContainerViewViewMo
public let activityIndicatorIsHidden: Signal<Bool, Never>
public let buttonStyleType: Signal<ButtonStyleType, Never>
public let buttonTitleText: Signal<String, Never>
public let notifyDelegateCTATapped: Signal<Void, Never>
public let notifyDelegateCTATapped: Signal<PledgeStateCTAType, Never>
public let pledgeCTAButtonIsHidden: Signal<Bool, Never>
public let pledgeRetryButtonIsHidden: Signal<Bool, Never>
public let spacerIsHidden: Signal<Bool, Never>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal final class PledgeCTAContainerViewViewModelTests: TestCase {
let activityIndicatorIsHidden = TestObserver<Bool, Never>()
let buttonStyleType = TestObserver<ButtonStyleType, Never>()
let buttonTitleText = TestObserver<String, Never>()
let notifyDelegateCTATapped = TestObserver<Void, Never>()
let notifyDelegateCTATapped = TestObserver<PledgeStateCTAType, Never>()
let pledgeCTAButtonIsHidden = TestObserver<Bool, Never>()
let pledgeRetryButtonIsHidden = TestObserver<Bool, Never>()
let spacerIsHidden = TestObserver<Bool, Never>()
Expand Down
91 changes: 81 additions & 10 deletions Library/ViewModels/ProjectPamphletViewModel.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import KsApi
import Prelude
import ReactiveSwift
public protocol ProjectPamphletViewModelInputs {
/// Call when "Back this project" is tapped
func backThisProjectTapped()

public typealias BackingData = (Project, User?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like

// BackingViewModel.swift
fileprivate let projectAndBackerProperty = MutableProperty<(Project, User?)?>(nil)

could use this data type...should we update it?

// BackingViewModel.swift
fileprivate let projectAndBackerProperty = MutableProperty<BackingData?>(nil)


public protocol ProjectPamphletViewModelInputs {
/// Call with the project given to the view controller.
func configureWith(projectOrParam: Either<Project, Param>, refTag: RefTag?)

Expand All @@ -14,6 +14,9 @@ public protocol ProjectPamphletViewModelInputs {
/// Call after the view loads and passes the initial TopConstraint constant.
func initial(topConstraint: CGFloat)

/// Call when the pledge CTA button is tapped
func pledgeCTAButtonTapped(with state: PledgeStateCTAType)

/// Call when pledgeRetryButton is tapped.
func pledgeRetryButtonTapped()

Expand All @@ -34,6 +37,12 @@ public protocol ProjectPamphletViewModelOutputs {
/// Emits a (project, isLoading) tuple used to configure the pledge CTA view
var configurePledgeCTAView: Signal<(Either<Project, ErrorEnvelope>, Bool), Never> { get }

var goToDeprecatedManagePledge: Signal<PledgeData, Never> { get }

var goToDeprecatedViewBacking: Signal<BackingData, Never> { get }

var goToManageViewPledge: Signal<PledgeData, Never> { get }

/// Emits a project and refTag to be used to navigate to the reward selection screen.
var goToRewards: Signal<(Project, RefTag?), Never> { get }

Expand Down Expand Up @@ -80,13 +89,38 @@ public final class ProjectPamphletViewModel: ProjectPamphletViewModelType, Proje
let freshProjectAndRefTag = freshProjectAndRefTagEvent.values()

let ctaButtonTapped = freshProjectAndRefTag
.takeWhen(self.backThisProjectTappedProperty.signal)
.map { project, refTag in
(project, refTag)
.takePairWhen(self.pledgeCTAButtonTappedProperty.signal)
.map(unpack)

let goToManagePledge = ctaButtonTapped
.filter { canShowManageViewPledgeScreen($0.0, state:$0.2) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this in action, it might actually be more readable to name those params. I'll leave this out to you, I'm OK with both - but wanted to raise it in case you were in favour of the other option.

.filter { project, _, state in canShowManageViewPledgeScreen(project, state: state) }

(this of course would apply to other places in this PR.

.map { (project, refTag, _) -> PledgeData in
PledgeData(project: project,
reward: reward(from: project.personalization.backing, inProject: project),
refTag: refTag)
}

self.goToManageViewPledge = goToManagePledge
.filter { _ in featureNativeCheckoutPledgeViewIsEnabled() }

self.goToDeprecatedManagePledge = ctaButtonTapped
.filter { shouldGoToDeprecatedManagePledge($0.0, state:$0.2) }
.map { (project, refTag, _) -> PledgeData in
PledgeData(project: project,
reward: reward(from: project.personalization.backing, inProject: project),
refTag: refTag)
}

self.goToDeprecatedViewBacking = ctaButtonTapped
.map { project, _, state in (project, state) }
.filter { shouldGoToDeprecatedViewBacking($0.0, state:$0.1) }
.map { project, _ in
BackingData(project, AppEnvironment.current.currentUser)
}

self.goToRewards = ctaButtonTapped
.filter { _ in userCanSeeNativeCheckout() }
.filter { canShowRewardsScreen($0.0, state:$0.2) }
.map { project, refTag, _ in (project, refTag) }

let project = freshProjectAndRefTag
.map(first)
Expand Down Expand Up @@ -148,9 +182,9 @@ public final class ProjectPamphletViewModel: ProjectPamphletViewModelType, Proje
.observeValues { AppEnvironment.current.cookieStorage.setCookie($0) }
}

private let backThisProjectTappedProperty = MutableProperty(())
public func backThisProjectTapped() {
self.backThisProjectTappedProperty.value = ()
private let pledgeCTAButtonTappedProperty = MutableProperty<PledgeStateCTAType?>(nil)
public func pledgeCTAButtonTapped(with state: PledgeStateCTAType) {
self.pledgeCTAButtonTappedProperty.value = state
}

private let configDataProperty = MutableProperty<(Either<Project, Param>, RefTag?)?>(nil)
Expand Down Expand Up @@ -191,6 +225,9 @@ public final class ProjectPamphletViewModel: ProjectPamphletViewModelType, Proje

public let configureChildViewControllersWithProject: Signal<(Project, RefTag?), Never>
public let configurePledgeCTAView: Signal<(Either<Project, ErrorEnvelope>, Bool), Never>
public let goToDeprecatedManagePledge: Signal<PledgeData, Never>
public let goToDeprecatedViewBacking: Signal<BackingData, Never>
public let goToManageViewPledge: Signal<PledgeData, Never>
public let goToRewards: Signal<(Project, RefTag?), Never>
public let setNavigationBarHiddenAnimated: Signal<(Bool, Bool), Never>
public let setNeedsStatusBarAppearanceUpdate: Signal<(), Never>
Expand Down Expand Up @@ -284,3 +321,37 @@ private func fetchProject(projectOrParam: Either<Project, Param>, shouldPrefix:

return projectProducer
}

private func reward(from backing: Backing?, inProject project: Project) -> Reward {
return backing?.reward
?? project.rewards.filter { $0.id == backing?.rewardId }.first
?? Reward.noReward
}

private func canShowRewardsScreen(_: Project, state: PledgeStateCTAType?) -> Bool {
guard let state = state else {
return false
}
return userCanSeeNativeCheckout() && (state == .pledge || state == .viewRewards)
}

private func canShowManageViewPledgeScreen(_ project: Project, state: PledgeStateCTAType?) -> Bool {
guard let isBacking = project.personalization.isBacking, let state = state else {
return false
}
return isBacking && (state == .manage || state == .viewBacking)
}

private func shouldGoToDeprecatedViewBacking(_ project: Project, state: PledgeStateCTAType?) -> Bool {
guard let isBacking = project.personalization.isBacking, let state = state else {
return false
}
return !featureNativeCheckoutPledgeViewIsEnabled() && isBacking && state == .viewBacking
}

private func shouldGoToDeprecatedManagePledge(_ project: Project, state: PledgeStateCTAType?) -> Bool {
guard let isBacking = project.personalization.isBacking, let state = state else {
return false
}
return !featureNativeCheckoutPledgeViewIsEnabled() && isBacking && state == .manage
}
Loading