Fix - navigation.js TypeError without #site-navigation, and submenu caret never flipping - #90
rajatgautam755421 wants to merge 3 commits into
Conversation
…aret 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) <noreply@anthropic.com>
…thout #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) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The E2E coverage contains a critical assertion issue and a moderate nondeterministic setup issue.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes missing-navigation JavaScript errors and corrects mobile submenu caret behavior.
Changes:
- Adds a null guard for absent navigation.
- Swaps submenu caret classes correctly.
- Versions the navigation script and adds E2E coverage.
| File | Summary |
|---|---|
tests/e2e/specs/mobile-menu/navigation.spec.ts |
Adds regression tests; the caret assertion has a critical CSS-escape issue, and navigation removal is nondeterministic (moderate). |
js/navigation.js |
Adds the navigation guard and caret behavior fix. |
inc/functions.php |
Versions the navigation script for cache invalidation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| 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)); |
There was a problem hiding this comment.
Checked — not reproducible, so leaving the parsing out. getComputedStyle(el, '::before').content returns the resolved string, not the stylesheet's escape: for fa-caret-down it is the U+F0D7 character itself, and glyph() reads f0d7 in Chromium, Firefox and WebKit (tested all three; none return \f0d7). The spec also fails on develop and passes with the fix, which could not happen if it always read 5c.
… in the nav-less spec Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Re the overview note on the nav-less spec being nondeterministic: the removal is deterministic in practice (the parser runs a microtask checkpoint before each parser-inserted script, so the observer removes the nav before |
Fixes themegrill/accelerate-pro#53 (reported by @iamprazol) and the free side of themegrill/accelerate-pro#101 (reported by @subin-shk). Pro: themegrill/accelerate-pro#121. Same null-guard fix as Radiate (themegrill/radiate-pro#107).
container.querySelectorAll()with no null check, so pages without#site-navigationthrewTypeError … navigation.js:96:30— reproduced on Appearance → Widgets with a Legacy Widget preview, exactly as reported. Now returns early..fa-caret-right, which the icon never has, so the caret never changed. Now swapsfa-caret-down/fa-caret-up.Testing (
developvs this branch): Widgets screen and a page without the nav — error before, none after. Real 3-level menu: open/close, nested, 7 rapid taps — 19/19 checks vs 14/19 ondevelop; desktop hover and touch tap-to-open unchanged. Two new@freshspecs, both fail ondevelopand pass here.Changelog: Fix - JavaScript error on pages without the primary menu, and the mobile submenu caret not changing direction.
🤖 Generated with Claude Code