Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.43k stars 1.99k forks source link

Enable Bulk Plugin Management: Fix mobile layout and title alignment #96576

Closed p-jackson closed 16 hours ago

p-jackson commented 2 days ago

Fixes https://github.com/Automattic/dotcom-forge/issues/9742 Related to https://github.com/Automattic/wp-calypso/pull/96437 (it's an alternative fix)

Proposed Changes

I had to add a is-bulk-plugin-management class because, in my testing at least, the strategy of loading different CSS files based on features flags wasn't working. i.e. the code imported with import('style.scss') was being included, even when the feature flag was disabled. I'm not sure why, maybe webpack statically analyses the file while bundling and so you can't load CSS on demand?

Why are these changes being made?

The mobile layout was broken on the plugin screen on Jetpack Cloud. Titles weren't aligned. The background colour used to differentiate between the top and bottom section wasn't right. We want our page to look noice 💅

I think we should accept this fix (instead of #96437) because it's a smaller change which doesn't need to rearrange too much DOM. And it also is resilient to the presence of scrollbars.

Testing Instructions

You might need this info to get set up with the Jetpack Cloud dashboard pet6gk-1Gs-p2

Access Jetpack Cloud at jetpack.cloud.localhost:3001/plugins/manage Access Calypso screen at calypso.localhost:3000/plugins/manage

Test with the feature flag enabled and disabled:

Calypso without feature flag

https://github.com/user-attachments/assets/00b47710-3e42-4bc3-8621-0dd3e69527fc

Calypso with feature flag

https://github.com/user-attachments/assets/c31d38f7-0bfb-4c4b-a4fd-e6549a2b420d

Jetpack Cloud without feature flag

https://github.com/user-attachments/assets/d402ebac-92ee-494d-a70a-ddb260c5ebed

Jetpack Cloud with feature flag

https://github.com/user-attachments/assets/d3fd34c5-c6f1-4e40-a13d-a90a64b8191c

Pre-merge Checklist

github-actions[bot] commented 2 days ago
Calypso Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-125302
Jetpack Cloud live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-125302&env=jetpack
Automattic for Agencies live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-125302&env=a8c-for-agencies
matticbot commented 2 days ago

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 install-plugin.sh $pluginSlug fix/plugin-management-align on your sandbox.

matticbot commented 2 days ago

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~11 bytes added 📈 [gzipped])

``` name parsed_size gzip_size plugins +73 B (+0.0%) +11 B (+0.0%) jetpack-cloud-plugin-management +73 B (+0.0%) +11 B (+0.0%) ``` 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. **Gzip Size:** Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

vykes-mac commented 1 day ago

I think this fix looks cleaner but there might be some overlay with sticky header work done by @paulopmt1. See the linked PR

paulopmt1 commented 16 hours ago

I added one incremental change @p-jackson so it will limit the header size and keep it aligned with the DataViews:

https://github.com/user-attachments/assets/5341cdc9-699b-4fc1-972a-9046a7c9ae97