Home: Move notices to separate components #40299

mmtr · 2020-03-19T17:06:52Z

Changes proposed in this Pull Request

Moves all Home notices into their own components.
It also consolidates all of them into the same celebrate format we started to use as a pattern for this kind of notices.
The introductory welcome banner added for users that have never been redirected there (Customer Home: Add a welcome banner. #39557) has been removed here in order to clean up a bit further the codebase. Likely, most of active users have already seen that banner so it should be safe to remove it

Before	After

Testing instructions

Create a site.
Make sure Home displays a congratulatory message for the new site created.
Launch the site.
Make sure Home displays a congratulatory message for the launched site.
Complete the checklist.
Make sure Home displays a congratulatory message for the site setup being complete.
Reload the page and make sure the checklist complete notice is gone.
Migrate a site to WP.com (switch to a Simple site, go to Tools > Import, select WordPress, enter the URL of a Jetpack site and select Everything).
Make sure Home displays a congratulatory message for the new imported site.

matticbot · 2020-03-19T17:06:54Z

Test live: https://calypso.live/?branch=update/home-notices

gwwar · 2020-03-19T20:59:36Z

Reserving this comment for testing

Pretty minor, but after creating a new user + site, there appears to be a lack of whitespace between the page title and banner. Refreshing seems to clear this up.

Initial load:

After Refresh:

✅Verified that we now see this ^^ banner over this vv in stage

✅Launch Site Banner looks good, though I am getting that same whitespace issue. I wonder if this is a difference between page.js navigation vs a full page refresh.

:D fun testing note, we don't see the checklist complete message if we complete the edit your homepage task last, or launch your site. (Behavior doesn't need to be modified in this PR.) So I think this banner will only show up if we complete the mobile app step last.

We can leave this for an enhancement PR but it appears we have a pattern in these banners to have secondary text to guide folks on what to do next. (Eg maybe a sentence explaining the quick-links?)

The notice shown for recently migrated sited has been also moved, but couldn't test it since I didn't find the needed steps for having site that is considered migrated.

We might need to ask @Automattic/start-samus if this worked before. Forcing the value makes the banner render okay, but I don't see site meta being updated after creating a new site, then importing content. We can investigate further in another PR. I don't think this is blocking.

Current prod behavior after migration:

gwwar · 2020-03-19T21:45:04Z

client/my-sites/customer-home/notices/site-setup-complete-card/index.jsx

+ */
+import CelebrateNotice from '../celebrate-notice';
+
+const SiteSetupCompleteCard = ( { displayChecklist } ) => {


Minor naming note for these. I think we'd probably call these banners or notices vs cards.

Agree names are confusing, but decided to stick with a cards terminology for consistency reasons since we're aiming for an API that returns a layout of cards. Happy to rename them if rest of folks agree on a better name 🙂

gwwar

Excellent refactor @mmtr! I left a few minor notes but nothing blocking!

mattsherman · 2020-03-20T01:11:42Z

The notice shown for recently migrated sited has been also moved, but couldn't test it since I didn't find the needed steps for having site that is considered migrated.

We might need to ask @Automattic/start-samus if this worked before. Forcing the value makes the banner render okay, but I don't see site meta being updated after creating a new site, then importing content. We can investigate further in another PR. I don't think this is blocking.

@gwwar Your screenshot shows a WordPress import ("content only"), not a WordPress migration ("import everything"). So, no banner should be shown on the Home page afterwards.

andfinally · 2020-03-20T08:51:02Z

Tested with a WordPress migration, and the migration banner is appearing OK.

mmtr · 2020-03-20T10:30:00Z

Pretty minor, but after creating a new user + site, there appears to be a lack of whitespace between the page title and banner. Refreshing seems to clear this up.

Yeah, I noted this too while testing locally but wasn't sure if it was a caching issue. Will investigate before landing.

mmtr · 2020-03-20T10:50:09Z

We can leave this for an enhancement PR but it appears we have a pattern in these banners to have secondary text to guide folks on what to do next. (Eg maybe a sentence explaining the quick-links?)

Pushed a commit adding a secondary text explaining the quick links if the checklist is completed:

Before	After

Is the copy clear? cc @obi2020 for feedback.

matticbot · 2020-03-20T10:58:48Z

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

Sections (~3524 bytes removed 📉 [gzipped])

name  parsed_size           gzip_size
home     -13805 B  (-5.2%)    -3524 B  (-4.9%)

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.

mmtr · 2020-03-20T11:09:20Z

Pretty minor, but after creating a new user + site, there appears to be a lack of whitespace between the page title and banner. Refreshing seems to clear this up.

Yeah, I noted this too while testing locally but wasn't sure if it was a caching issue. Will investigate before landing.

Turns out that CSS chunks are loaded in different order on page load. Without reloading, there was a CSS file overriding the desired margin (0 0 16px):

But on page load, that CSS file was loaded with lower priority, leaving the header with the right margin:

I handled this on 087518a by removing the sometimes overriding margin given it is actually not needed.

mmtr · 2020-03-20T12:44:47Z

Going to land this. We can open a follow-up PR if we want to change the "quick-links explanation" copy.

danhauk · 2020-03-20T13:13:38Z

Looks good @mmtr. I like that the styles are all consolidated to create a standardized pattern here. I'd like to follow up with maybe some custom illustrations so we don't overuse the fireworks.

One question (also not blocking for this PR): thoughts on making these dismissible with the x on the right side? Since these are not essential it would be nice if people could get it out of the way if they want.

mmtr · 2020-03-20T14:14:05Z

thoughts on making these dismissible with the x on the right side? Since these are not essential it would be nice if people could get it out of the way if they want.

Yup, agree. Not sure how trivial would be that work (have a feeling we cannot reuse the existing dismissible card component straight away) but it definitely worths to take a look at it.

mmtr added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial [Feature] My Home The main dashboard for managing a WordPress.com site. labels Mar 19, 2020

mmtr requested a review from a team March 19, 2020 17:06

mmtr self-assigned this Mar 19, 2020

gwwar reviewed Mar 19, 2020

View reviewed changes

gwwar approved these changes Mar 19, 2020

View reviewed changes

Home: Move notices to separate components

566e061

mmtr force-pushed the update/home-notices branch from dc3c00b to 566e061 Compare March 20, 2020 10:28

Add secondary text explaining quick links if checklist is completed

34d565b

Remove unnecessary CSS margin

087518a

mmtr added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 20, 2020

mmtr merged commit 1b0d9b8 into master Mar 20, 2020

mmtr deleted the update/home-notices branch March 20, 2020 14:15

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 20, 2020

mmtr mentioned this pull request Mar 23, 2020

Home: Add dismissal support to celebratory notices #40365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Home: Move notices to separate components #40299

Home: Move notices to separate components #40299

mmtr commented Mar 19, 2020 •

edited

Loading

matticbot commented Mar 19, 2020

gwwar commented Mar 19, 2020 •

edited

Loading

gwwar Mar 19, 2020

mmtr Mar 20, 2020

gwwar left a comment •

edited

Loading

mattsherman commented Mar 20, 2020

andfinally commented Mar 20, 2020

mmtr commented Mar 20, 2020

mmtr commented Mar 20, 2020 •

edited

Loading

matticbot commented Mar 20, 2020 •

edited

Loading

mmtr commented Mar 20, 2020

mmtr commented Mar 20, 2020

danhauk commented Mar 20, 2020

mmtr commented Mar 20, 2020

Home: Move notices to separate components #40299

Home: Move notices to separate components #40299

Conversation

mmtr commented Mar 19, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Mar 19, 2020

gwwar commented Mar 19, 2020 • edited Loading

gwwar Mar 19, 2020

Choose a reason for hiding this comment

mmtr Mar 20, 2020

Choose a reason for hiding this comment

gwwar left a comment • edited Loading

Choose a reason for hiding this comment

mattsherman commented Mar 20, 2020

andfinally commented Mar 20, 2020

mmtr commented Mar 20, 2020

mmtr commented Mar 20, 2020 • edited Loading

matticbot commented Mar 20, 2020 • edited Loading

mmtr commented Mar 20, 2020

mmtr commented Mar 20, 2020

danhauk commented Mar 20, 2020

mmtr commented Mar 20, 2020

mmtr commented Mar 19, 2020 •

edited

Loading

gwwar commented Mar 19, 2020 •

edited

Loading

gwwar left a comment •

edited

Loading

mmtr commented Mar 20, 2020 •

edited

Loading

matticbot commented Mar 20, 2020 •

edited

Loading