-
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
View pledge screen #826
View pledge screen #826
Conversation
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.
|
||
private lazy var editButton: UIBarButtonItem = { | ||
UIBarButtonItem( | ||
barButtonSystemItem: .edit, |
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.
Nino, we didn't have image resources to show the More
button (3 dots)?
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.
We don't. Android has it because it's native, not a resource.
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.
Can't we just get PDF from Abstract?
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.
Let me know if you'd want me to extract it ... I should have a copy of Sketch on my machine
let nav = UINavigationController(rootViewController: managePledgeViewController) | ||
|> \.modalPresentationStyle .~ .formSheet | ||
|
||
self.present(nav, animated: true, completion: nil) |
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.
Again, we can omit completion
to have less code.
@@ -0,0 +1,28 @@ | |||
import KsApi |
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.
Would be cool to also prepare ManagePledgeViewModelTests.swift
@@ -0,0 +1,77 @@ | |||
import KsApi |
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.
Would be cool to also prepare ManagePledgeViewControllerTests.swift
.filter { project, reward, _ -> Bool in | ||
project.state == .live && userIsBacking(reward: reward, inProject: project) | ||
} | ||
.map { project, reward, refTag in |
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.
We can't do this point free, correct?
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 didn't compile. Maybe because the point free syntax doesn't apply for constructors.
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.
Stupid Swift
@@ -157,6 +163,131 @@ final class RewardsCollectionViewModelTests: TestCase { | |||
XCTAssertEqual(self.vm.outputs.selectedReward(), project.rewards[endIndex - 1]) | |||
} | |||
|
|||
func testGoToManagePledge_featureNativeCheckoutPledgeView_Enabled() { | |||
let config = .template | |||
|> Config.lens.features .~ [Feature.nativeCheckoutPledgeView.rawValue: true] |
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.
How are we going to proceed from here taking nativeCheckoutV1
into account? Should we test for both of those flags or is nativeCheckoutPledgeView
sufficient?
|> \.abExperiments .~ [Experiment.Name.nativeCheckoutV1.rawValue: "experimental"]
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 ManagePledgeViewController
is only accessible if nativeCheckoutV1
is experimental, because we're presenting from the Rewards Carousel. That said, nativeCheckoutPledgeView
should be sufficient.
…wift Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>
…wift Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>
…wift Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>
…wift Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>
…os-oss into view-pledge-screen
Generated by 🚫 Danger |
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.
🚢 🚢 🚢
🚢 🚢 🚢
🚢 🚢 🚢
"Quick, merge your PR before Justin"
📲 What
🤔 Why
Cancel Pledge
,Manage Pledge
,Contact creator
...JIRA ticket
🛠 How
featureNativeCheckoutPledgeView
feature flag is enabled.. If this is not the case, the
DeprecatedRewardPledgeViewController
will be presented.👀 See
✅ Acceptance criteria
With
featureNativeCheckoutPledgeView
enabled.On the Rewards Carousel screen:
Select
button. ThePledgeViewController
should be presented.Manage your pledge
button. TheManagePledgeViewController
(blank) should be presented.Select this reward instead
button. ThePledgeViewController
should be presented.View your pledge
and the oldBackingViewController
should be presented (The logic to go to the new view pledge screen is not implemented in this PR).With
featureNativeCheckoutPledgeView
disabled.On the Rewards Carousel screen:
Select
button. TheDeprecatedRewardPledgeViewController
should be presented.Manage your pledge
button. TheDeprecatedRewardPledgeViewController
should be presented.Select this reward instead
button. TheDeprecatedRewardPledgeViewController
should be presented.View your pledge
and theBackingViewController
should be presented.