-
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
[MBL-1958] Use New PLOT Disclaimer Copy On Project Fragment #2237
Conversation
… into scott/mbl-1958
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.
👍 Nice! Great to have this moved to the server.
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.
LGTM overall! Just one small concern: could we clean up the unused thresholdAmount
fields?
@@ -123,7 +123,7 @@ public final class PledgePaymentPlansOptionViewModel: | |||
|
|||
self.ineligibleBadgeText = configData | |||
.filterWhenLatestFrom(ineligible, satisfies: { $0 == true }) | |||
.map { getIneligibleBadgeText(with: $0.project, thresholdAmount: $0.thresholdAmount) } |
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.
Since thresholdAmount
will no longer be used, should we remove all references to it? Also, what about PledgePaymentPlansAndSelectionData.thresholdAmount
? It might be a good idea to clean those up if they're not needed anymore.
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.
good call!
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.
Nice!!!
📲 What
Exposes a new
pledgeOverTimeMinimumExplanation
property on the Project fragment and implements it.The threshold should be $125
🤔 Why
To keep this string dynamic in case the plot eligibility threshold changes in the future, payments added it for us on the Project fragment.
🛠 How
👀 See
✅ Acceptance criteria