From 39a959cc2939c391d03f33fdc0fef4b6d3de5d80 Mon Sep 17 00:00:00 2001 From: rg755421 Date: Thu, 1 Oct 2026 10:17:25 +0545 Subject: [PATCH 1/3] Fix - navigation.js TypeError without #site-navigation, and submenu caret never flipping - The touch-submenu handler called container.querySelectorAll() with no null check, so pages without #site-navigation (such as the Legacy Widget previews on Appearance > Widgets) threw a TypeError. It now returns early, like the first handler and Radiate's fix. - The sub-toggle handler toggled .fa-caret-right, which the icon never has (it is created as fa-caret-down), so the caret never changed. It now swaps fa-caret-down and fa-caret-up. - The script is versioned with the theme so a theme update reaches cached browsers. Fixes themegrill/accelerate-pro#53 and themegrill/accelerate-pro#101 (free side). Co-Authored-By: Claude Opus 5.5 (1M context) --- inc/functions.php | 3 ++- js/navigation.js | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/inc/functions.php b/inc/functions.php index 608aad0..69ec6a8 100644 --- a/inc/functions.php +++ b/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 ); diff --git a/js/navigation.js b/js/navigation.js index 15512c4..24e55a2 100644 --- a/js/navigation.js +++ b/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. */ From 7f87364ba6de2cbd7b31e39e9a923e30bc794369 Mon Sep 17 00:00:00 2001 From: rg755421 Date: Thu, 1 Oct 2026 10:23:41 +0545 Subject: [PATCH 2/3] Test - Guard the submenu caret flipping and navigation.js on pages without #site-navigation Two specs: the caret turns up when a submenu opens and down when it closes (a submenu is added before the script runs, so no menu setup is needed), and a page without #site-navigation throws no script error. Both fail on develop and pass with the fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../e2e/specs/mobile-menu/navigation.spec.ts | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 tests/e2e/specs/mobile-menu/navigation.spec.ts diff --git a/tests/e2e/specs/mobile-menu/navigation.spec.ts b/tests/e2e/specs/mobile-menu/navigation.spec.ts new file mode 100644 index 0000000..9b26910 --- /dev/null +++ b/tests/e2e/specs/mobile-menu/navigation.spec.ts @@ -0,0 +1,76 @@ +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", ''); + }), + ); + 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(() => + new MutationObserver(() => document.getElementById("site-navigation")?.remove()).observe(document, { + childList: true, + subtree: true, + }), + ); + + await page.goto("/"); + await page.waitForLoadState("networkidle"); + await expect(page.locator("#site-navigation")).toHaveCount(0); + expect(errors, "script errors on a page without #site-navigation").toEqual([]); +}); From da5476f543e81484f6fbed9ed6f151f2234eb7f8 Mon Sep 17 00:00:00 2001 From: rg755421 Date: Thu, 1 Oct 2026 12:24:56 +0545 Subject: [PATCH 3/3] Test - Assert navigation.js ran with #site-navigation already removed in the nav-less spec Co-Authored-By: Claude Opus 5.5 (1M context) --- .../e2e/specs/mobile-menu/navigation.spec.ts | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/e2e/specs/mobile-menu/navigation.spec.ts b/tests/e2e/specs/mobile-menu/navigation.spec.ts index 9b26910..09c58d3 100644 --- a/tests/e2e/specs/mobile-menu/navigation.spec.ts +++ b/tests/e2e/specs/mobile-menu/navigation.spec.ts @@ -62,15 +62,27 @@ test("the submenu caret turns up when a mobile submenu opens and back down when 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(() => - new MutationObserver(() => document.getElementById("site-navigation")?.remove()).observe(document, { + 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"); - await expect(page.locator("#site-navigation")).toHaveCount(0); + 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([]); });