Hiding primary tabs on Settings within WooCommerce since it's redunda…

[joelclimbsthings]

Fixes #5572

The given issue surfaced the fact that the top (primary) tabs within Settings were actually redundant with the new navigation, and it was decided that the tabs should be hidden when navigation is active.

I also discovered that there were a couple pages missing from the navigation that would have been inaccessible after the change, so I added those in as well. You'll find them as "Logs" and "Scheduled Actions" under "Tools."

There is a small style tweak to the secondary tabs (which appear in "Settings -> Advanced") to make them a bit more prominent with some additional whitespace. These will see further visual changes in a future PR, as discussed in the issue.

Screenshots

Settings -> Advanced without the primary tabs:

Tools menu with added items:

Detailed test instructions:

• Checkout branch
• Ensure new navigation feature is enabled
• Navigate to WooCommerce -> Settings -> Advanced
• Observe the the top tabs are not visible, and that the secondary tabs still are
• Navigate to WooCommerce -> Tools
• Observe that we now have items for Logs & Scheduled Actions

[joelclimbsthings]

@elizaan36 I just wanted to let you know that I tentatively made a couple very small changes to the styling on the secondary tabs for this PR; bumped up the font size 1px and added some top/bottom margin. You can view the result in the screenshot above^. It sounds like we'll be changing this display in a further PR as well, but just let me know if you'd rather I revert these changes.

[elizaan36]

hey @joelclimbsthings just out of curiosity, why did you make those changes? The design was meant to be using the WP text components - body for the subnav items and subtitle for the section header.

[joelclimbsthings]

@elizaan36 Sans the primary tabs, the secondary tabs seemed a bit crowded above and below. You mentioned the potential of using the tab component in the issue, so this was just a quick interim tweak until we get that design finalized. We're still using the same components/etc; just a small style tweak, that I'm more than happy with or without.

[elizaan36]

oh my bad, I thought you were referring to the new nav. Tweaking the size of the small tabs was a good idea!🚢

[joshuatf]

Code here looks good and tests well for the tool pages. But we're going to need to discuss hiding all nav tab wrappers as this may lead to inadvertently hiding some pages.

[joshuatf]

This is a little bit risky. I've noticed some of the other extensions using the woo nav tab wrapper while working on #5335.

This is also used on several other core pages, like the marketplace.

I think this is going to require more thought before we hide all of these nav tab wrappers since they can easily be reused or have items added/removed without the navigation being aware.

[joelclimbsthings]

You're right @joshuatf , that selector was way too broad 😱.

I think I figured out a way to narrowly target just the tabs within Settings via a body class, and I've pushed that change. It actually does not apply the styles to any React-based pages within Settings (ie Analytics), but I'm noticing they didn't have the primary tabs to begin with.

I've noticed some of the other extensions using the woo nav tab wrapper while working on #5335.

Let me know if you still think this change could still have issues with other extensions as mentioned^

[joshuatf]

Nice change here 👍

[joshuatf]

The updated selector looks better and manually hiding nav groups seems like the best path forward for now.

I think we can do the same with wc-status screens as well and register those items via a filter.

	add_filter( 'woocommerce_admin_status_tabs', array( $this, 'add_tool_items' ), PHP_INT_MAX );

	public function add_tool_items( $tabs ) {
		$order = 10;
		foreach( $tabs as $key => $tab ) {
			self::add_item(
				array(
					'parent'     => 'tools',
					'title'      => $tab,
					'capability' => 'manage_woocommerce',
					'id'         => 'wc-status-' . $key,
					'url'        => 'admin.php?page=wc-status&tab=' . $key,
					'order'      => $order,
				)
			);
			$order += 10;
		}
		return $tabs;
	}

We'll need to register the screens earlier though since that hook will fire too late:

	add_filter( 'admin_init', array( $this, 'register_status_screen' ) );
	public function register_status_screen() {
		Screen::add_screen( 'wc-status' );
	}

Then we can remove all the manual tool page registrations with the exception of import/export.

[joshuatf]

I think we may also need to add wc-status to $excluded_items for this to work so that it doesn't get migrated to the settings group.

[joelclimbsthings]

Thanks @joshuatf , easy 'nuff to apply those styles to wc-status pages as well. As far as adding those pages via filter instead of manually, the code you provided almost works. After poking at it a bit the main issue seems to be that the filter you mentioned is actually only executed on a subset of pages, preventing those menu items from being added when the navigation is displayed over others.

In more detail, the woocommerce_admin_status_tabs filter is actually applied in another function (wc_admin_get_core_pages_to_connect) that is itself only executed via the filter wc_admin_filter_core_page_breadcrumbs. I'm assuming this is run only on pages that utilize the breadcrumbs/tabs themselves.

An easy solution to that approach isn't jumping out at me. Perhaps we can create an separate issue to revisit that, while narrowing to only address the tab display issue in this one?

[joshuatf]

Thanks for the change, Joel! Will approve this one as is for now, but definitely worth opening an issue in core to get a method to retrieve those status pages.

This is something that could easily get out of sync and I'm not too comfortable hiding it and waiting for bugs to surface.