Add/new email comparison card props #58632

AllTerrainDeveloper · 2021-11-30T09:04:35Z

Changes proposed in this Pull Request

Create a new component "EmailProvidersComparisonStacked" placeholder to wire things up.

Modify EmailPromoCard to match Saygun's design

Add a new property to inject a header component (in addition to the old title string)

Modify Price component to add a new property "additional information"

Create a new EmailProviderStackedCard to provide the Saygun's modifications (features inside of the card, logo at the end of it)

Work in progress

Testing instructions

Go to a site with domains without email subscription
Click on 'Add email' for the desired site
Once you are in the comparison page, add at the end the following flag: ?flags=emails/new-email-comparison (it is only

Remarks: visible through the path /email/{domain}/purchase/{site}?flags=emails/new-email-comparison

You should see the beginning of a new component being developed.

Related to 1200182182542585-as-1201414571816235

Subsequent PR

github-actions · 2021-11-30T09:04:57Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-21390
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-21390&env=jetpack

client/my-sites/email/email-providers-stacked-comparison/email-provider-stacked-card/index.tsx

a8ci18n · 2021-11-30T09:10:30Z

...my-sites/email/email-providers-stacked-comparison/provider-cards/professional-email-card.tsx

+const badge = <img src={ poweredByTitanLogo } alt={ translate( 'Powered by Titan' ) } />;
+const getTitanFeatures = () => {
+	return [
+		translate( 'Inbox, calendars, and contacts' ),


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 20 times:
translate( 'Email, calendars, and contacts' ) ES Score: 6

daledupreez

I have a bunch of comments from inspection. I'll loop back to test later this evening.

daledupreez · 2021-11-30T14:56:32Z

client/components/promo-section/promo-card/index.tsx

+				{ titleComponent && (
+					<>
+						{ titleComponent }
+						{ badge && <Badge className="promo-card__title-badge">{ badge }</Badge> }


Nit: it may be worth defining the badge wrapper once rather than repeating it.

const badgeComponent = badge ? <Badge className="promo-card__title-badge">{ badge }</Badge> : null;

and then refer to this component in the later code.

daledupreez · 2021-11-30T14:57:39Z

client/components/promo-section/promo-card/index.tsx

+	title?: string | TranslateResult;
+	titleComponent?: ReactElement;
 	isPrimary?: boolean;
-	badge?: string;
+	badge?: string | ReactElement;


It looks like some of these types are imported via import instead of import type.

daledupreez · 2021-11-30T14:58:52Z

client/components/promo-section/promo-card/price.tsx

@@ -1,14 +1,19 @@
 import classnames from 'classnames';
 import { useTranslate, TranslateResult } from 'i18n-calypso';
-import { FunctionComponent } from 'react';
+import { FunctionComponent, ReactElement } from 'react';


Can you only import the ReactElement type?

Suggested change

import { FunctionComponent, ReactElement } from 'react';

import { FunctionComponent } from 'react';

import type { ReactElement } from 'react';

daledupreez · 2021-11-30T15:00:14Z

...providers-stacked-comparison/email-provider-stacked-card/email-provider-stacked-features.tsx

+import { TranslateResult, useTranslate } from 'i18n-calypso';
+import { FunctionComponent } from 'react';
+import { preventWidows } from 'calypso/lib/formatting';


Nit: can you import only the TranslateResult type?

Suggested change

import { TranslateResult, useTranslate } from 'i18n-calypso';

import { FunctionComponent } from 'react';

import { preventWidows } from 'calypso/lib/formatting';

import { useTranslate } from 'i18n-calypso';

import { FunctionComponent } from 'react';

import { preventWidows } from 'calypso/lib/formatting';

import type { TranslateResult } from `i18-calypso';

daledupreez · 2021-11-30T15:01:33Z

...providers-stacked-comparison/email-provider-stacked-card/email-provider-stacked-features.tsx

+	);
+};
+
+export interface EmailProviderStackedFeatureProps {


Nit: I prefer having the prop types defined before they get used, otherwise I need to scan down to find them.

daledupreez · 2021-11-30T15:22:54Z

client/my-sites/email/email-providers-stacked-comparison/index.tsx

+			productsList: getProductsList( state ),
+			requestingSiteDomains: isRequestingSiteDomains( state, domainName ),
+			selectedSite,
+			titanMailProduct: getProductBySlug( state, TITAN_MAIL_MONTHLY_SLUG ),


Maybe call this titanMailMonthlyProduct to prep for the annual product?

daledupreez · 2021-11-30T15:23:12Z

client/my-sites/email/email-providers-stacked-comparison/index.tsx

+			domain,
+			domainName,
+			domainsWithForwards: getDomainsWithForwards( state, domains ),
+			gSuiteProduct: getProductBySlug( state, GOOGLE_WORKSPACE_BUSINESS_STARTER_YEARLY ),


Maybe call this gSuiteAnnualProduct to prep for the monthly product?

daledupreez · 2021-11-30T15:23:57Z

...my-sites/email/email-providers-stacked-comparison/provider-cards/professional-email-card.tsx

+import React from 'react';
+import poweredByTitanLogo from 'calypso/assets/images/email-providers/titan/powered-by-titan.svg';
+import { getTitanProductName } from 'calypso/lib/titan';
+import { ProviderCard } from 'calypso/my-sites/email/email-providers-stacked-comparison';


Can we import this as a type?

Suggested change

import { ProviderCard } from 'calypso/my-sites/email/email-providers-stacked-comparison';

import type { ProviderCard } from 'calypso/my-sites/email/email-providers-stacked-comparison';

daledupreez · 2021-11-30T15:25:24Z

...my-sites/email/email-providers-stacked-comparison/provider-cards/professional-email-card.tsx

+	formFields: <form> Placeholder </form>,
+	formattedPrice: '3.5$',


I am not sure if these props should come from here, as they need to be computed dynamically based on the user's currency and need to wait for the query responses from the server.

Yes, all those price tags has been addressed in the next pull request. Those were only placeholders :)

daledupreez · 2021-11-30T15:25:40Z

...my-sites/email/email-providers-stacked-comparison/provider-cards/professional-email-card.tsx

+	formFields: <form> Placeholder </form>,
+	formattedPrice: '3.5$',
+	onExpandedChange: noop,
+	providerKey: '',


Should this not be 'titan'?

daledupreez

I've just tested and things look reasonable, especially if there are some hard-coding items that we're cleaning up in a follow-up PR (mainly the price values).

I think we do need to address the translation items and type imports in this PR, mainly because we want to get the correct strings queued up for translation.

matticbot · 2021-11-30T21:30:19Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~1391 bytes added 📈 [gzipped])

name            parsed_size           gzip_size
email               +5374 B  (+1.0%)    +1316 B  (+0.9%)
domains              +877 B  (+0.1%)     +137 B  (+0.0%)
scan                  +63 B  (+0.0%)      +26 B  (+0.0%)
jetpack-search        +63 B  (+0.0%)      +26 B  (+0.0%)
earn                  +63 B  (+0.0%)      +23 B  (+0.0%)
backup                +63 B  (+0.0%)      +26 B  (+0.0%)

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 (~25 bytes added 📈 [gzipped])

name                          parsed_size           gzip_size
async-load-design-playground        +63 B  (+0.0%)      +25 B  (+0.0%)
async-load-design                   +63 B  (+0.0%)      +25 B  (+0.0%)

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

olaseni

Looks almost ready. There's a comment on absolute positioning that needs to be responded to.

client/my-sites/email/email-providers-stacked-comparison/email-provider-stacked-card/style.scss

olaseni

Look good at this stage.

… and prices

…roved visuals

a8ci18n · 2021-12-06T13:16:29Z

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

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

a8ci18n · 2021-12-17T13:28:48Z

Translation for this Pull Request has now been finished.

AllTerrainDeveloper requested a review from a team November 30, 2021 09:04

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

a8ci18n reviewed Nov 30, 2021

View reviewed changes

client/my-sites/email/email-providers-stacked-comparison/email-provider-stacked-card/index.tsx Outdated Show resolved Hide resolved

a8ci18n reviewed Nov 30, 2021

View reviewed changes

daledupreez reviewed Nov 30, 2021

View reviewed changes

olaseni suggested changes Dec 1, 2021

View reviewed changes

client/my-sites/email/email-providers-stacked-comparison/email-provider-stacked-card/style.scss Outdated Show resolved Hide resolved

AllTerrainDeveloper self-assigned this Dec 1, 2021

AllTerrainDeveloper requested a review from olaseni December 1, 2021 18:28

AllTerrainDeveloper mentioned this pull request Dec 2, 2021

Add/new email provider comparison component #58400

Merged

olaseni approved these changes Dec 4, 2021

View reviewed changes

AllTerrainDeveloper added 7 commits December 6, 2021 13:21

Emails: Initial commit to start building the PR

3245284

Emails: Initial code for new provider card and features

eed7a61

Emails: Email providers comparison improvements in header, promo card…

583b564

… and prices

Emails: Connected with cart key and shopping cart. Needs cleanup, imp…

2a61b92

…roved visuals

Emails: PR suggestions

e0921b0

Emails: Fix regression issue in the original emails comparison page.

bc423c8

Emails: Use flex container on Email Features

7c27bbe

AllTerrainDeveloper force-pushed the add/new-email-comparison-card-props branch from 9a2ee3f to 7c27bbe Compare December 6, 2021 12:22

AllTerrainDeveloper merged commit 0ab6ae2 into trunk Dec 6, 2021

AllTerrainDeveloper deleted the add/new-email-comparison-card-props branch December 6, 2021 12:50

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 6, 2021

nelsonec87 pushed a commit that referenced this pull request Dec 9, 2021

Add/new email comparison card props (#58632)

24c5dcd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/new email comparison card props #58632

Add/new email comparison card props #58632

AllTerrainDeveloper commented Nov 30, 2021 •

edited

Loading

github-actions bot commented Nov 30, 2021 •

edited by matticbot

Loading

a8ci18n Nov 30, 2021

daledupreez left a comment

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez Nov 30, 2021

AllTerrainDeveloper Nov 30, 2021

daledupreez Nov 30, 2021

daledupreez left a comment

matticbot commented Nov 30, 2021 •

edited

Loading

olaseni left a comment

olaseni left a comment

a8ci18n commented Dec 6, 2021

a8ci18n commented Dec 17, 2021

	import { FunctionComponent, ReactElement } from 'react';
	import { FunctionComponent } from 'react';
	import type { ReactElement } from 'react';

	import { ProviderCard } from 'calypso/my-sites/email/email-providers-stacked-comparison';
	import type { ProviderCard } from 'calypso/my-sites/email/email-providers-stacked-comparison';

		formFields: <form> Placeholder </form>,
		formattedPrice: '3.5$',

Add/new email comparison card props #58632

Add/new email comparison card props #58632

Conversation

AllTerrainDeveloper commented Nov 30, 2021 • edited Loading

Changes proposed in this Pull Request

Work in progress

Testing instructions

github-actions bot commented Nov 30, 2021 • edited by matticbot Loading

Choose a reason for hiding this comment

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

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

daledupreez left a comment

Choose a reason for hiding this comment

matticbot commented Nov 30, 2021 • edited Loading

olaseni left a comment

Choose a reason for hiding this comment

olaseni left a comment

Choose a reason for hiding this comment

a8ci18n commented Dec 6, 2021

a8ci18n commented Dec 17, 2021

AllTerrainDeveloper commented Nov 30, 2021 •

edited

Loading

github-actions bot commented Nov 30, 2021 •

edited by matticbot

Loading

matticbot commented Nov 30, 2021 •

edited

Loading