Fix - navigation.js TypeError without #site-navigation, and submenu caret never flipping

inc/functions.php

@@ -66,7 +66,8 @@ function accelerate_scripts_styles_method() {
 		wp_enqueue_script( 'jquery-cycle2-swipe' );
 	}
 
-	wp_enqueue_script( 'accelerate-navigation', ACCELERATE_JS_URL . '/navigation.js', array( 'jquery' ), false, true );
+	// Theme version, so a theme update busts cached copies (false would use the WordPress version).
+	wp_enqueue_script( 'accelerate-navigation', ACCELERATE_JS_URL . '/navigation.js', array( 'jquery' ), ACCELERATE_THEME_VERSION, true );
 
 	// Skip link focus fix JS enqueue.
 	wp_enqueue_script( 'accelerate-skip-link-focus-fix', ACCELERATE_JS_URL . '/skip-link-focus-fix.js', array(), false, true );

js/navigation.js

@@ -75,7 +75,8 @@ jQuery( document ).ready( function() {
 	jQuery( '#site-navigation .sub-toggle' ).click( function() {
 
 		jQuery( this ).parent( '.menu-item-has-children' ).children( 'ul.sub-menu' ).first().slideToggle( '1000' );
-		jQuery( this ).children( '.fa-caret-right' ).first().toggleClass( 'fa-caret-down' );
+		// The icon is created as fa-caret-down; swap it with fa-caret-up so it turns when opened.
+		jQuery( this ).children( '.fa' ).first().toggleClass( 'fa-caret-down fa-caret-up' );
 		jQuery( this ).toggleClass( 'active' );
 
 	} );
@@ -87,6 +88,11 @@ jQuery( document ).ready( function() {
 
 	var container = document.getElementById( 'site-navigation' );
 
+	// Pages without the primary nav (e.g. Legacy Widget previews in wp-admin) have no container.
+	if ( ! container ) {
+		return;
+	}
+
 	/**
 	 * Toggles `focus` class to allow submenu access on tablets.
 	 */

tests/e2e/specs/mobile-menu/navigation.spec.ts

@@ -0,0 +1,88 @@
+import { test, expect } from "@playwright/test";
+
+/**
+ * @area mobile-menu
+ * @tier fresh
+ * @guards themegrill/accelerate-pro#101
+ * @source themegrill/accelerate-pro#101 (reported by subin-shk)
+ * @why The sub-toggle handler in js/navigation.js toggled `.fa-caret-right`,
+ *      which the icon never has (it is created as fa-caret-down), so the caret
+ *      never changed. A submenu is added to the first menu item before the
+ *      script runs, so this does not depend on the site's menu. Checks the
+ *      glyph actually drawn, not just the classes.
+ */
+test("the submenu caret turns up when a mobile submenu opens and back down when it closes @mobile-menu @fresh", async ({
+  page,
+}) => {
+  // Registered before jQuery's ready handler, so the theme sees a real parent item.
+  await page.addInitScript(() =>
+    document.addEventListener("DOMContentLoaded", () => {
+      const item = document.querySelector("#site-navigation ul li");
+      if (!item) return;
+      item.classList.add("menu-item-has-children");
+      item.insertAdjacentHTML("beforeend", '<ul class="sub-menu"><li><a href="#tgqa-child">Injected child</a></li></ul>');
+    }),
+  );
+  await page.setViewportSize({ width: 375, height: 812 });
+  await page.goto("/");
+
+  await page.locator("#site-navigation .menu-toggle").click();
+  const toggle = page.locator("#site-navigation .sub-toggle").first();
+  const icon = toggle.locator("i.fa");
+  const submenu = toggle.locator("xpath=../ul").first();
+  const glyph = () => icon.evaluate((el) => getComputedStyle(el, "::before").content.replace(/["']/g, "").codePointAt(0)?.toString(16));
+
+  await expect(icon).toHaveClass(/\bfa-caret-down\b/);
+  expect(await glyph(), "closed: caret-down glyph").toBe("f0d7");
+
+  await toggle.click();
+  await expect(submenu).toBeVisible();
+  await expect(icon).toHaveClass(/\bfa-caret-up\b/);
+  await expect(icon).not.toHaveClass(/\bfa-caret-down\b/);
+  expect(await glyph(), "open: caret-up glyph").toBe("f0d8");
+
+  await toggle.click();
+  await expect(submenu).toBeHidden();
+  await expect(icon).toHaveClass(/\bfa-caret-down\b/);
+  await expect(icon).not.toHaveClass(/\bfa-caret-up\b/);
+  expect(await glyph(), "closed again: caret-down glyph").toBe("f0d7");
+});
+
+/**
+ * @area mobile-menu
+ * @tier fresh
+ * @guards themegrill/accelerate-pro#53
+ * @source themegrill/accelerate-pro#53; themegrill/radiate-pro#107 (same fix, same spec)
+ * @why The touch-submenu handler in js/navigation.js called
+ *      container.querySelectorAll() with no null check, so any page without
+ *      #site-navigation threw "Cannot read properties of null". On a real site
+ *      that is the Legacy Widget preview on Appearance > Widgets; here the nav
+ *      is removed while the page parses, before the footer script runs.
+ */
+test("a page without the primary navigation throws no script error @mobile-menu @fresh", async ({ page }) => {
+  const errors: string[] = [];
+  page.on("pageerror", (e) => errors.push(e.message));
+  await page.addInitScript(() => {
+    const original = Document.prototype.getElementById;
+    // The observer uses the unwrapped lookup, so only the page's own lookups are recorded below.
+    new MutationObserver(() => original.call(document, "site-navigation")?.remove()).observe(document, {
+      childList: true,
+      subtree: true,
+    });
+    // Record what each lookup of the nav returned, so the test proves the script ran with it already gone.
+    const lookups: boolean[] = [];
+    (window as unknown as { tgqaNavLookups: boolean[] }).tgqaNavLookups = lookups;
+    Document.prototype.getElementById = function (id: string) {
+      const found = original.call(this, id);
+      if ("site-navigation" === id) lookups.push(null === found);
+      return found;
+    };
+  });
+
+  await page.goto("/");
+  await page.waitForLoadState("networkidle");
+  const lookups = await page.evaluate(() => (window as unknown as { tgqaNavLookups: boolean[] }).tgqaNavLookups);
+  expect(lookups.length, "navigation.js never looked up #site-navigation").toBeGreaterThan(0);
+  expect(lookups.every(Boolean), "#site-navigation still existed when the script looked it up").toBe(true);
+  expect(errors, "script errors on a page without #site-navigation").toEqual([]);
+});