Add welcome modal when coming from Calypso #6004
Conversation
|
@pmcpinto I've added in two tracks events in line with the events created by the existing welcome modal:
|
|
@becdetat thanks for the heads up. Just a small detail, I think that we should follow the Tracks naming convention and prefix those event names with |
Oh, sorry, the prefix is automatically added by the tracks library so I forgot to include it. The events will be created with the prefix. |
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
Code looks great! I just left one comment for a particular use case where the user visits the WooCommerce store for the first time from Calypso, causing some weird things behind the scenes, see comment below.
| welcomeModalDismissedHasResolved && | ||
| ! welcomeModalDismissed && | ||
| welcomeFromCalypsoModalDismissedResolved && | ||
| ! welcomeFromCalypsoModalDismissed; |
There was a problem hiding this comment.
Should we change the last item here to ! shouldShowWelcomeFromCalypsoModal instead of ! welcomeFromCalypsoModalDismissed.
I don't know how common the case is that the user visits Woo admin for the first time coming from calypso, but this would trigger both shouldShowWelcomeModal and shouldShowWelcomeFromCalypsoModal to be true at the same time.
Actually causing a strange effect, where the welcome modal gets quickly loaded and closed, and then showing the calypso modal, as you can see from the tracks that it generated (on initial window load).
Triggering the open track for both modal's, but then immediately closing the welcome modal.
@becdetat would it be worth in this case showing the welcome modal, but replacing the first page with the Calypso page?
There was a problem hiding this comment.
Code looks great! I just left one comment for a particular use case where the user visits the WooCommerce store for the first time from Calypso, causing some weird things behind the scenes, see comment below.
cc @pmcpinto to see if this use case might actually happen.
There was a problem hiding this comment.
@becdetat @louwie17 if he's a brand new user he shouldn't see the modal because he never interacted with the Store UI. Makes sense? Or maybe I'm missing something here.
cc @elizaan36 for additional feedback
There was a problem hiding this comment.
if he's a brand new user he shouldn't see the modal because he never interacted with the Store UI.
@pmcpinto which modal are you referring to here that shouldn't be shown? The calypso welcome modal or the original welcome modal (the one with 3 pages, mentioning the inbox, and performance stats).
I didn't test this with Calypso, but the way the logic is set up, is that a brand new user would only see the single page modal - Calypso welcome screen (new store management experience).
There was a problem hiding this comment.
the way the logic is set up, is that a brand new user would only see the single page modal - Calypso welcome screen (new store management experience)
that was certainly my intent. A new user from Calypso should only see the new Calypso welcome modal, and never the original welcome modal. This corresponds with @elizaan36 's design for the new Calypso welcome modal as well.
There was a problem hiding this comment.
Just to clarify, the Calypso welcome modal would only be relevant for the existing Calypso users, so anyone that's been a Store user or becomes a Store user during the sunset period.
If we're talking about brand-new Store users after Store sunset, then they should see the regular multi-page welcome modal upon landing on the Home screen. That may be the logic we have, but just wanted to clarify.
There was a problem hiding this comment.
@louwie17 I'm mainly talking about what Elizabeth mentioned here:
If we're talking about brand-new Store users after Store sunset, then they should see the regular multi-page welcome modal upon landing on the Home screen. That may be the logic we have, but just wanted to clarify.
If a user signs up during the sunset period, they should also see the regular welcome modal. Makes sense?
There was a problem hiding this comment.
Great, thanks for the clarification everyone. I believe everything is working as expected then.
Approving this PR.
louwie17
added
needs: author feedback
|
I've updated this with the final copy |
|
Modal looks great! One super small thing - can we remove the period |
We can, however the existing welcome modal screens all have periods in the headline so I added it for consistency. I'm happy to remove it if you want though. |
|
No worries, it's a really small thing. I may go on a period-removing rampage at some point. |
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
After clarification, everything tested well, good to 🚢 👍
Just mentioned if you could add the PR number to the readme before merging :)
readme.txt
Outdated
| - Fix: Updating (non wordpress user) customer with order data | ||
| - Dev: Add documentation for filter `woocommerce_admin_pages_list` and `wc_admin_register_page` #5844 | ||
| - Dev: Revert work done in #4857 for automated shipping after OBW is completed #5971 | ||
| - Add: Welcome modal when coming from Calypso |
There was a problem hiding this comment.
Just add the PR number here before merging.
Fixes Automattic/wp-calypso#48420
The intent of Automattic/wp-calypso#48420 is to show a welcome modal when entering WooCommerce Admin from Calypso. Automattic/wp-calypso#48420 adds
&from-calypsoto the URL to WCA, and this PR handles that URL param to display the welcome modal.Screenshots
edit Updated with final copy
Detailed test instructions:
woocommerce_task_list_welcome_modal_dismissedoption), visit/wp-admin/admin.php?page=wc-admin. You should see the standard welcome modal.&from-calypsoto the URL. You should see the Calypso welcome modal.&from-calypsofrom the URL. The standard welcome modal should not reappear.