Skip to content

Fix: keep parent menu + siblings visible on submenu pages #1

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.

Sign up for GitHub

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

Open
rmorse wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: keep parent menu + siblings visible on submenu pages #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 15 additions & 6 deletions content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@
}

const isCore = CORE_MENU_SLUGS.has(hrefRel);
const isCurrent = keep.has(hrefRel) || link.classList.contains('current');
const isCurrent = keep.has(hrefRel) ||
link.classList.contains('current') ||
li.classList.contains('wp-has-current-submenu');

// Mark with data attributes for CSS targeting
if (isCore) {
Expand All @@ -259,17 +261,24 @@
// Handle submenu items
const submenu = li.querySelector('.wp-submenu');
if (submenu && (isCore || isCurrent)) {
cleanSubmenu(submenu, allowed, keep);
const showAllItems = li.classList.contains('wp-has-current-submenu');
cleanSubmenu(submenu, allowed, keep, showAllItems);
}
});

// CSS is now handled by content.css file injected by manifest
}

function cleanSubmenu(submenu, allowed, currentSlugs) {
function cleanSubmenu(submenu, allowed, currentSlugs, showAll = false) {
// Mark submenu items with data attributes for CSS targeting
const submenuItems = submenu.querySelectorAll('li');
submenuItems.forEach((li) => {
// If showAll is true, mark all items as core to keep them visible
if (showAll) {
li.setAttribute('data-vgp-core', 'true');
return;
}
Comment on lines +276 to +280
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider setting data-vgp-current even when showAll is true.

When showAll is true, all items are marked as data-vgp-core but the actual current item won't receive the data-vgp-current attribute due to the early return. If your CSS uses data-vgp-current for highlighting the active submenu item, this could cause a styling inconsistency where the current item loses its visual emphasis.

If highlighting isn't needed in this scenario, the current implementation is fine.

Proposed fix to preserve current item highlighting 
       // If showAll is true, mark all items as core to keep them visible
       if (showAll) {
         li.setAttribute('data-vgp-core', 'true');
-        return;
+        // Still check if this is the current item for styling purposes
+        const link = li.querySelector('a');
+        if (link && (link.classList.contains('current') || li.classList.contains('current'))) {
+          li.setAttribute('data-vgp-current', 'true');
+        } else {
+          li.removeAttribute('data-vgp-current');
+        }
+        return;
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// If showAll is true, mark all items as core to keep them visible
if (showAll) {
li.setAttribute('data-vgp-core', 'true');
return;
}
// If showAll is true, mark all items as core to keep them visible
if (showAll) {
li.setAttribute('data-vgp-core', 'true');
// Still check if this is the current item for styling purposes
const link = li.querySelector('a');
if (link && (link.classList.contains('current') || li.classList.contains('current'))) {
li.setAttribute('data-vgp-current', 'true');
} else {
li.removeAttribute('data-vgp-current');
}
return;
}
🤖 Prompt for AI Agents 
In `@content.js` around lines 276 - 280, The early return when showAll is true
prevents setting the current highlight; update the logic in the block handling
showAll (where li.setAttribute('data-vgp-core', 'true') is called) to also set
or preserve the data-vgp-current attribute for the item that should be current
(or explicitly clear/set it based on currentItem state) before returning, or
remove the early return and let the subsequent current-item assignment code run
so the current element receives data-vgp-current while all elements still get
data-vgp-core.


const link = li.querySelector('a');
if (!link) {
// If no link, mark as core to be safe
Expand All @@ -290,17 +299,17 @@
}

const isCore = allowed.has(hrefRel);
const isCurrent = currentSlugs.has(hrefRel) ||
const isCurrent = currentSlugs.has(hrefRel) ||
link.classList.contains('current') ||
li.classList.contains('current');

// Mark with data attributes for CSS targeting
if (isCore) {
li.setAttribute('data-vgp-core', 'true');
} else {
li.removeAttribute('data-vgp-core');
}

if (isCurrent) {
li.setAttribute('data-vgp-current', 'true');
} else {
Expand Down