-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Try Enabling Customer Home for all Simple and Atomic Sites #39071
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~12 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~3 bytes added 📈 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
Seems that VIP sites are currently included:
As long as all the cards, tools, actions and links work there, I'd suggest to have the same behavior always. cc @Automattic/vip for feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this with a bunch of different types of sites (private, public, pre-Aug 2019, site created today, site redirect, domain-only) and all worked as expected!
canCurrentUserUseCustomerHome( createState( { created_at: '0000-00-00T00:00:00+00:00' } ) ) | ||
).toBe( false ); | ||
} ); | ||
|
||
test( "should return false if user can't manage site options", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a test to check for exclusion of VIP sites, too? Or do those qualify as "jetpack" sites?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original check didn't include VIP as a consideration, but we should verify with folks as that might have been an oversight
To clarify some of the VIP questions above, we still have sites that are both "traditional" WordPress.com and others are Jetpack/remote. So excluding for VIP would need an additional check to match the ones that are traditional sites, if that were to be desired :) |
Added p9GBOq-1A2-p2 |
@WPprodigy @mmtr I've created #39090 to exclude VIP sites for now. I'll rebase this branch after this lands. |
This all looks great to me 🎉 I also noticed VIP sites, but it sounds like #39090 is a followup to handle that. It's a nice touch how the "Your site has been created!" celebratory notice goes away after 30 mins. (I really like how joyful it is, compared to the uninspired plain My Home title – I wonder if keeping it around for a few days would be a good idea.) |
c001c54
to
a209af6
Compare
Going to land this enabled only in horizon/dev. Thanks for the reviews folks! |
Fixes a regression that was included in #39071 causing the checklist to be displayed even if it is completed.
Fixes #38999
This PR adds a new feature flag
home/all
enabled indevelopment
andhorizon
, that skips the site age check if the flag is set to true and also fixes a small bug where an empty checklist would render for sites created before Aug 19th.Open Question
p9GBOq-1A2-p2
Testing Instructions