feat(ui): add campaign impact reporting - #1027
Conversation
|
hey, implemented the campaign impact reporting feature for #672. added an Impact tab that shows completed milestones and funding stats. ready for review — happy to make adjustments if needed! |
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
There was a problem hiding this comment.
thanks for this, the impact tab is a nice addition. the tab wiring in CampaignTabs is clean, the component follows the existing zinc/dark styling, and you translated the tab labels and all new strings in both en and es instead of hardcoding them. also good that you localized the amount formatting via formatXlm with the active locale.
on CI: the red checks are not yours. the latest lint/typecheck runs fail with parse errors in src/components/WalletContext.tsx and src/app/[locale]/causes/[id]/CauseDetailClient.tsx, files this PR does not touch, so it is a stale/broken base at merge time. a rebase on current main should clear it. branch name check already passes on the latest run after your rename.
a few asks before merge:
- src/components/ImpactReport.tsx:36 the "updates posted" stat is hardcoded to "0". showing a permanent zero is misleading on campaigns that do have updates. either pull the real count (the updates section already fetches them) or drop this stat until the data is available.
- src/components/ImpactReport.tsx:42
stats[2].value === "0"is always true because of the hardcoded value above, so the empty-state condition is effectively justcompletedMilestones.length === 0. once you fix ask 1 this works, otherwise simplify the condition so the intent is clear. - src/components/ImpactReport.tsx:96 using the array index as the react key is fine for a static list, but milestones have a description/target you could key on to be safe if the list ever reorders.
- messages/fr.json and messages/pt.json (from #1028) will not have the new CauseDetail.tabs and ImpactReport keys, so whichever of these two PRs lands second needs to add the keys to the other locales. worth coordinating with #1028.
|
Auto-review failed (API error). Leaving PR for human review. |
|
the review items are fixed: the hardcoded updates-posted stat is gone and the empty state now shows the noMilestones message, nice. two tiny cleanups: updatesPosted and outcomeHighlights keys in en/es are no longer referenced, and milestone.description as a react key could collide, consider the index. main blocker left is the base: please rebase onto current main and fix the commit-message check, then this is mergeable. |
|
hey @davidmaronio, quick update — the three code review items you raised have already been addressed by @Richiey1 in commits 2393f74 and de98c6b: the hardcoded 'updates posted' stat is gone, the empty-state condition is simplified, and milestones now use description-based React keys. the ImpactReport and CampaignTabs changes are clean. the remaining CI failures (lint, typecheck, build) are the same pre-existing parse errors in WalletContext.tsx and CauseDetailClient.tsx that exist on upstream/main — this PR doesn't touch those files. happy to rebase onto latest main if that helps clear the stale checks. |
|
Auto-review failed (API error). Leaving PR for human review. |
Add ImpactReport component showing completed milestones and funding stats. Add Impact tab to CampaignTabs with i18n support (en/es). Closes Iris-IV#672
- Drop hardcoded "updates posted" stat (was always 0, misleading) - Simplify empty-state condition to completedMilestones.length === 0 - Use milestone description as React key instead of array index - Remove noUpdates translation key that is no longer referenced
5ecac2f to
6ca9879
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
|
hey @davidmaronio, rebased onto current main and addressed the remaining items:
the review feedback from @Richiey1 is already in place (hardcoded stat removed, empty state simplified, description-based keys were there but I’ve switched to index per your note). the merge conflicts in en/es are resolved. the remaining CI failures on this PR are the same pre-existing parse errors in WalletContext.tsx and CauseDetailClient.tsx that exist on upstream/main — this branch doesn’t touch those files. let me know if anything else is needed. |
|
@davidmaronio , kindly review |
Closes #672
Summary of Changes
Adds a new Impact tab to the campaign detail page that shows donors the tangible outcomes of their contributions. The tab displays completed milestones with their targets and a summary of total funds raised vs. milestones hit.
What Changed
New component:
src/components/ImpactReport.tsxcampaign.milestones[]and filters completed ones (wheretargetAmount <= amountRaised)Modified:
src/components/CampaignTabs.tsxImpactReportwhen Impact tab is activeuseTranslationsfor tab labelsi18n: Added translations for both English and Spanish
messages/en.json: CauseDetail.tabs.impact + ImpactReport namespacemessages/es.json: Same keys with Spanish translationsTesting / Local Verification
npm run lint— passes (only pre-existing warnings in unrelated files)npm run format:check— passesnpm run typecheck— passes (only pre-existing errors in unrelated files)npm test -- i18n— 3/3 tests pass (translation parity verified)git log upstream/main..HEAD --oneline— clean history, commits from thebabalolaNotes
The build (
npm run build) and some CI checks fail due to pre-existing syntax errors inWalletContext.tsxandCauseDetailClient.tsx(unrelated to this PR). These exist onupstream/mainand block the build for everyone.The ImpactReport component is ready for enhancement — future work can parse creator updates for outcome metrics (e.g., "200 families served") and display them as highlights.