Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My Home: Try Home Layout i1. #41000

Closed
wants to merge 13 commits into from
Closed

My Home: Try Home Layout i1. #41000

wants to merge 13 commits into from

Conversation

kwight
Copy link
Contributor

@kwight kwight commented Apr 9, 2020

This PR explores what changes we need to make for something like My Home Layout i1.

See: pbAPfg-g0-p2

Testing instructions

  • Apply D41677-code and sandbox the API.
  • Go to /home/:site with an prelaunch, launched and established site and observe the layout.

@kwight kwight requested a review from a team as a code owner April 9, 2020 22:12
@matticbot
Copy link
Contributor

@kwight kwight self-assigned this Apr 9, 2020
@kwight kwight added [Status] In Progress [Feature] My Home The main dashboard for managing a WordPress.com site. DO NOT MERGE labels Apr 9, 2020
{ React.createElement( cardComponents[ stats ], {
checklistMode: stats === 'home-primary-checklist-site-setup' ? checklistMode : null,
} ) }
<h2>{ translate( 'Learn and grow' ) }</h2>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Learn and grow with My Home' ) ES Score: 6

@matticbot
Copy link
Contributor

matticbot commented Apr 9, 2020

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

Webpack Runtime (~16 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
manifest       -180 B  (-0.1%)      +16 B  (+0.0%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

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

name        parsed_size           gzip_size
entry-main        -77 B  (-0.0%)       +2 B  (+0.0%)

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

Sections (~16957 bytes removed 📉 [gzipped])

name                parsed_size            gzip_size
home                   -70213 B  (-27.4%)   -17053 B  (-25.2%)
google-my-business      -1264 B   (-0.5%)     -918 B   (-1.2%)
email                   -1176 B   (-0.4%)     -161 B   (-0.2%)
checkout                 -212 B   (-0.0%)      -56 B   (-0.0%)
plans                     +71 B   (+0.0%)    +1118 B   (+1.0%)
marketing                 -39 B   (-0.0%)     +118 B   (+0.1%)
stats                     -34 B   (-0.0%)       -5 B   (-0.0%)

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

Async-loaded Components (~4744 bytes added 📈 [gzipped])

name                                                                              parsed_size            gzip_size
async-load-my-sites-customer-home-cards-primary-checklist-site-setup-wpcom-ch...     +16749 B  (+88.4%)    +4764 B  (+87.4%)
async-load-my-sites-sidebar                                                            -257 B   (-0.4%)      -18 B   (-0.1%)
async-load-design                                                                      +184 B   (+0.0%)       -2 B   (-0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

@kwight
Copy link
Contributor Author

kwight commented Apr 9, 2020

One of the first questions that comes up is, where do we get the section headings from (eg. "Learn and grow")? If we handle them in the client (as is done here), the translations are handled in Calypso, but we're kinda breaking our guiding idea that the client is just mindlessly rendering whatever the API passes to it (well, at least we're making assumptions in the client of what content we'll be rendering). The headings could be translated on the server and sent by API based on a key to the section, but... that also feels weird and fragile. That would need more nested view arrays, I think:

Screen Shot 2020-04-09 at 3 23 53 PM

I can't decide if I like the idea of keeping the API responses just one level deep as they are now, and having the client would cut them up, like this PR does. It would depend on how much we anticipate the location sections changing; if we always have a stats card, it seems a premature optimization to make sure that can all be handled server-side.

@gwwar
Copy link
Contributor

gwwar commented Apr 9, 2020

I can't decide if I like the idea of keeping the API responses just one level deep as they are now, and having the client would cut them up, like this PR does.

My intuition is that keeping the client pretty simple would be ideal. We may need to break up the abstractions further. For example instead of just locations, do we specify the pages/tasks to render for a component. Let's play around a bit to see what feels best.

I'm also running into a similar problem for if we pick some sort of pager for educational cards. It could be that these turn into another "location/container"

image

@mmtr
Copy link
Member

mmtr commented Apr 10, 2020

It could be that these turn into another "location/container"

I like that. I think the API should treat each section with a header as a location.

{
  'alerts': [ ... ], // Congratulatory banners or site-wide notices.
  'up-next': [ ... ], // What to do up next (site setup checklist, current post-launch task).
  'stats': [ ... ], // Stats at a glance.
  'features': [ ... ], // Learn and grow.
  'management': [ ... ], // Manage your site.
}

Note that I deliberately not used current locations names so we can add the new layout to the endpoint without breaking existing behavior.

Calypso would decide then how to render each section:

  • Column mode. It's the default style, all cards displayed one below the other.
  • Page mode. It'll combine the cards and display some sort of pager so the user can browse them. We'll use this mode on the educational cards.
  • Stacked mode. It'll combine the cards and display them one behind the other. Dismissing / completing a card will show the one behind. We can use this mode on the notices.

EDIT: Pushed an update to D41677-code exploring what the API response looks like returning all these new locations. We need to update now this PR to handle them.

EDIT2: Just pushed a commit to this PR demonstrating how to use the new locations.

@@ -16,7 +16,7 @@ const cardComponents = {
'home-banner-legacy-stats-banners': StatsBanners,
};

const Upsells = ( { cards, primary, siteId, slug } ) => {
const Tasks = ( { cards, primary, siteId, slug } ) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up that I started something for Tasks in #41018 :)


return (
<>
<h2>{ translate( 'Learn and grow' ) }</h2>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Learn and grow with My Home' ) ES Score: 6

@@ -58,6 +58,10 @@
@include grid-template-columns( 12, 24px, 1fr );
grid-gap: 24px;
}

h2 {
font-family: 'Recoleta', serif;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to download the font, we can maybe check in to see how Gutenboarding is doing it. There are some performance considerations here as we do it, so we can maybe consult perfops on best practices.

@gwwar
Copy link
Contributor

gwwar commented Apr 10, 2020

I think this is starting to come together nicely @kwight @mmtr , I left some notes on the backend patch and asked a few clarifying questions for requirements in related thread.

Here's a snapshot of WIP for the branch:
Screen Shot 2020-04-10 at 4 03 01 PM

Some next steps/tasks in misc order (I'll file issues).


return (
<>
<h2>{ translate( 'Learn and grow' ) }</h2>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Learn and grow with My Home' ) ES Score: 6

<EducationalCard
header={ translate( 'The WordPress.com free photo library' ) }
text={ translate(
'Our free photo library integrates your site with over 40,000 beautiful copyright-free photos to create stunning designs.'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 20 times:
translate( 'The WordPress.com Free Photo Library integrates your site with beautiful copyright-free photos to create stunning designs.' ) ES Score: 8

return (
<>
<h2 className="learn-grow__heading customer-home__section-heading">
{ translate( 'Learn and grow' ) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 19 times:
translate( 'Learn and grow with My Home' ) ES Score: 6

@gwwar
Copy link
Contributor

gwwar commented Apr 16, 2020

@kwight I think we can close this one out? Please reopen if we still need this

@gwwar gwwar closed this Apr 16, 2020
@mmtr mmtr deleted the try/home-layout-i1 branch April 17, 2020 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE [Feature] My Home The main dashboard for managing a WordPress.com site.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants