Improve ICANN verification flow #15880

mikeshelton1503 · 2017-07-05T19:16:14Z

Fixes #2478 #13231
Also as a bonus, added a fix for #6098

This improves the presentation of the ICANN email verification step.

Before	After

Changes:

Updated copy for ICANN verification card - removed all mention of ICANN.
^^ That includes adding specific mention of what email address the email was and will be sent to and a direct link to change it.
Replace global success notices with a status bar that communicates the action needed.

How to test:

Purchase a domain but don't verify your email address yet.
Go to Domains and click on the domain you just bought.
Affirm you see the Email verification box at the top.
Click "Send Again".
Affirm you were shown a "Email sent" message.
After 5 seconds the message should disappear. Then click "Change Email Address".
Affirm you are taken to the Edit Contact Info screen.

matticbot · 2017-07-05T19:16:25Z

Test live: https://calypso.live/?branch=update/icann-verification-flow

mikeshelton1503 · 2017-07-05T19:17:57Z

Also @ranh this could use a copy review.

michelleweber · 2017-07-05T19:20:07Z

Just checking -- do you need a copy review from @ranh, or from Editorial? (The "Needs Copy Review" label pings us.)

mikeshelton1503 · 2017-07-05T19:26:13Z

Just checking -- do you need a copy review from @ranh, or from Editorial? (The "Needs Copy Review" label pings us.)

Oh I see! I intended it for Ran but won't turn down more copy reviews but probably not necessary for two reviews :)

klimeryk · 2017-07-05T20:01:00Z

Awesome stuff, @mikeshelton1503! 💟
I'll try to finish the developer part of this before the end of the week!

klimeryk · 2017-07-05T20:02:47Z

A suggestion comes to mind - AFAIK, we shouldn't be using the global ("floating") notices for stuff like success notices. Maybe we could, for example, on success substitute the verification card with a nice, big green notice?

mikeshelton1503 · 2017-07-06T00:24:41Z

A suggestion comes to mind - AFAIK, we shouldn't be using the global ("floating") notices for stuff like success notices.

Oh, this would be news to me. Where did you hear this? Seems to me that a floating notice is the perfect type for a quick acknowledgement of an action such as success. I was out of Calypso land for much of last year so I could have missed it. cc @folletto @drw158

I'm certainly not opposed to keeping the success acknowledgment inline within the verification card but wouldn't want to use a big green notice. It is a "successful" action but not a successful completion of the task since you need to check the email so we don't want to give the impression that you're done.

ranh · 2017-07-06T06:55:31Z

I don't understand some of the changes you made to the copy. Can you explain why you changed the reason we give for the need to verify? I didn't know it was necessary to verify the contact information to make changes to the domain. Even if it is, most people will not want to make changes to their domain, so they may assume this message doesn't apply to them. The fact that the domain will stop working if not verified is probably going to be more relevant to most users.

We can cut down on the term "verification". For example instead of "look for the verification message", we can say "look for our email", and instead of "resend verification email", we can just say "resend email".

I tend to agree with @klimeryk, I don't think the global notice is a good fit here. Is there any specific reason to have one part of the UI in one area of the screen and another part of the same UI in an entirely different area?

I think we would be better off with a much more subtle indication, near the button itself.

ranh · 2017-07-06T08:18:44Z

I didn't know it was necessary to verify the contact information to make changes to the domain

I see now that this comes from #2478. And that I actually wrote the piece of copy in question 😂

However, the copy in that issue was meant specifically for the name servers screen, not the main domain screen. If users are on the name servers screen, it does make sense to assume they are trying to make changes, but on the main domain screen that assumption doesn't hold. So I think we're better off focusing on the domain getting suspended there.

folletto · 2017-07-06T10:49:53Z

AFAIK, we shouldn't be using the global ("floating") notices for stuff like success notices.

The patterns is actually exactly that: floating notices are for any quick confirmation of an action that requires text and has no other in-screen UI change going on.

So yeah, I confirm:

Seems to me that a floating notice is the perfect type for a quick acknowledgement of an action such as success.

That's the use of a global notice bubble.
Short text is great too, so good job in shortening it. Also good take in switching to the info notice as it requires a follow-up action to be completed.

Note however that if there's a way to message this inside the page UI in a different way, like changing status on some of the UI blocks on the page to say "Email sent, waiting for reply [resend]" or something, then the UI itself will communicate that and there would be no need for a bubble notification.

mikeshelton1503 · 2017-07-06T12:04:04Z

Note however that if there's a way to message this inside the page UI in a different way, like changing status on some of the UI blocks on the page to say "Email sent, waiting for reply [resend]" or something, then the UI itself will communicate that and there would be no need for a bubble notification.

Well I think this is ideal though I'm not aware of any existing patterns for this. Are there any? Anyways I'll work on keeping it within the UI, I agree it makes the most sense here.

mikeshelton1503 · 2017-07-06T12:07:33Z

However, the copy in that issue was meant specifically for the name servers screen, not the main domain screen.

Oh I see, that makes sense. I see now that is a separate step. There is also a version of the copy I edited that specifically for nameservers in the code. I updated that copy too.

Anyways I'll update the nameservers step with this copy and add new copy for the main screen unless you have suggestions.

folletto · 2017-07-06T12:37:09Z

Well I think this is ideal though I'm not aware of any existing patterns for this. Are there any?

It's not a pattern: it depends on the UI you're transforming to the informative temporary state.
I think one of the best and most explicit examples is Imports, like #10770.

But you can also see our "Manage Purchases" screen as a UI that changes to different states depending on extra-UI factors, so that's another example probably.

I hope it helps :)

klimeryk · 2017-07-07T19:03:35Z

The patterns is actually exactly that: floating notices are for any quick confirmation of an action that requires text and has no other in-screen UI change going on.

@folletto Thanks for explanation - my bad, I must have remembered wrongly some comment or misinterpreted it. I think it was #3150 (comment) - on of my first Calypso PRs... I thought that Matias meant that we should avoid global notices completely, but reading that again, he meant that about the old client/notices only 😓

folletto · 2017-07-07T19:04:33Z

he meant that about the old

Ah yes, you got caught up in the period where we were transitioning that pattern. Sorry!

mikeshelton1503 · 2017-07-08T01:43:56Z

Ok I've made some changes based on the above feedback. Instead of using a success notice bubble, I switched to an inline method using statuses. There are now two statuses: waiting and email-sent. I also changed up the copy a bit, letting the status convey the action and the intro copy explain the process.

When a user first visit domain section they will see this, which is the waiting status:

When a user clicks "Send Again" the button will go to disabled state and the button copy will change to "Sending ...".

Once the action is complete, the contents of the status bar fade out and a new status fades in. This is the email-sent status. It appears until the page is navigated away from or refreshed, then the status changes back to waiting.

I just pushed up changes to put this new structure in place but I'll need help with the functionality as described above. cc @klimeryk

I also have some responsive issues to clean up.

ranh · 2017-07-09T09:50:40Z

These notices look great, I really like how you incorporated the status indicator inside the notice.

The big button for "send again" makes it seem like it's the obvious next action - is that intentional? If we want to make sure that users will have a new email from us and not have to dig through the archives for the one we sent after purchase, we can just resend the email automatically when we show the notice, rather than add an extra step for it.

This might be nitpicking, but I still think that users may end up clicking the "send again" button and thinking they were done. The green icon, disabled button and "email sent" text can be confusing, but even without them, just the fact that users "acted" on the notice in some way may be enough to have them skip the actual verification altogether.

That's why it may be better to not have a prominent next action to do on this screen, so that it's very clear that the next action actually has to be done elsewhere.

@hewsut

Props to @hewsut for the implementation, fixed two issues (the first one was preventing it from working at all, it seems): Turns out you can't mix string and array notation in the lodash' `get`. Fixed typo in prop's name (`requesting` -> `isRequesting`).

And that's OK - so let's update the propTypes to reflect that.

klimeryk · 2017-07-12T12:40:07Z

I've pushed one more small fix (nothing impacting design/logic, just developer stuff) and rebased with master because in the meantime there's been a pretty important update to webpack 3 which might break some stuff (though, that's just me being overly cautious).

klimeryk · 2017-07-12T12:54:33Z

And one more small fix in case the user has a long email, but a small viewport ;)
Before:

After:

klimeryk · 2017-07-12T16:54:56Z

@hewsut No worries, I've noticed that it's not used anywhere ATM, so of course some regression might creep in. It was just funny to see that it's not actually working after all that feedback and code reviews - rule #1 of code reviews: actually run the code 😜

And, of course, kudos on Reduxifying it in the first place, I know it must have been a beast to implement.

aidvu

LGTM! Minor comments.

aidvu · 2017-07-12T22:30:33Z

client/components/data/query-whois/index.jsx

@@ -30,6 +30,6 @@ QueryWhois.propTypes = {
 };

 export default connect(
-	( state, { domain } ) => ( { requesting: isRequestingWhois( state, domain ) } ),


Ow wow. Good job catching this.

aidvu · 2017-07-12T22:55:22Z

...my-sites/domains/domain-management/components/icann-verification/icann-verification-card.jsx

+	( state, ownProps ) => ( {
+		contactDetails: getRegistrantWhois( state, ownProps.selectedDomainName ),
+	} ),
+	dispatch => bindActionCreators( { errorNotice, successNotice }, dispatch )


I don't think you need to do this anymore. If you just pass

{ errorNotice, successNotice }

it will automagically bind: https://github.com/Automattic/wp-calypso/pull/15058/files#diff-9b3dc846fe889e4ab5d78da8b44490ddR208

TIL, thanks! Also, while fixing this I remembered that successNotice is no longer used and can be removed 😀

aidvu · 2017-07-12T23:03:35Z

...my-sites/domains/domain-management/components/icann-verification/icann-verification-card.jsx

+
+		this.setState( { submitting: true } );
+
+		upgradesActions.resendIcannVerification( this.props.selectedDomainName, ( error ) => {


Would be amazing if we move this to redux as part of the refactor. Your call.

I agree, but let's not do this in this PR. The discussion has been long enough and this refactor and subsequent testing would block this for another few days and I don't want to block Mike for that long. It might be a good idea for a mob programming session, though :)

Also, `successNotice` is no longer used.

Also remove global override for `.notice__text` (added in #15880)

* Domains: remove unused .icann-verification styles The stylesheet was duplicated/migrated to `email-verification` in #19577 but never removed. * Domains: migrate .email-verification CSS styles to webpack Also remove global override for `.notice__text` (added in #15880) * Domains: shorten the import path of IcannVerificationCard component

mikeshelton1503 added [Feature Group] Emails & Domains Features related to email integrations and domain management. [Status] In Progress labels Jul 5, 2017

mikeshelton1503 self-assigned this Jul 5, 2017

mikeshelton1503 requested a review from klimeryk July 5, 2017 19:16

matticbot added the [Size] M Medium sized issue label Jul 5, 2017

mikeshelton1503 added the [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR label Jul 5, 2017

michelleweber removed the [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR label Jul 5, 2017

mikeshelton1503 mentioned this pull request Jul 6, 2017

Domain Management: Change ICANN verification message copy #2478

Closed

matticbot added [Size] L Large sized issue and removed [Size] M Medium sized issue labels Jul 8, 2017

klimeryk force-pushed the update/icann-verification-flow branch from b1f5faa to 10a984e Compare July 9, 2017 20:14

matticbot added [Size] XL Probably needs to be broken down into multiple smaller issues and removed [Size] L Large sized issue labels Jul 9, 2017

Igor Klimer and others added 12 commits July 12, 2017 14:38

Fix link to change the email via WHOIS update

30cb20a

Fix linter warnings for too long lines

b4e4cd2

Fixes to the WHOIS redux stuff

d04c188

Props to @hewsut for the implementation, fixed two issues (the first one was preventing it from working at all, it seems): Turns out you can't mix string and array notation in the lodash' `get`. Fixed typo in prop's name (`requesting` -> `isRequesting`).

Show the correct registrant email in the notice

1e1e1da

Use busy style when sending and show a different text

9bb9acc

Hide the button and change copy when the email has been sent

f641105

De-emphasized send button and updated copy.

3a29850

Copy updates

616b657

Removing unused CSS

8a6d77e

Add timer for reverting to waiting state

7b9b172

Add CSS animation

6170735

contactDetails can be undefined when we have yet to fetch them

ac6f268

And that's OK - so let's update the propTypes to reflect that.

klimeryk force-pushed the update/icann-verification-flow branch from 5d11864 to ac6f268 Compare July 12, 2017 12:38

klimeryk requested review from matthusby, aidvu, hambai and delputnam July 12, 2017 12:40

Make sure the email does not overflow

9c6cf53

aidvu approved these changes Jul 12, 2017

View reviewed changes

No need to explicitly bind action creators

bb95d03

Also, `successNotice` is no longer used.

klimeryk merged commit a095183 into master Jul 13, 2017

klimeryk deleted the update/icann-verification-flow branch July 13, 2017 11:38

klimeryk removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 13, 2017

klimeryk mentioned this pull request Jul 13, 2017

Domains: Don't allow unverified email for domain registration and G Suite #16166

Merged

klimeryk mentioned this pull request Jul 21, 2017

Janitorial: Refactor Domain Transfer components per code standards #16382

Merged

jsnajdr added a commit that referenced this pull request Mar 26, 2019

Domains: migrate .email-verification CSS styles to webpack

511401a

Also remove global override for `.notice__text` (added in #15880)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve ICANN verification flow #15880

Improve ICANN verification flow #15880

mikeshelton1503 commented Jul 5, 2017 •

edited

Loading

matticbot commented Jul 5, 2017

mikeshelton1503 commented Jul 5, 2017

michelleweber commented Jul 5, 2017

mikeshelton1503 commented Jul 5, 2017

klimeryk commented Jul 5, 2017

klimeryk commented Jul 5, 2017

mikeshelton1503 commented Jul 6, 2017

ranh commented Jul 6, 2017

ranh commented Jul 6, 2017

folletto commented Jul 6, 2017 •

edited

Loading

mikeshelton1503 commented Jul 6, 2017

mikeshelton1503 commented Jul 6, 2017

folletto commented Jul 6, 2017

klimeryk commented Jul 7, 2017

folletto commented Jul 7, 2017

mikeshelton1503 commented Jul 8, 2017 •

edited

Loading

ranh commented Jul 9, 2017

klimeryk commented Jul 12, 2017

klimeryk commented Jul 12, 2017

klimeryk commented Jul 12, 2017 •

edited

Loading

aidvu left a comment

aidvu Jul 12, 2017

aidvu Jul 12, 2017

klimeryk Jul 13, 2017

aidvu Jul 12, 2017

klimeryk Jul 13, 2017


		this.setState( { submitting: true } );

		upgradesActions.resendIcannVerification( this.props.selectedDomainName, ( error ) => {

Improve ICANN verification flow #15880

Improve ICANN verification flow #15880

Conversation

mikeshelton1503 commented Jul 5, 2017 • edited Loading

matticbot commented Jul 5, 2017

mikeshelton1503 commented Jul 5, 2017

michelleweber commented Jul 5, 2017

mikeshelton1503 commented Jul 5, 2017

klimeryk commented Jul 5, 2017

klimeryk commented Jul 5, 2017

mikeshelton1503 commented Jul 6, 2017

ranh commented Jul 6, 2017

ranh commented Jul 6, 2017

folletto commented Jul 6, 2017 • edited Loading

mikeshelton1503 commented Jul 6, 2017

mikeshelton1503 commented Jul 6, 2017

folletto commented Jul 6, 2017

klimeryk commented Jul 7, 2017

folletto commented Jul 7, 2017

mikeshelton1503 commented Jul 8, 2017 • edited Loading

ranh commented Jul 9, 2017

klimeryk commented Jul 12, 2017

klimeryk commented Jul 12, 2017

klimeryk commented Jul 12, 2017 • edited Loading

aidvu left a comment

Choose a reason for hiding this comment

aidvu Jul 12, 2017

Choose a reason for hiding this comment

aidvu Jul 12, 2017

Choose a reason for hiding this comment

klimeryk Jul 13, 2017

Choose a reason for hiding this comment

aidvu Jul 12, 2017

Choose a reason for hiding this comment

klimeryk Jul 13, 2017

Choose a reason for hiding this comment

mikeshelton1503 commented Jul 5, 2017 •

edited

Loading

folletto commented Jul 6, 2017 •

edited

Loading

mikeshelton1503 commented Jul 8, 2017 •

edited

Loading

klimeryk commented Jul 12, 2017 •

edited

Loading