-
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
My Sites: Fix Jetpack sites' listing on the "My Home" (/home
) page
#57952
Conversation
Sites displayed on the "My Home" page (`/home`) are filtered based on certain rules. This commit removes filter condition that prevents Jetpack-connected sites from loading.
Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:commit-f428a1ec60bdb39aa94a96d0c4f8f1b996042612 |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~34 bytes removed 📉 [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. |
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.
It works for me, I am glad you tagged someone familiar with VIP. I would want to make sure this doesn't adversely affect any VIP sites? 🤷♂️
How does this affect https://cloud.jetpack.com/ ? (It also runs on Calypso) |
Thanks for your comment Mikael. The code change shouldn't affect https://cloud.jetpack.com/ as |
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.
Reviewed and tested. LGTM.
Thank you so much for your feedback folks! 🙇🏼 I will deploy the change tomorrow morning since I am at the end of the day for today. |
Successfully merged and deployed to production. |
Sites displayed on the "My Home" page (`/home`) are filtered based on certain rules. This PR removes filter condition that prevents Jetpack-connected sites from loading on the `/home` page.
Sites displayed on the "My Home" page (
/home
) are filtered based on the following rule:wp-calypso/client/my-sites/sites/index.jsx
Lines 59 to 62 in b019e58
It looks like the rule was originally added to filter out VIP sites only, but it is filtering out all Jetpack-connected sites as well (issue reported in #57295).
To add more specificity, the condition
! site.is_vip && ! ( site.jetpack && ! site.options.is_automated_transfer )
returnsfalse
for every Jetpack-connected site. This results in each Jetpack-connected site to be left out from thesites
array that is then rendered in theSite Selector
component:wp-calypso/client/components/site-selector/index.jsx
Lines 279 to 281 in b019e58
Changes proposed in this Pull Request
! ( site.jetpack && ! site.options.is_automated_transfer )
) to make sure only VIP sites are filtered out.Hey @gwwar and @kwight 👋🏼 I am tagging you here in case there's something I might have missed in relation to VIP sites (as the filter was originally added in #39090). I don't think the change would have any effect on VIP sites, just wanted to double-check with you.
Screenshots
Before:
After:
Testing instructions
It shouldn't be possible to reproduce #57295.
In steps:
Related to #57295