Skip to content
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

Separate legacy WC menu handling to a separate hook, #1038

Merged
merged 2 commits into from
Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 2 additions & 20 deletions js/src/components/navigation-classic/main-tab-nav.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import { useEffect } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { getNewPath, getPath } from '@woocommerce/navigation';

Expand All @@ -10,6 +9,7 @@ import { getNewPath, getPath } from '@woocommerce/navigation';
*/
import { glaData } from '.~/constants';
import AppTabNav from '.~/components/app-tab-nav';
import useLegacyMenuEffect from '.~/hooks/useLegacyMenuEffect';

let tabs = [
{
Expand Down Expand Up @@ -46,25 +46,7 @@ const getSelectedTabKey = () => {
};

const MainTabNav = () => {
useEffect( () => {
// Highlight the wp-admin dashboard menu
const marketingMenu = document.querySelector(
'#toplevel_page_woocommerce-marketing'
);

if ( ! marketingMenu ) {
return;
}

const dashboardLink = marketingMenu.querySelector(
"a[href^='admin.php?page=wc-admin&path=%2Fgoogle%2Fdashboard']"
);

marketingMenu.classList.add( 'current', 'wp-has-current-submenu' );
if ( dashboardLink ) {
dashboardLink.parentElement.classList.add( 'current' );
}
}, [] );
useLegacyMenuEffect();

const selectedKey = getSelectedTabKey();

Expand Down
42 changes: 42 additions & 0 deletions js/src/hooks/useLegacyMenuEffect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* External dependencies
*/
import { useEffect } from '@wordpress/element';
/**
* Internal dependencies
*/
import isWCNavigationEnabled from '.~/utils/isWCNavigationEnabled';

/**
* Mocked result of parsing a page entry from {@link /js/src/index.js} by WC-admin's <Route>.
*
* @see https://github.com/woocommerce/woocommerce-admin/blob/release/2.7.1/client/layout/controller.js#L240-L244
*/
const dashboardPage = {
match: { url: '/google/dashboard' },
wpOpenMenu: 'toplevel_page_woocommerce-marketing',
};

/**
* Effect that highlights the GLA Dashboard menu entry in the legacy WC menu.
*
* Should be called for every "root page" (`.~/pages/*`) that wants to open the GLA menu.
*
* The hook could be removed once WC Navigation will be always enabled,
* or if we make the plugin fully use the routing feature of WC,
* and let this be done by proper matching of URL matchers from {@link /js/src/index.js}
*
* @see window.wpNavMenuClassChange
*/
export default function useLegacyMenuEffect() {
const navigationEnabled = isWCNavigationEnabled();
return useEffect( () => {
// Highlight the wp-admin dashboard menu
if ( ! navigationEnabled ) {
window.wpNavMenuClassChange(
dashboardPage,
dashboardPage.match.url
);
}
}, [ navigationEnabled ] );
}
4 changes: 4 additions & 0 deletions js/src/settings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { getQuery, getHistory } from '@woocommerce/navigation';
/**
* Internal dependencies
*/
import useLegacyMenuEffect from '.~/hooks/useLegacyMenuEffect';
import useGoogleAccount from '.~/hooks/useGoogleAccount';
import { subpaths, getReconnectAccountsUrl } from '.~/utils/urls';
import NavigationClassic from '.~/components/navigation-classic';
Expand All @@ -19,6 +20,9 @@ import './index.scss';

const Settings = () => {
const { subpath } = getQuery();
// Make the component highlight GLA entry in the WC legacy menu.
useLegacyMenuEffect();

const { google } = useGoogleAccount();
const isReconnectAccountsPage = subpath === subpaths.reconnectAccounts;

Expand Down