Normalize /sites sidebar animation on desktop.#101817
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
/sites navigations on desktop./sites sidebar animation on desktop.
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~8 bytes added 📈 [gzipped]) DetailsCommon code that is always downloaded and parsed every time the app is loaded, no matter which route is used. 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. |
|
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 |
c4e89fc to
071a9a1
Compare
fushar
left a comment
There was a problem hiding this comment.
Thanks for this! I believe it is an acceptable solution for now given the current way on how we do things.
There's more context and discussion here: paYKcK-5Qk-p2
|
@fushar Thanks for the quick review. Luckily an e2e test failed and exposed an error in the original logic. Because we're using |
fushar
left a comment
There was a problem hiding this comment.
Oohhh.... good catch 🤦 thanks, I've smoke-tested and didn't find any regression 👍
Related to #92911
Why are these changes being made?
When you click through the sidebar items, you'll get differing animations in the sidebar.
Proposed Changes
At face value, this seems like a problem with the transition and, it kinda is I guess.
We switch out
--sidebar-width-maxfrom272pxto295pxonce we add .is-global-sidebar-visible in client/layout/index.jsx.With Domains and Themes there is a delay in
isGlobalSidebarVisiblebeing said to true. That delay is why we see the transition.The solution in this PR is to add the routes to
getSiteDashboardRouteswhich is used by shouldShowSitesDashboard to determine (earlier) that we are viewing a page with the global sidebar.Considerations
I don't completely understand the implication of adding the routes here. The comment suggests that apart from affecting this layout, the presence of those routes will determine where they should up. Reading through the associated p2 also leaves things unclear for me.
Maybe you can confirm or provide better context @fushar? (Updated here).
Screenshots
Testing Instructions
/sitesPre-merge Checklist