Emails: Initial code to create new comparison component #59402

AllTerrainDeveloper · 2021-12-20T11:47:52Z

Changes proposed in this Pull Request

Initial code for a new comparison component in Emails Comparison page.

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
Click then in the "see how they compare" in the top of the screen

Related to {1200182182542585-as-1201334995813848}

github-actions · 2021-12-20T11:48:07Z

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

a8ci18n · 2021-12-20T11:52:12Z

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

+const ProfessionalEmailComparisonObject: ProviderComparison = {
+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts.'),
+	storage: translate( '30 GB storage.' ),


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 5 times:
translate( '30 GB of Storage' ) ES Score: 12

a8ci18n · 2021-12-20T11:52:13Z

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

+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts.'),
+	storage: translate( '30 GB storage.' ),
+	importing: translate( 'One-click import of existing email 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( 'One-click import of existing emails and contacts' ) ES Score: 9

a8ci18n · 2021-12-20T11:52:14Z

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

+	tools: translate( 'Integrated email management, Inbox, calendar and contacts.'),
+	storage: translate( '30 GB storage.' ),
+	importing: translate( 'One-click import of existing email and contacts.'),
+	support: translate( '24/7 support via email.')


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 18 times:
translate( '24/7 support via email' ) ES Score: 8

matticbot · 2021-12-20T11:57:32Z

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

App Entrypoints (~31 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                   +76 B  (+0.0%)      +15 B  (+0.0%)
entry-domains-landing        +76 B  (+0.0%)      +16 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~322 bytes added 📈 [gzipped])

name   parsed_size           gzip_size
email      +1223 B  (+0.2%)     +322 B  (+0.2%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

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.

a8ci18n · 2021-12-20T15:32:52Z

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

+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts.' ),
+	storage: translate( '30 GB storage.' ),
+	importing: translate( 'One-click import of existing email 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( 'One-click import of existing emails and contacts' ) ES Score: 9

a8ci18n · 2021-12-20T15:32:53Z

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

+	tools: translate( 'Integrated email management, Inbox, calendar and contacts.' ),
+	storage: translate( '30 GB storage.' ),
+	importing: translate( 'One-click import of existing email and contacts.' ),
+	support: translate( '24/7 support via email.' ),


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 18 times:
translate( '24/7 support via email' ) ES Score: 8

a8ci18n · 2021-12-20T15:46:42Z

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

+const ProfessionalEmailComparisonObject: ProviderComparison = {
+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts' ),
+	storage: translate( '30 GB storage' ),


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 5 times:
translate( '30 GB of Storage' ) ES Score: 12

a8ci18n · 2021-12-20T15:46:43Z

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

+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts' ),
+	storage: translate( '30 GB storage' ),
+	importing: translate( 'One-click import of existing email 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( 'One-click import of existing emails and contacts' ) ES Score: 9

olaseni

There are a couple of suggestions here. While your illustration alludes to it, the instructions are not super clear that one actually has to click on the see how they compare button.

olaseni · 2021-12-22T02:38:10Z

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

+	comparisonObject: ProviderComparison[];
+};
+
+export const InDepthComparison: FunctionComponent< InDepthComparison > = () => {


I feel like we can take the opportunity to name this component better, so it is easier to read. Using the same variable name as the type can be confusing at a glance.

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

olaseni · 2021-12-22T03:28:38Z

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

+const ProfessionalEmailComparisonObject: ProviderComparison = {
+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts' ),
+	storage: translate( '30 GB storage' ),


We have used 30GB storage in the past. Is this a deliberate change?

No, I just followed mock ups, I will reuse 30GB storage then.

olaseni · 2021-12-22T03:34:40Z

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

+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts' ),
+	storage: translate( '30 GB storage' ),
+	importing: translate( 'One-click import of existing email and contacts' ),


Did you mean to use emails here?

olaseni · 2021-12-22T03:36:21Z

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

-			/>
+			{ inDepthComparison && (
+				<InDepthComparison
+					comparisonObject={ [


This should be plural if it's a list of things/objects i.e. comparisonObjects

olaseni · 2021-12-22T03:39:37Z

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

+
+const ProfessionalEmailComparisonObject: ProviderComparison = {
+	header: <h1> Professional Email </h1>,
+	tools: translate( 'Integrated email management, Inbox, calendar and contacts' ),


Would it be better to see these texts as a list of features and pass an array instead? It might be easier to wrangle and display then.

I think this way is safer in the sense of providing what we need to provide for each case, the component will just read that object and place everything in place and in the right order.

Let's leave it like this and see if it fits good with what I've designed in the next PR 🙂

stephanethomas · 2021-12-22T07:52:22Z

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

+				<Button
+					borderless
+					className="email-providers-stacked-comparison__how-they-compare-link"
+					onClick={ () => setInDepthComparison( ! inDepthComparison ) }


Please let's redirect the user to a new page. The Email Comparison page and the In Depth Comparison page should have their own url, and should be responsible for loading any data they require. Let's have a clear separation of responsibilities (such as stylesheets).

I'll do it for the next PR if that's fine?

a8ci18n · 2021-12-22T10:09:37Z

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

+const GoogleWorkspaceComparisonObject: ProviderComparison = {
+	header: <h1> Google </h1>,
+	tools: translate( 'Gmail, Calendar, Meet, Chat, Drive, Docs, Sheets, Sliders and more' ),
+	storage: translate( '30 GB storage' ),


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 5 times:
translate( '30 GB of Storage' ) ES Score: 12

client/my-sites/email/controller.js

stephanethomas · 2021-12-22T15:02:34Z

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

+					onClick={ () => page( emailManagementInDepthComparison( siteName, selectedDomainName ) ) }
+				>
+					{ translate( 'See how they compare.' ) }
+				</Button>


On the mockup this is just a link, why do we need this <Button> component? As far as I know, we can achieve the same thing with an HTML anchor, and without additional CSS styles.

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

client/my-sites/email/paths.js

client/my-sites/email/email-providers-in-depth-comparison/index.tsx

stephanethomas · 2021-12-22T15:42:02Z

client/my-sites/email/email-providers-in-depth-comparison/in-depth-comparison/index.tsx

+	storage: TranslateResult;
+	importing: TranslateResult;
+	support: TranslateResult;
+	footerBadge?: ReactElement;


We don't define that particular prop in the other component, let's remove it for now.

client/my-sites/email/email-providers-in-depth-comparison/index.tsx

client/my-sites/email/email-providers-in-depth-comparison/in-depth-comparison/index.tsx

…ison-page # Conflicts: # client/my-sites/email/email-providers-stacked-comparison/style.scss

stephanethomas

I've tested it, and it works. I had a few comments about naming but nothing serious so feel free to merge this pull request once they have been addressed.

client/my-sites/email/email-providers-in-depth-comparison/comparison-table/index.tsx

client/my-sites/email/email-providers-in-depth-comparison/data.ts

client/my-sites/email/email-providers-in-depth-comparison/comparison-table/index.tsx

client/my-sites/email/email-providers-in-depth-comparison/data.ts

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

a8ci18n · 2021-12-23T15:16:30Z

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

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

a8ci18n · 2022-01-14T10:28:44Z

Translation for this Pull Request has now been finished.

Emails: Initial code/draft to create new comparison component

a77455a

AllTerrainDeveloper self-assigned this Dec 20, 2021

a8ci18n reviewed Dec 20, 2021

View reviewed changes

AllTerrainDeveloper added 2 commits December 20, 2021 16:05

Emails: Linting

009e331

Emails: Add styling

006f05d

a8ci18n reviewed Dec 20, 2021

View reviewed changes

AllTerrainDeveloper added 2 commits December 20, 2021 16:36

Emails: Translation suggestions + linting

a18a5c7

Emails: Lint

823d059

a8ci18n reviewed Dec 20, 2021

View reviewed changes

Emails: Linting issues (my linter was not working)

b2f832c

olaseni suggested changes Dec 22, 2021

View reviewed changes

stephanethomas reviewed Dec 22, 2021

View reviewed changes

Emails: PR suggestions

2aced93

a8ci18n reviewed Dec 22, 2021

View reviewed changes

Emails: Linting

da19e84

AllTerrainDeveloper marked this pull request as ready for review December 22, 2021 10:28

AllTerrainDeveloper requested a review from a team as a code owner December 22, 2021 10:28

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

Emails: i18n suggestion

fa7e97c

AllTerrainDeveloper changed the title ~~Emails: Initial code/draft to create new comparison component~~ Emails: Initial codeto create new comparison component Dec 22, 2021

AllTerrainDeveloper requested a review from olaseni December 22, 2021 11:03

Emails: Move component to a new URL

4c64619

stephanethomas reviewed Dec 22, 2021

View reviewed changes

Emails: Move code

4de3ddc

AllTerrainDeveloper added 2 commits December 22, 2021 18:05

Emails: Linting

fb1407d

Merge branch 'trunk' into add/in-depth-comparison-in-new-email-compar…

2b72d19

…ison-page # Conflicts: # client/my-sites/email/email-providers-stacked-comparison/style.scss

stephanethomas approved these changes Dec 23, 2021

View reviewed changes

Emails: Stéphane suggestions

07e4d9d

AllTerrainDeveloper changed the title ~~Emails: Initial codeto create new comparison component~~ Emails: Initial code to create new comparison component Dec 23, 2021

AllTerrainDeveloper merged commit 511b132 into trunk Dec 23, 2021

AllTerrainDeveloper deleted the add/in-depth-comparison-in-new-email-comparison-page branch December 23, 2021 14:20

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 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emails: Initial code to create new comparison component #59402

Emails: Initial code to create new comparison component #59402

AllTerrainDeveloper commented Dec 20, 2021 •

edited

Loading

github-actions bot commented Dec 20, 2021 •

edited by matticbot

Loading

a8ci18n Dec 20, 2021

a8ci18n Dec 20, 2021

a8ci18n Dec 20, 2021

matticbot commented Dec 20, 2021 •

edited

Loading

a8ci18n Dec 20, 2021

a8ci18n Dec 20, 2021

a8ci18n Dec 20, 2021

a8ci18n Dec 20, 2021

olaseni left a comment

olaseni Dec 22, 2021

olaseni Dec 22, 2021

AllTerrainDeveloper Dec 22, 2021

olaseni Dec 22, 2021

AllTerrainDeveloper Dec 22, 2021

olaseni Dec 22, 2021

olaseni Dec 22, 2021

AllTerrainDeveloper Dec 22, 2021

AllTerrainDeveloper Dec 22, 2021

stephanethomas Dec 22, 2021 •

edited

Loading

AllTerrainDeveloper Dec 22, 2021

a8ci18n Dec 22, 2021

stephanethomas Dec 22, 2021

stephanethomas Dec 22, 2021

stephanethomas left a comment

a8ci18n commented Dec 23, 2021

a8ci18n commented Jan 14, 2022

Emails: Initial code to create new comparison component #59402

Emails: Initial code to create new comparison component #59402

Conversation

AllTerrainDeveloper commented Dec 20, 2021 • edited Loading

Changes proposed in this Pull Request

Testing instructions

github-actions bot commented Dec 20, 2021 • edited by matticbot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Dec 20, 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

olaseni 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

stephanethomas Dec 22, 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

Choose a reason for hiding this comment

stephanethomas left a comment

Choose a reason for hiding this comment

a8ci18n commented Dec 23, 2021

a8ci18n commented Jan 14, 2022

AllTerrainDeveloper commented Dec 20, 2021 •

edited

Loading

github-actions bot commented Dec 20, 2021 •

edited by matticbot

Loading

matticbot commented Dec 20, 2021 •

edited

Loading

stephanethomas Dec 22, 2021 •

edited

Loading