-
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
Sites dashboard: redirect to My Home without flashing preview pane #97172
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~100 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~386 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. Async-loaded Components (~12 bytes removed 📉 [gzipped])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
ffd943a
to
d36b82c
Compare
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 |
d36b82c
to
2f65e51
Compare
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.
Tested on a P2 site and a Jetpack-connected site and it works as intended 👍
Fixes #97109
Proposed Changes
After we removed JS overrides from sites dashboard (pbxlJb-6DF-p2), many logic around it are not necessary anymore. However, we have a regression where we show a flashing preview pane when a row click leads to My Home.
This is due to a different implementation of how we tell DataViews which site is currently selected. Previously we maintain a
dataViewsState.selectedItem
; now we use Calypso'sselectedSite
directly, which is ultimately derived from the URL. When the URL is redirected/home/:site
, we're already too late in stopping the preview pane from opening.This PR cleans up the logic and fixes the regression, by returning null in
SitesDashboard
when a site is selected but the route is not matching a valid site dashboard route.Screen.Capture.on.2024-12-05.at.17-56-16.mp4
mG73Ox.mp4
Testing Instructions
It's hard to log in as different user in Calypso live URLs in other browser, so we can use P2s instead:
Regression testing: click around on /sites and verify that nothing related to opening/closing preview panes breaks.
Pre-merge Checklist