Stats: Disable subscribers tab if subscribers module is disabled#101842
Stats: Disable subscribers tab if subscribers module is disabled#101842Nikschavan merged 19 commits intotrunkfrom
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~6 bytes added 📈 [gzipped]) DetailsCommon code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~3901 bytes removed 📉 [gzipped]) DetailsSections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~613 bytes added 📈 [gzipped]) DetailsReact 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. Generated by performance advisor bot at iscalypsofastyet.com. |
kangzj
left a comment
There was a problem hiding this comment.
Fantastic work! It works well for me and the changes look good to me!
annacmc
left a comment
There was a problem hiding this comment.
Looks great, works well for me! 🚀
… is inactive" This reverts commit 60ed2f6.
There was a problem hiding this comment.
Great work to reuse siteSelection for the site data before going to pages on Odyssey Stats! It works well to avoid the Subscribers tab item flickering. 👍🏼 Just left some nit comments.
On the other hand, can we fetch the site options.active_modules when preparing the initial state on the Jetpack side? 🤔 cc @kangzj
Don't pass initialState to the context
I was planning to add this and make the render blocking stats request optional if we already have |
| setupContextMiddleware( store, queryClient ); | ||
|
|
||
| // Initialize site data early in the app boot process. | ||
| await initializeSiteData( store ); |
dognose24
left a comment
There was a problem hiding this comment.
LGTM and works as described! 👍🏼
The current improvement looks good enough. I was just curious if we could easily fetch |
I personally prefer doing async requests after the page generated. Fetching those data in the initial state would increase the page execution time, which isn't ideal as users wouldn't even see a spinner if the page is not generated by PHP yet. So I would like the current approach better. |
|
As an improvement point for the future: We could send the setLocale asynchronous calls together with the site information request in parallel, so that the page finishes loading faster. PS: I can feel the latency added to the page loading on my local with this PR, so it might be worth looking into as follow ups. |
Related to #80572
Proposed Changes
/statshomepage from the subscribers stats page if the stats module is disabled.Why are these changes being made?
Testing Instructions
Pre-merge Checklist