-
Notifications
You must be signed in to change notification settings - Fork 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
Adds route & sales page for Ultimate Traffic Guide #48154
Conversation
</strong> | ||
<br></br> | ||
{ translate( | ||
'It’s so effective you don’t even have to come close to a “perfect” site before you launch… even the ugliest site can drive an endless stream of business if you get this right!' |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 17 times:
translate( 'How to build a simple <b>“traffic magnet”</b> to instantly attract the right audience to your site and convert them to raving fans (It’s so effective you don’t even have to come close to a “perfect” site before you launch... even the ugliest site can drive an endless stream of business if you get this right!)' )
ES Score: 8
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~159 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~6382 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~138 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
</p> | ||
<p> | ||
{ translate( | ||
'The Ultimate Traffic GuideThis is a goldmine of traffic tips and how-tos that reveals the exact “Breakthrough Traffic” process we’ve developed over the past decade.' |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 17 times:
translate( 'The Ultimate Traffic Guide is a goldmine of traffic tips and how-tos that reveals the exact “Breakthrough Traffic” process we’ve developed over the past decade.' )
ES Score: 12
See 2 additional suggestions in the PR translation status page
</p> | ||
<p> | ||
<Button onClick={ handleDownloadPageButtonClick } primary> | ||
{ translate( 'Click here to download your copy now!' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Click here to download your copy now.' )
ES Score: 10
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D54155-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2 |
Rebased and forgot to push, fixing this branch 🤯 |
e639c97
to
0a8524b
Compare
</ul> | ||
<p> | ||
{ translate( | ||
'You’re going to want to print out all 96 pages of the Ultimate Traffic Guide and keep it by your desk, because you’re going to consult this guide often.' |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Print it out and keep it by your desk, because you’re going to consult this guide often.' )
ES Score: 6
export const WPCOM_TRAFFIC_GUIDE = 'traffic-guide'; | ||
|
||
export const hasTrafficGuidePurchase = ( purchases ) => | ||
!! ( | ||
purchases && purchases.find( ( purchase ) => WPCOM_TRAFFIC_GUIDE === purchase.productSlug ) | ||
); | ||
|
||
export const downloadTrafficGuide = () => { | ||
window.location.href = 'https://public-api.wordpress.com/wpcom/v2/traffic-guide-download'; | ||
}; |
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.
Not sure if this is the best place for these exported consts.
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 main problem is with the WPCOM_TRAFFIC_GUIDE constant - it looks like Calypso doesn't have a centralized place or a unified standard for naming such constants. For example, the plans slugs are in lib/plans/constants.js and transfers are in lib/domains/constants. Jetpack products are in lib/products-values/constants.js
Should you create lib/ebooks/constants for this specific constant? Not sure. Maybe. An alternative would be to simply hardcode the product slug instead of use a constant, like the Concierge Session does.
If it was me, I'd probably use the constant and put it where the Jetpack constants are. Looks like Jetpack folks tried to create a unified place but stopped after adding their constants and nobody else picked it up.
@@ -721,4 +721,21 @@ describe( 'getThankYouPageUrl', () => { | |||
} ); | |||
expect( url ).toBe( '/checkout/thank-you/foo.bar/1234abcd' ); | |||
} ); | |||
|
|||
it( 'redirects to thank you page (with traffic guide display mode) if traffic guide is in cart', () => { |
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.
Thank you for adding a test! ❤️
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 works well for me. Left a few notes.
export const WPCOM_TRAFFIC_GUIDE = 'traffic-guide'; | ||
|
||
export const hasTrafficGuidePurchase = ( purchases ) => | ||
!! ( | ||
purchases && purchases.find( ( purchase ) => WPCOM_TRAFFIC_GUIDE === purchase.productSlug ) | ||
); | ||
|
||
export const downloadTrafficGuide = () => { | ||
window.location.href = 'https://public-api.wordpress.com/wpcom/v2/traffic-guide-download'; | ||
}; |
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 main problem is with the WPCOM_TRAFFIC_GUIDE constant - it looks like Calypso doesn't have a centralized place or a unified standard for naming such constants. For example, the plans slugs are in lib/plans/constants.js and transfers are in lib/domains/constants. Jetpack products are in lib/products-values/constants.js
Should you create lib/ebooks/constants for this specific constant? Not sure. Maybe. An alternative would be to simply hardcode the product slug instead of use a constant, like the Concierge Session does.
If it was me, I'd probably use the constant and put it where the Jetpack constants are. Looks like Jetpack folks tried to create a unified place but stopped after adding their constants and nobody else picked it up.
</h2> | ||
<p> | ||
{ translate( | ||
'We developed this 96 page guide to teach you every modern website traffic trick you need to know in 2020 and beyond.' |
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 will soon become 2021. Do you think we should make the year dynamic?
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 catch! Made this dynamic.
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 think it's good to go but it can't be merged just yet, we need the tax code diffs to be merged first.
6a84407
to
0039609
Compare
const defaultVariation = () => ( | ||
<> | ||
<h1 className="ultimate-traffic-guide__header"> | ||
{ translate( 'Introducing the WordPress.com Ultimate Traffic Guide' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 1 times:
translate( 'Introducing The Ultimate Traffic Guide' )
ES Score: 8
) } | ||
</h1> | ||
<h2 className="ultimate-traffic-guide__sub-header"> | ||
{ translate( 'Claim your copy of the Ultimate Traffic Guide today' ) } |
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.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 1 times:
translate( 'Claim your copy of the <br /><em>Ultimate Traffic Guide</em>' )
ES Score: 7
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
Depends on D54575-code to be reviewed and launched before merging or may cause Payments Admin issues
* Adds sales and download page for Traffic Guide * Adds thank you page * Fixed placeholder class name * Adds test for checkout thank you page * Adds alt copy for AB test * Show the marketing card only for English locales * Updates logo and copy of card * Adds alternate copy for experiment * Adds download link to checkout thank you page * Fixes incorrect import of the Experiment component * Moves traffic guide traffic constant to lib * Makes the year dynamic * Adds reference cost calculation * Use native Date instead of moment * Removes logic from old getCheckoutCompleteRedirectPath
Changes proposed in this Pull Request
Testing instructions
Marketing tools card
Sales page
Image Link: https://d.pr/i/jOxhOq
treatment
for thetraffic_guide_copy_test
experiment. The alternate copy should be shown.(This is currently broken)(works now)Checkout
Image Link: https://d.pr/i/S5o3ta
Image Link: https://d.pr/i/Ognmt4
Download Page
Accessing the Traffic Guide page through the card should now show a Download page instead of the Sales page
Image Link: https://d.pr/i/0ZR8sv
Clicking on the download CTA should download the file.
The purchase is per-user, so this behaviour should remain consistent irrespective of the site chosen.