-
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-361] Select new reward polish #899
Merged
Merged
Changes from all commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
3a5e5bd
Updated validation
justinswart fffa982
Fix snapshot tests, add submitButtonHidden
justinswart f933d99
Merge branch 'master' into change-payment-method-ui
justinswart 97f4cd8
Add PledgeAmountSummaryViewController
justinswart 02f290e
Merge branch 'master' into change-payment-method-ui
justinswart 02bebe8
Add updateButtonTapped
justinswart 82b6642
Add snapshot test and vm test
justinswart a4b8ffd
lol
justinswart e70e2e6
Merge branch 'master' into change-payment-method-ui
justinswart 5cbf40b
Remove unused var
justinswart afa9c9a
Rename signal, fix payment source ID comparison
justinswart e2b3afd
Fix tests
justinswart 1d69e99
Merge branch 'master' into change-payment-method-ui
justinswart c189ebe
Fix tests, simplify hiding of shipping location stackview
justinswart 031ebc2
Wire up change payment method to mutation for payment source and Appl…
justinswart b1de7b0
Add updateReward context to PledgeViewModel
justinswart 7c9f91c
Add tests
justinswart 5a03263
Merge branch 'master' into change-payment-method-ui
justinswart 30e77cc
Merge branch 'change-payment-method-ui' into change-payment-method-mu…
justinswart 891f50b
Merge branch 'change-payment-method-mutation' into select-this-reward…
justinswart 5bf6de3
Fix line lengths
justinswart 3a6ae51
Merge branch 'master' into change-payment-method-ui
justinswart 189adbe
Merge branch 'change-payment-method-ui' into change-payment-method-mu…
justinswart 5ae6f86
Remove payment method when backing with Apple Pay and vice versa
justinswart d68df53
Update tests for apple pay sheet race condition fixes
justinswart a8092af
Replace zip with simpler signals
justinswart d65823d
Add Stripe token error test
justinswart 4c49d3a
Fix snapshots
justinswart 0ed69e0
Commit the actual snapshots
justinswart 6d7bf10
Merge branch 'change-payment-method-ui' into change-payment-method-mu…
justinswart 43c7655
Merge branch 'change-payment-method-mutation' into select-this-reward…
justinswart 319688f
Merge branch 'master' into select-this-reward-instead
justinswart 4445e9b
Hide payment methods when selecting a new reward
justinswart f6a3f26
Added output to change Carousel title according to context
Scollaco c079a94
Fixed and added test for title
Scollaco d406f97
Added logic to centralize backed reward
Scollaco 7d6991c
SwiftFormat
Scollaco fbb661d
Merge branch 'master' into select-new-reward-polish
justinswart 311f48e
Remove code that crept back in during merge
justinswart c84f8ad
Fixed logic to centralize the backed reward
Scollaco 88ba19b
Fixed test
Scollaco 38ac910
Merge branch 'master' into select-new-reward-polish
Scollaco 044fcef
Merge branch 'master' into select-new-reward-polish
Scollaco b294416
Merge branch 'master' into select-new-reward-polish
Scollaco cd582dd
Renamed output
Scollaco c0cfd26
Merge remote-tracking branch 'oss/master' into select-new-reward-polish
Scollaco bd02acd
Updated private func to use sharedFunction
Scollaco 880355e
Swiftformat
Scollaco d6261bb
Merge branch 'select-new-reward-polish' of https://github.com/kicksta…
Scollaco File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,12 +5,18 @@ import ReactiveSwift | |
|
||
public typealias PledgeData = (project: Project, reward: Reward, refTag: RefTag?) | ||
|
||
public enum RewardsCollectionViewContext { | ||
case createPledge | ||
case managePledge | ||
} | ||
|
||
public protocol RewardsCollectionViewModelInputs { | ||
func configure(with project: Project, refTag: RefTag?) | ||
func configure(with project: Project, refTag: RefTag?, context: RewardsCollectionViewContext) | ||
func rewardCellShouldShowDividerLine(_ show: Bool) | ||
func rewardSelected(with rewardId: Int) | ||
func traitCollectionDidChange(_ traitCollection: UITraitCollection) | ||
func viewDidAppear() | ||
func viewDidLayoutSubviews() | ||
func viewDidLoad() | ||
func viewWillAppear() | ||
} | ||
|
@@ -23,6 +29,9 @@ public protocol RewardsCollectionViewModelOutputs { | |
var navigationBarShadowImageHidden: Signal<Bool, Never> { get } | ||
var reloadDataWithValues: Signal<[(Project, Either<Reward, Backing>)], Never> { get } | ||
var rewardsCollectionViewFooterIsHidden: Signal<Bool, Never> { get } | ||
var scrollToBackedRewardIndexPath: Signal<IndexPath, Never> { get } | ||
var title: Signal<String, Never> { get } | ||
|
||
func selectedReward() -> Reward? | ||
} | ||
|
||
|
@@ -46,6 +55,18 @@ public final class RewardsCollectionViewModel: RewardsCollectionViewModelType, | |
let rewards = project | ||
.map { $0.rewards } | ||
|
||
self.title = configData | ||
.map(third) | ||
.combineLatest(with: self.viewDidLoadProperty.signal.ignoreValues()) | ||
.map(first) | ||
.map(titleForContext) | ||
|
||
self.scrollToBackedRewardIndexPath = Signal.combineLatest(project, rewards) | ||
.takeWhen(self.viewDidLayoutSubviewsProperty.signal.ignoreValues()) | ||
.map(backedReward(_:rewards:)) | ||
.skipNil() | ||
.take(first: 1) | ||
|
||
self.reloadDataWithValues = Signal.combineLatest(project, rewards) | ||
.map { project, rewards in | ||
rewards.map { (project, Either<Reward, Backing>.left($0)) } | ||
|
@@ -108,9 +129,9 @@ public final class RewardsCollectionViewModel: RewardsCollectionViewModelType, | |
) | ||
} | ||
|
||
private let configDataProperty = MutableProperty<(Project, RefTag?)?>(nil) | ||
public func configure(with project: Project, refTag: RefTag?) { | ||
self.configDataProperty.value = (project, refTag) | ||
private let configDataProperty = MutableProperty<(Project, RefTag?, RewardsCollectionViewContext)?>(nil) | ||
public func configure(with project: Project, refTag: RefTag?, context: RewardsCollectionViewContext) { | ||
self.configDataProperty.value = (project, refTag, context) | ||
} | ||
|
||
private let rewardCellShouldShowDividerLineProperty = MutableProperty<Bool>(false) | ||
|
@@ -133,6 +154,11 @@ public final class RewardsCollectionViewModel: RewardsCollectionViewModelType, | |
self.viewDidAppearProperty.value = () | ||
} | ||
|
||
private let viewDidLayoutSubviewsProperty = MutableProperty(()) | ||
public func viewDidLayoutSubviews() { | ||
self.viewDidLayoutSubviewsProperty.value = () | ||
} | ||
|
||
private let viewDidLoadProperty = MutableProperty(()) | ||
public func viewDidLoad() { | ||
self.viewDidLoadProperty.value = () | ||
|
@@ -150,6 +176,8 @@ public final class RewardsCollectionViewModel: RewardsCollectionViewModelType, | |
public let navigationBarShadowImageHidden: Signal<Bool, Never> | ||
public let reloadDataWithValues: Signal<[(Project, Either<Reward, Backing>)], Never> | ||
public let rewardsCollectionViewFooterIsHidden: Signal<Bool, Never> | ||
public let scrollToBackedRewardIndexPath: Signal<IndexPath, Never> | ||
public let title: Signal<String, Never> | ||
|
||
private let selectedRewardProperty = MutableProperty<Reward?>(nil) | ||
public func selectedReward() -> Reward? { | ||
|
@@ -159,3 +187,18 @@ public final class RewardsCollectionViewModel: RewardsCollectionViewModelType, | |
public var inputs: RewardsCollectionViewModelInputs { return self } | ||
public var outputs: RewardsCollectionViewModelOutputs { return self } | ||
} | ||
|
||
private func titleForContext(_ context: RewardsCollectionViewContext) -> String { | ||
return context == .createPledge ? Strings.Back_this_project() : Strings.Choose_another_reward() | ||
} | ||
|
||
private func backedReward(_ project: Project, rewards: [Reward]) -> IndexPath? { | ||
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. Please update this to use the function @ifbarrera is using in #905 when that merges. https://github.com/kickstarter/ios-oss/pull/905/files#diff-234cfa58d101616352832cb332756878R33 |
||
guard let backing = project.personalization.backing else { | ||
return nil | ||
} | ||
|
||
let backedReward = reward(from: backing, inProject: project) | ||
return rewards | ||
.firstIndex(where: { $0.id == backedReward.id }) | ||
.flatMap { IndexPath(row: $0, section: 0) } | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Using
viewWillAppearProperty
here didn't do anything, soviewDidLayoutSubviews
was the way to go.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.
Interesting 🤔 I believe it needs a run loop to render the cells. We could probably go ahead with this but for interest sake, if you used
viewWillAppear
and added.ksr_delay(AppEnvironment.current.apiDelayInterval, on: AppEnvironment.current.scheduler)
which is a delay of0
and just puts the emission on the next run loop, that would also work.