Domains: Add professional email upsell route #54941

runofthemillgeek · 2021-07-27T20:06:16Z

Changes proposed in this Pull Request

This PR is the first step to proposed changes in domain purchase flow. It adds a new route for the professional email upsell page at:

http://wordpress.com/domains/add/:domain/email/:siteName

The content for this page will be added in an upcoming PR based on this one.

Side note: I referred to @daledupreez's previous approach at #50152 and @stephanethomas's work on #53712 to come up with this.

Testing instructions

Run git checkout add/titan-email-upsell-route and start your server
Log into a WordPress.com account that has a site
Open the Domains page (replace yoursite with a site slug that you have): http://calypso.localhost:3000/domains/manage/yoursite.wordpress.com
Click Add Domain button > to this site from the popover
Search for a domain or choose one of the suggestions by clicking Select button

Once the page loads, replace google-workspace in the domain with email. For example:
If the URL shown is:

http://calypso.localhost:3000/domains/add/example.in/google-workspace/example.wordpress.com

replace it with:

http://calypso.localhost:3000/domains/add/example.in/email/example.wordpress.com

Verify that the new route loads and renders without errors

Screenshots

daledupreez

This is looking really good, @sangeeth96! The description and testing instructions covered pretty much everything. The only thing that may be worth adding is making it clear that the upsell content will be coming in another PR.

Other than that, I have a few small comments that you can take on. I am hoping we can get this shipped tomorrow!

daledupreez · 2021-07-28T11:46:55Z

client/my-sites/domains/email-providers-upsell/index.jsx

+	const translate = useTranslate();
+
+	const handleGoBack = () => {
+		page( `/domains/add/${ selectedSiteSlug }` );


I think it would be better to use domainAddNew( selectedSiteSlug) from calypso/my-sites/domains/paths.js.

daledupreez · 2021-07-28T11:47:50Z

client/my-sites/domains/email-providers-upsell/index.jsx

+	};
+
+	return (
+		<Fragment>


Nit: The Letero team has been trying to use <> instead of Fragment or React.Fragment.
Not blocking, but a minor thing.

@daledupreez I used that first, but then thought you peeps prefer to be explicit about it and changed it back anyway. 😅 But will do this. ✌️

daledupreez · 2021-07-28T11:49:36Z

client/my-sites/domains/email-providers-upsell/index.jsx

+				{ translate( 'Register %(domain)s', { args: { domain } } ) }
+			</HeaderCake>
+
+			<PromoCard


As a question, is the goal to improve/expand this content in a follow-up PR?

@daledupreez Yes, that is indeed the goal. I've updated the PR description as you requested.

daledupreez · 2021-07-28T11:51:37Z

client/my-sites/domains/controller.jsx

@@ -21,6 +21,7 @@ import {
 import { getCurrentUser } from 'calypso/state/current-user/selectors';
 import DomainSearch from './domain-search';
 import SiteRedirect from './domain-search/site-redirect';
+import EmailProvidersUpsell from './email-providers-upsell';


Nit: I know this import list isn't in order, but I'd prefer that the new import not make it less ordered! Could you move this new import one line up so it's below import DomainSearch?

On it. Is there an eslint config or some other editor automation folks use to automate this or is this a manual work at the moment? 🤔

Oh nvm, I see that work is being done on this. ❤️

daledupreez · 2021-07-28T11:52:33Z

client/my-sites/domains/controller.jsx

@@ -212,6 +213,25 @@ const googleAppsWithRegistration = ( context, next ) => {
 	next();
 };

+const emailWithRegistration = ( context, next ) => {


For full specificity and clarity, it may be better to call this emailUpsellForDomainRegistration.

daledupreez · 2021-07-28T13:40:23Z

client/my-sites/domains/email-providers-upsell/index.jsx

+
+			<PromoCard
+				isPrimary
+				title={ translate( 'Add a professional email address to %(domainName)s', translateArgs ) }


Given that we're only using translateArgs once, it may be simpler to inline the details.

runofthemillgeek · 2021-07-29T07:41:53Z

@daledupreez Addressed the comments. Let me know if there's more.

daledupreez

I should have gotten to this earlier today! 😐

There are still two more fixes/tweaks I'd like you to add before we merge and deploy. (I was going to try and handle them myself, but I don't have permissions to push them to the talk-to repo.)

daledupreez · 2021-07-29T12:53:23Z

client/my-sites/domains/controller.jsx

+			/>
+			<DocumentHead
+				title={ translate( 'Register %(domain)s', {
+					args: { domain: context.params.registerDomain },


Small bug: this should be context.params.domain - registerDomain is not defined in the main URL path as a variable.

Suggested change

args: { domain: context.params.registerDomain },

args: { domain: context.params.domain },

Whoops, gonna fix that.

daledupreez · 2021-07-29T12:54:19Z

client/my-sites/domains/email-providers-upsell/index.jsx

+ * Style dependencies
+ */
+import emailIllustration from 'calypso/assets/images/email-providers/email-illustration.svg';
+import { domainAddNew } from '../paths';


This should be under Internal dependencies and should be relative to the repo rather than the current file. (I''ll add a comment RE where to add.)

Suggested change

import { domainAddNew } from '../paths';

Made the change to use the repo alias.

client/my-sites/domains/email-providers-upsell/index.jsx

runofthemillgeek · 2021-07-29T15:27:47Z

I don't have permissions to push them to the talk-to repo.)

I genuinely thought PRs were editable by upstream maintainers but turns out org forks can't allow this (isaacs/github#1681).

I'll see if I can add you to the repo's collaborators list.

daledupreez

LGTM! I'll get this deployed first thing tomorrow.

Congrats on your first Calypso PR! 🎉

a8ci18n · 2021-07-30T07:16:35Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/6303536

Thank you @sangeeth96 for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2021-08-05T15:28:40Z

Translation for this Pull Request has now been finished.

Sangeeth Sudheer added 5 commits July 27, 2021 18:17

Add route and basic layout for email upsell page

4d0e4b3

Add upsell path helper function

e80572a

Move upsell path handler definition

3038fe7

Reorder imports and definitions

6be6acd

Replace placeholder domain with domain prop

75913f0

runofthemillgeek requested a review from a team as a code owner July 27, 2021 20:06

github-actions bot added the OSS Citizen label Jul 27, 2021

daledupreez reviewed Jul 28, 2021

View reviewed changes

Sangeeth Sudheer added 5 commits July 29, 2021 11:45

Use domainAddNew helper to get path

bf1a2e5

Use Fragment shorthand syntax

4fddb54

Change import order of EmailProvidersUpsell

3f1f8bc

Rename controller function to be more specific

b76aee6

Inline translateArgs value

35838b3

runofthemillgeek requested a review from daledupreez July 29, 2021 07:12

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 29, 2021

daledupreez reviewed Jul 29, 2021

View reviewed changes

Sangeeth Sudheer added 2 commits July 29, 2021 20:41

Fix: registerDomain -> domain

662750e

Reorder, use path alias for domainAddNew import

1a2ff30

runofthemillgeek requested a review from daledupreez July 29, 2021 15:12

daledupreez approved these changes Jul 29, 2021

View reviewed changes

daledupreez merged commit e809331 into Automattic:trunk Jul 30, 2021

runofthemillgeek mentioned this pull request Aug 5, 2021

Domains: Add titan email upsell page contents #55183

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Domains: Add professional email upsell route #54941

Domains: Add professional email upsell route #54941

runofthemillgeek commented Jul 27, 2021 •

edited

Loading

daledupreez left a comment

daledupreez Jul 28, 2021

daledupreez Jul 28, 2021

runofthemillgeek Jul 29, 2021 •

edited

Loading

daledupreez Jul 28, 2021

runofthemillgeek Jul 29, 2021

daledupreez Jul 28, 2021

runofthemillgeek Jul 29, 2021 •

edited

Loading

runofthemillgeek Jul 29, 2021

daledupreez Jul 28, 2021

daledupreez Jul 28, 2021

runofthemillgeek commented Jul 29, 2021

daledupreez left a comment

daledupreez Jul 29, 2021

runofthemillgeek Jul 29, 2021

runofthemillgeek Jul 29, 2021

daledupreez Jul 29, 2021

runofthemillgeek Jul 29, 2021

runofthemillgeek commented Jul 29, 2021

daledupreez left a comment

a8ci18n commented Jul 30, 2021

a8ci18n commented Aug 5, 2021

	args: { domain: context.params.registerDomain },
	args: { domain: context.params.domain },

Domains: Add professional email upsell route #54941

Domains: Add professional email upsell route #54941

Conversation

runofthemillgeek commented Jul 27, 2021 • edited Loading

Changes proposed in this Pull Request

Testing instructions

Screenshots

daledupreez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runofthemillgeek Jul 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runofthemillgeek Jul 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runofthemillgeek commented Jul 29, 2021

daledupreez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runofthemillgeek commented Jul 29, 2021

daledupreez left a comment

Choose a reason for hiding this comment

a8ci18n commented Jul 30, 2021

a8ci18n commented Aug 5, 2021

runofthemillgeek commented Jul 27, 2021 •

edited

Loading

runofthemillgeek Jul 29, 2021 •

edited

Loading

runofthemillgeek Jul 29, 2021 •

edited

Loading