-
Notifications
You must be signed in to change notification settings - Fork 986
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
Add PPO cards for Fix Payment, Authorize Card, and Take Survey #2049
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2049 +/- ##
=========================================
Coverage 67.75% 67.75%
Complexity 2097 2097
=========================================
Files 353 353
Lines 21590 21590
Branches 3051 3051
=========================================
Hits 14628 14628
Misses 5317 5317
Partials 1645 1645 ☔ View full report in Codecov by Sentry. |
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, especially with the ui state design pattern 💯
colorFilter = ColorFilter.tint(color = colors.textSecondary) | ||
) | ||
}, | ||
onClickAction = { onFixPaymentClicked.invoke() }, |
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.
I assume you didn't mean to add this click action here. I can remove it in my PR for MBL-1469 if you want
colorFilter = ColorFilter.tint(color = colors.textSecondary) | ||
) | ||
}, | ||
onClickAction = { onFixPaymentClicked.invoke() }, |
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.
Or here
colorFilter = ColorFilter.tint(color = colors.textSecondary) | ||
) | ||
}, | ||
onClickAction = { onFixPaymentClicked.invoke() }, |
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.
Or here.
fun PPOCardView( | ||
viewType: PPOCardViewType, | ||
onCardClick: () -> Unit, | ||
projectName: String? = null, |
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.
@mtgriego how about sending the project itself or a subclass of it, instead of all the individual paramenters?. Probably a subclass that is populated with all the params that are not call to actions might make it simpler
Could we add tests as well for the UI card configuration? |
Happy to do this as a separate ticket https://kickstarter.atlassian.net/browse/MBL-1527 |
📲 What
Adding the card views for the upcoming PPO feature
🤔 Why
So users can see and interact with their backed projects more meaningfully
👀 See
designs from figma:
https://www.figma.com/design/r0WKSECmMTCJTSKQskt2SQ/Backed-Projects-Dashboard?node-id=1016-9617&t=7AtcGvFmztkcKp3o-0
Notes: Minimum clickable size for accessibility is typically 48dp, but designs are for 36dp tall buttons, will discuss with design
📋 QA
Not needed currenlty other than visual approval
Story 📖
MBL-1468
MBL-1470
MBL-1471