CSS file is loaded in Marketing Overview page causing CSS issue

[ecgan]

Describe the bug:

When Google Listings and Ads is installed and activated, and we visit the Marketing Overview page (/wp-admin/admin.php?page=wc-admin&path=%2Fmarketing), its CSS file (/wp-content/plugins/google-listings-and-ads/js/build/index.css?ver=1668185841) is loaded.

This causes a CSS issue in the Multichannel Marketing page. I noticed this when I was working on PR woocommerce/woocommerce#35542. See screenshots below.

When GLA is not installed, things look as expected:

When GLA is installed and activated, the expand/collapse button icon becomes much smaller, and that's because of the padding: 0 8px:

I did some searches in the code base and I couldn't find the relevant CSS. I suspect the padding CSS comes from the bundled @wordpress/component package.

Possible solution 1 - does not work

(Update: This does not work, see #1765 (comment)).

I think the solution to this issue is that we should not load the CSS file when we are not in GLA pages.

The CSS file is loaded here - notice the $wc_admin_condition:

google-listings-and-ads/src/Admin/Admin.php

Lines 137 to 143 in 23c09d8

	$assets[] = ( new AdminStyleAsset(
	'google-listings-and-ads-css',
	'/js/build/index',
	defined( 'WC_ADMIN_PLUGIN_FILE' ) ? [ 'wc-admin-app' ] : [],
	(string) filemtime( "{$this->get_root_dir()}/js/build/index.css" ),
	$wc_admin_condition
	) );

$wc_admin_condition is defined as:

google-listings-and-ads/src/Admin/Admin.php

Lines 106 to 108 in 23c09d8

	$wc_admin_condition = function() {
	return wc_admin_is_registered_page();
	};

We probably just need to specify additional checks in the function so that the CSS file is only loaded when we are in GLA pages and not in Marketing Overview page.

Possible solution 2

As per #1765 (comment), @eason9487 suggested to consider one or both of the following approaches to see if they could fix this issue:

• DEWPed Button by import { Button } from 'extracted/@wordpress/components'
• Move all button-related CSS styles under the .gla-admin-page CSS class including the @import "node_modules/@wordpress/components/src/button/style" for scoping the GLA required adjustments.

Steps to reproduce:

1. Go to WC Settings > Advanced > Features (/wp-admin/admin.php?page=wc-settings&tab=advanced&section=features) and enable Multichannel Marketing experience.
2. Install and activate Google Listings and Ads.
3. Go to Marketing > Overview page (/wp-admin/admin.php?page=wc-admin&path=%2Fmarketing).

Expected behavior:

The expand/collapse button should appear as normal size.

Actual behavior:

The expand/collapse button would appear small in size.

[ecgan]

Possible solution

I think the solution to this issue is that we should not load the CSS file when we are not in GLA pages.
[...]
We probably just need to specify additional checks in the function so that the CSS file is only loaded when we are in GLA pages and not in Marketing Overview page.

I looked into this issue with the above solution in mind. I thought about how to determine whether the current page the user is in belongs to GLA plugin or not. I looked into things like WP Screen API, WC APIs like wc_admin_is_connected_page and wc_admin_is_registered_page, but none of them are able to tell us we are in a GLA page.

I came up with a solution to just check for the path param in the URL in $wc_admin_condition:

		$wc_admin_condition = function() {
			$path = urldecode( $_GET['path'] ?? '' );
			return wc_admin_is_registered_page() && str_starts_with( $path, '/google/' );
		};

The above is based on the path properties in pluginAdminPages array, which shows that all the pages in GLA have a path that starts with /google/:

google-listings-and-ads/js/src/index.js

Lines 53 to 139 in 161f254

	const pluginAdminPages = [
	{
	breadcrumbs: [ ...initialBreadcrumbs ],
	container: GetStartedPage,
	path: '/google/start',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-start',
	},
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Setup Merchant Center', 'google-listings-and-ads' ),
	],
	container: SetupMC,
	path: '/google/setup-mc',
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Setup Google Ads', 'google-listings-and-ads' ),
	],
	container: SetupAds,
	path: '/google/setup-ads',
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Dashboard', 'google-listings-and-ads' ),
	],
	container: Dashboard,
	path: '/google/dashboard',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-dashboard',
	},
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Reports', 'google-listings-and-ads' ),
	],
	container: Reports,
	path: '/google/reports',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-reports',
	},
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Product Feed', 'google-listings-and-ads' ),
	],
	container: ProductFeed,
	path: '/google/product-feed',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-product-feed',
	},
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Attribute Mapping', 'google-listings-and-ads' ),
	],
	container: AttributeMapping,
	path: '/google/attribute-mapping',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-attribute-mapping',
	},
	},
	{
	breadcrumbs: [
	...initialBreadcrumbs,
	__( 'Settings', 'google-listings-and-ads' ),
	],
	container: Settings,
	path: '/google/settings',
	wpOpenMenu: 'toplevel_page_woocommerce-marketing',
	navArgs: {
	id: 'google-settings',
	},
	},
	];

An assumption I made is that when we navigate between GLA pages and non-GLA pages, it is always a full page loading, not a dynamic front-end React routing. However, as pointed out by @eason9487 helpfully in our Slack conversation (p1672325494543439/1672320930.039149-slack-C046L2P9BEK), that is not always the case.

Specifically, if we are navigating from or to pages without page=wc-admin in the URL, the navigation will be a full page load. Examples of such pages:

• WC Settings
• WC Products
• Facebook plugin

If we are navigating between pages with page=wc-admin in the URL, they will be powered by dynamic front-end React routing. Examples of such pages:

• All GLA pages.
• WooCommerce > Home page
• WooCommerce > Customers page
• Marketing > Overview page
• All Analytics pages

The solution I proposed above wouldn't work for navigation between pages with page=wc-admin in the URL. Imagine that we start from the WooCommerce > Home page, GLA pages are not registered, GLA CSS and JS files are not loaded, and when we click on Marketing > Google Listings and Ads menu item, we would see a message that says "Sorry, you are not allowed to access this page."

demo-wc-admin-condition-wc-admin-pages.mp4

@eason9487 suggested to consider one or both of the following approaches to see if they could fix this issue:

• DEWPed Button by import { Button } from 'extracted/@wordpress/components'
• Move all button-related CSS styles under the .gla-admin-page CSS class including the @import "node_modules/@wordpress/components/src/button/style" for scoping the GLA required adjustments.

[eason9487]

📝 Related issue: #1391

[ecgan]

• DEWPed Button by import { Button } from 'extracted/@wordpress/components'

I'm going to try this approach, by doing the above import in the AppButton component, and change all the @wordpress/components <Button> to <AppButton>. AppButton is the wrapper around Button component. In the future, if there breaking changes around Button component, we just need to fix it in AppButton and it should work throughout the whole GLA plugin.