Mobile web: Improve the way we show email address in site setup #58515

spraveenitpro · 2021-11-25T14:46:01Z

Before:

In the site setup checklist on the mobile layout under My Home we do not see the entered email address under the Confirm your email address step:

Direct Link

After:

Direct Link

We can here see the email address which is viewable without the user needing to click on the Confirm your email address step.

Testing instructions

Create a new WordPress.com site with a new account
Goto https://wordpress.com/home/:site on a mobile layout to see the site setup checklist
We should now see the user email under the Confirm your Email address step without it being collapsed

Related to #58360

@tyxla @enejb

annchichi · 2021-11-26T08:41:26Z

Heya there 👋 @enejb and @spraveenitpro ,

Thanks for the implementation. 😸

I was not able to see the email address showing in step 2 (Goto https://wordpress.com/home/:site on a mobile layout to see the site setup checklist) you mentioned.
I signed up as a new user in an incognito mode and went through the free domain, free site, and design theme. In the homepage, I was not able to see that as well.

Please let me know what steps I've missed here. :)

Based on the video you shared when the item is expanded,

the copy should be: "We have sent an email to this address to verify your account. Not in inbox or spam folder? Tap the Resend button!"
the email address as a subtitle will be hidden. And it will show after the copy.

I have attached the screenshot with two arrows. Hope this helps! 🙂

tyxla

Thanks for spinning up a PR on this one, @spraveenitpro!

On top of the review by @annchichi, I've added some feedback and questions.

tyxla · 2021-11-26T13:30:25Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

@@ -79,7 +79,18 @@ export const getTask = (
 		case CHECKLIST_KNOWN_TASKS.EMAIL_VERIFIED:
 			taskData = {
 				timing: 1,
-				title: translate( 'Confirm your email address' ),
+				title: translate(
+					'Confirm your email address {{emailWrapper}} %(email)s {{/emailWrapper}} ',


We seem to have a few extra leading and trailing spaces here and there that could be removed:

Suggested change

'Confirm your email address {{emailWrapper}} %(email)s {{/emailWrapper}} ',

'Confirm your email address {{emailWrapper}}%(email)s{{/emailWrapper}}',

tyxla · 2021-11-26T13:31:57Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

@@ -79,7 +79,18 @@ export const getTask = (
 		case CHECKLIST_KNOWN_TASKS.EMAIL_VERIFIED:
 			taskData = {
 				timing: 1,
-				title: translate( 'Confirm your email address' ),
+				title: translate(
+					'Confirm your email address {{emailWrapper}} %(email)s {{/emailWrapper}} ',


If we put the email inside the emailWrapper component, we could have a stronger string, something like 'Confirm your email address {{emailAddress}}. It would also help make translating the string a bit easier and less error prone.

Good point, also revising https://wpcalypso.wordpress.com/devdocs/packages/i18n-calypso/README.md to really understand this.

tyxla · 2021-11-26T13:32:16Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

+							email: userEmail,
+						},
+						components: {
+							br: <br />,


Is the br component necessary?

I have refactored and removed it.

tyxla · 2021-11-26T13:33:57Z

client/my-sites/customer-home/cards/tasks/site-setup-list/index.jsx

@@ -251,7 +251,7 @@ const SiteSetupList = ( {
 				<CardHeading>{ translate( 'Site setup' ) }</CardHeading>
 				<ul className="site-setup-list__list">
 					{ tasks.map( ( task ) => {
-						const enhancedTask = getTask( task );
+						const enhancedTask = getTask( task, { userEmail } );


Could you please elaborate on this change?

Here I am destructuring and including userEmail as a parameter while calling the getTask function. Now, within the getTask function, under the conditional check of task.id, if it is CHECKLIST_KNOWN_TASKS.EMAIL_VERIFIED then we use/display the user email in the Title and the description:

tyxla · 2021-11-26T13:34:24Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

+						},
+						components: {
+							br: <br />,
+							emailWrapper: <span className="site-setup-list__emailstyle" />,


Classname naming nit: style in a classname is redundant.

Sure, now it is site-setup-list__email

spraveenitpro · 2021-12-01T05:40:57Z

Testing Instructions:

Created a new account on WordPress.com
Signed in to the newly created site at http://calypso.localhost:3000/home/:Site , in my case this site
On the home page, we see the below during the desktop and mobile layout testing:

spraveenitpro · 2021-12-01T05:45:49Z

Hi @annchichi

Great points, I have just made the copy changes you requested, I have also listed the exact testing steps above, You will have to switch to the branch add/show-email-address-inline-in-site-setup, any other questions then let me know 🙌🏽

tyxla · 2021-12-01T13:47:35Z

client/my-sites/customer-home/cards/tasks/site-setup-list/style.scss

+			}
+		}
+
+		.site-setup-list__emailbold {


What if we decide that the email will be with a larger font size instead of bold? Or make it with red color? Then we'll have to change the class name to reflect that.

That's an example of why we don't name our classes based on the positioning or visual appearance. You can read more about that here:

https://github.com/Automattic/wp-calypso/blob/trunk/docs/coding-guidelines/css.md#classes

I have changed the class names to .site-setup-list__nav-item-email and .site-setup-list__task-description-email so that it shows the relationship to the parent component as per documentation. Let me know if that is alright

Those sound good to me, thanks 👍

tyxla · 2021-12-01T13:55:05Z

client/my-sites/customer-home/cards/tasks/site-setup-list/style.scss

+		}
+
+		.site-setup-list__emailbold {
+			font-weight: bolder;


I haven't seen bolder used in a while 😅

I'd recommend using a numeric value. bolder can be relative to the parent, and using a non-relative value makes it easier to reason about. Also, we mostly use numeric values all throughout Calypso.

That is correct, Just looked around.

tyxla · 2021-12-01T13:56:42Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

 				description: translate(
-					'Please click the link in the email we sent to %(email)s. ' +
-						'Typo in your email address? {{changeButton}}Change it here{{/changeButton}}.',
+					'We have sent an email to this address to verify your account. Not in inbox or spam folder? Tap the Resend button!' +


Doesn't the button say "Resend email" to be precise?

You are right, I used the exact copy provided, will change.

tyxla · 2021-12-01T13:57:50Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

-					'Please click the link in the email we sent to %(email)s. ' +
-						'Typo in your email address? {{changeButton}}Change it here{{/changeButton}}.',
+					'We have sent an email to this address to verify your account. Not in inbox or spam folder? Tap the Resend button!' +
+						'{{emailWrapperBold}} %(email)s {{/emailWrapperBold}} {{changeButton}} Change {{/changeButton}}',


Let's keep an eye on those extra leading and trailing spaces inside the components. They're in a translatable string and it's easy for the translator to miss them if we're relying on them for some reason. See how we didn't have spaces there in the previous version of the string.

tyxla · 2021-12-01T13:59:24Z

client/my-sites/customer-home/cards/tasks/site-setup-list/style.scss

@@ -65,6 +65,20 @@
 				border-bottom: none;
 			}
 		}
+
+		.site-setup-list__email {


Can we try to think of a better classname that serves the purpose of this? Seems like it's the class name that's in the title.

tyxla · 2021-12-01T14:01:10Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

@@ -79,17 +79,21 @@ export const getTask = (
 		case CHECKLIST_KNOWN_TASKS.EMAIL_VERIFIED:
 			taskData = {
 				timing: 1,
-				title: translate( 'Confirm your email address' ),
+				title: translate( 'Confirm your email address {{emailAddress /}}', {


🤔 What will happen if the translator decides that the place of the emailAddress component is in the beginning of the string? That may be true for some languages. Should it be positioned outside of the translatable string?

That will lead to the email displaying above the actual title Confirm your email address which is not what we want,

Should it be positioned outside of the translatable string?, it should still be part of the title key in the taskData object, but moving it out like title: translate( 'Confirm your email address' ) + userEmail, will not help as we then can not target the email with css, so how/what would you suggest? @tyxla

I'm not sure I understand what you mean - by moving it outside of the translate() call to the end of the expression it still remains part of the title property? So we should be able to style it? Or am I misunderstanding you?

in the latest commit I am displaying the email outside of the translation, hope it is acceptable.

Yes, I think it looks great now!

tyxla

I'm happy with the code and this looks great to me!

There's just one minor change I'd recommend - making the email non-bold on desktop. I believe this is going to be ready to ship afterwards!

I'd also like to ask @enejb for his feedback from a mobile web perspective.

Thanks @spraveenitpro!

tyxla · 2021-12-08T12:23:13Z

client/my-sites/customer-home/cards/tasks/site-setup-list/style.scss

+
+		.site-setup-list__nav-item-email {
+			color: var( --studio-gray-50 );
+			display: block;


I think on desktop it would be better if the email is not bold:

This PR:

My suggestion:

enejb · 2021-12-09T07:43:39Z

client/my-sites/customer-home/cards/tasks/site-setup-list/style.scss

@@ -65,6 +65,24 @@
 				border-bottom: none;
 			}
 		}
+
+		.site-setup-list__nav-item-email {
+			color: var( --studio-gray-50 );


This colour should be --studio-gray-40 as per figma.

enejb · 2021-12-09T07:54:46Z

Nice work @spraveenitpro

I notice that the circle and the checkmarks are not center aligned. Can we make them top aligned so that they don't move around when you expand the accordion? Just as the screenshot at the very top indicates?

Also when the accordion is expended the email address shouldn't be bold.

Thanks for all your work on this. We are super close!

spraveenitpro · 2021-12-13T19:34:58Z

I notice that the circle and the checkmarks are not center aligned. Can we make them top aligned so that they don't move around when you expand the accordion? Just as the screenshot at the very top indicates?

I see that, you are right @enejb I am trying to target .site-setup-list .nav-item__status to align it to top [vertical align] but something is inhibiting, Let me dig into this.

Also when the accordion is expended the email address shouldn't be bold.

I think I got it like this:

Direct Link

tyxla · 2021-12-14T10:18:21Z

client/my-sites/customer-home/cards/tasks/site-setup-list/get-task.js

-					'Please click the link in the email we sent to %(email)s. ' +
-						'Typo in your email address? {{changeButton}}Change it here{{/changeButton}}.',
+					'We have sent an email to this address to verify your account. Not in inbox or spam folder? Tap the Resend email button! ' +
+						'{{emailWrapper}}%(email)s{{/emailWrapper}} {{changeButton}}Change{{/changeButton}}',


Noting that as the email and change button parts are at the end of the string, they're making it needlessly more complex for the translators. I'd suggest moving them out of the string to simplify the localization for translators.

Thanks @tyxla I have removed the components from appearing within the translation function, going forward I will remember not to do this. Great to be back from holiday!

enejb · 2021-12-23T22:00:39Z

Thanks for the PR @spraveenitpro We are so close.

The only thing that I see is that the email now shows up in the H2 on desktop. So you end up with something like this. When the email is extra long. Also notice that the address is right next to the users's email.

There should be a space there.

See

I think we should have this instead

spraveenitpro · 2022-01-07T23:57:30Z

The only thing that I see is that the email now shows up in the H2 on desktop. So you end up with something like this. When the email is extra long. Also notice that the address is right next to the users's email.

HI @enejb , Happy to be back from holiday, please check it out and let me know if any other changes needed.

enejb · 2022-01-08T00:23:02Z

Thanks for the fixes @spraveenitpro .
The issue that I have reported is fixed 🥳

But in my testing things don't work as expected anymore.
The email is now only showing up when the "Confirm your email" tab is.
I would expect the email to always be visible even in the tab even if the tab is not selected.

See:

Also this PR seems like its needs a rebase there is a conflct in the client/my-sites/customer-home/cards/tasks/site-setup-list/index.jsx file.

enejb · 2022-01-10T22:46:36Z

Thanks for the fixes @spraveenitpro Do you mind also rebasing this PR. So that we can merge it.

This is what I am currently seeing. Thanks!

…e accordion

…start

.github/workflows/codesee-arch-diagram.yml

enejb

Thanks for all the changes @spraveenitpro!

They look good and the code works as expected

a8ci18n · 2022-01-11T21:16:51Z

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

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

a8ci18n · 2022-01-24T05:28:59Z

Translation for this Pull Request has now been finished.

spraveenitpro added the [Type] Feature Request Feature requests label Nov 25, 2021

spraveenitpro requested a review from tyxla November 25, 2021 14:46

spraveenitpro requested a review from a team as a code owner November 25, 2021 14:46

github-actions bot added the OSS Citizen label Nov 25, 2021

spraveenitpro removed the request for review from a team November 25, 2021 22:07

tyxla reviewed Nov 26, 2021

View reviewed changes

tyxla reviewed Dec 1, 2021

View reviewed changes

tyxla reviewed Dec 8, 2021

View reviewed changes

enejb reviewed Dec 9, 2021

View reviewed changes

tyxla reviewed Dec 14, 2021

View reviewed changes

codesee-architecture-diagrams[bot] and others added 9 commits January 10, 2022 22:30

Add CodeSee architecture diagram workflow to repository

0fd6e1d

Adding user email address to the CheckList title

983550e

Styling the inline email text on the site setup list in mobile layout

9ae9312

Hiding the email address on mobile layout and including the copy

7662bb0

Updating className based on coding standards/refactor for translation()

a531122

Applying the correct color/weight to Email address that appears in th…

bcffd3c

…e accordion

Changing flex alignment in the Accordion button from center to align-…

cd1fbb7

…start

Hide email from showing up in the h2 title while on desktop

6b75f48

Fix Email to show up in mobile layout

8c1bcfe

tyxla reviewed Jan 11, 2022

View reviewed changes

.github/workflows/codesee-arch-diagram.yml Outdated Show resolved Hide resolved

Removing workflow file added by codesee a repo visualizing tool

d8b7ea9

enejb approved these changes Jan 11, 2022

View reviewed changes

enejb merged commit 377fec7 into Automattic:trunk Jan 11, 2022

	'Confirm your email address {{emailWrapper}} %(email)s {{/emailWrapper}} ',
	'Confirm your email address {{emailWrapper}}%(email)s{{/emailWrapper}}',

Mobile web: Improve the way we show email address in site setup #58515

Mobile web: Improve the way we show email address in site setup #58515

Conversation

spraveenitpro commented Nov 25, 2021 • edited Loading

Before:

After:

Testing instructions

annchichi commented Nov 26, 2021 • edited Loading

tyxla 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

spraveenitpro Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spraveenitpro commented Dec 1, 2021

Testing Instructions:

Full Desktop Layout:

Mobile Layout Collapsed:

Mobile Expanded:

spraveenitpro commented Dec 1, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyxla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enejb commented Dec 9, 2021

spraveenitpro commented Dec 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enejb commented Dec 23, 2021 • edited Loading

spraveenitpro commented Jan 7, 2022 • edited Loading

enejb commented Jan 8, 2022

enejb commented Jan 10, 2022

enejb left a comment

Choose a reason for hiding this comment

a8ci18n commented Jan 11, 2022

a8ci18n commented Jan 24, 2022

spraveenitpro commented Nov 25, 2021 •

edited

Loading

annchichi commented Nov 26, 2021 •

edited

Loading

spraveenitpro Dec 1, 2021 •

edited

Loading

enejb commented Dec 23, 2021 •

edited

Loading

spraveenitpro commented Jan 7, 2022 •

edited

Loading