Skip to content
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-1211: Add new thank-you page copy for post-campaign pledges #1968

Merged

Conversation

amy-at-kickstarter
Copy link
Contributor

📲 What

Adds new copy to the thank you page for post-campaign pledges.

👀 See

Screenshot 2024-03-05 at 5 22 21 PM

@@ -796,6 +796,7 @@
"You_canceled_your_pledge_for_this_project" = "You canceled your pledge for this project.";
"You_cant_use_this_credit_card_to_back_a_project_from_project_country" = "You can’t use this credit card to back a project from %{project_country}.";
"You_have_successfully_backed_project_html" = "You have successfully backed <b>%{project_name}</b>. This project is now one step closer to a reality, thanks to you. Spread the word!";
"You_have_successfully_backed_project_post_campaign_html" = "<p>You have successfully backed <b>%{project_name}</b>. Your pledge of <b>%{pledge_total}</b> has been collected.</p>\n<p>You’ll receive a confirmation email at %{user_email} when your rewards are ready to fulfill so that you can finalize and pay shipping and tax.</p>\n<p>This project is now one step closer to a reality, thanks to you. Spread the word!</p>";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the <p> tags. Because we interpret this string as HTML, raw newlines (\n) are stripped out. Instead, we have to actually treat it like HTML to get the formatting we want!

I think NSAttributedString has some tools for changing newline spacing, if we need to, but I think the default behavior is fine for now.

if featurePostCampaignPledgeEnabled(),
project.isInPostCampaignPledgingPhase,
let email = AppEnvironment.current.currentUserEmail {
let formattedTotal = Format.formattedCurrency(pledgeTotal, country: project.country)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scottkicks This is my first foray into formatting monetary strings in the app. Am I pulling the correct fields? project.country seemed like the most immediate/correct option, versus project.stats.country but let me know if I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a note for future refactoring, we should really be passing around something like a Money type that includes the currency with the amount. The raw double being passed around makes me nervous.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is how currency formatting is being handled elsewhere in the checkout flow. I agree it's not the best. I'm pushing to get some time allocated after the first release of PCP to make refactors like this. It's a little too in the weeds right now, I think.


if featurePostCampaignPledgeEnabled(),
project.isInPostCampaignPledgingPhase,
let email = AppEnvironment.current.currentUserEmail {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency on currentUserEmail is a little bit awkward - but it also seemed like a shame to add a synchronous server fetch to what is currently a static screen, especially when currentUserEmail already was supposed to be there...

@amy-at-kickstarter amy-at-kickstarter marked this pull request as ready for review March 5, 2024 22:29
@amy-at-kickstarter amy-at-kickstarter requested review from a team and scottkicks and removed request for a team March 5, 2024 22:29
@amy-at-kickstarter amy-at-kickstarter self-assigned this Mar 5, 2024
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@amy-at-kickstarter amy-at-kickstarter merged commit 83e9ddf into main Mar 6, 2024
4 checks passed
@amy-at-kickstarter amy-at-kickstarter deleted the feat/adyer/MBL-1211/connect-thank-you-page-copy branch March 6, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants