Skip to content

Replace per-report menu with shared cross-dashboard navigation#52

Open
Copilot wants to merge 7 commits intomasterfrom
copilot/replace-navigation-menu
Open

Replace per-report menu with shared cross-dashboard navigation#52
Copilot wants to merge 7 commits intomasterfrom
copilot/replace-navigation-menu

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 12, 2026

  • Review the new PR comments and inspect the current navigation customization files
  • Check recent workflow runs/logs for this branch
  • Remove the unintended website color changes while preserving the shared navigation behavior
  • Run targeted validation for the updated navigation customization
  • Run final review/security validation on the committed change
  • Reply on the new PR comments with the addressing commit and screenshot

Copilot AI linked an issue Apr 12, 2026 that may be closed by this pull request
Copilot AI and others added 2 commits April 12, 2026 16:47
Agent-Logs-Url: https://github.com/meshery/qa/sessions/3ade9a24-577c-45dd-a39a-9e1958fd0286

Co-authored-by: pontusringblom <170570911+pontusringblom@users.noreply.github.com>
Agent-Logs-Url: https://github.com/meshery/qa/sessions/3ade9a24-577c-45dd-a39a-9e1958fd0286

Co-authored-by: pontusringblom <170570911+pontusringblom@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace existing navigation menu with consistent layout Replace per-report menu with shared cross-dashboard navigation Apr 12, 2026
Copilot AI requested a review from pontusringblom April 12, 2026 16:58
@leecalcote leecalcote marked this pull request as ready for review April 15, 2026 01:46
Copilot AI review requested due to automatic review settings April 15, 2026 01:46
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Introduces a shared, consistent top navigation across all generated Allure dashboards/reports by injecting navigation metadata into each page and rendering a uniform nav bar while suppressing the legacy per-report menu.

Changes:

  • Build and inject window.mesheryReportNav into each generated report page during customization.
  • Render a shared sticky top navigation and mark the active page on load.
  • Hide the legacy report-specific navigation UI so the shared nav becomes primary.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
customize-report.js Discovers report pages, computes relative links, and injects shared nav metadata into each page.
custom-script.js Renders shared nav from injected metadata and hides the legacy menu/panel in the UI.
custom-style.css Adds styling for the new sticky navigation bar, hover/focus, and active state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread customize-report.js
Comment on lines +106 to +114
function createNavDataScript(currentPageDir, navEntries) {
const navData = navEntries.map(({ label, relativeDir, reportPageDir }) => ({
id: relativeDir || 'home',
label,
href: toRelativeHref(currentPageDir, reportPageDir),
}));

return ` <script>${navDataMarker} ${JSON.stringify(navData)};</script>\n`;
}
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Injecting raw JSON.stringify(navData) into an inline <script> can be broken (and become an XSS/HTML-injection vector) if any string value contains </script> or similar sequences. Consider either (a) emitting the JSON via <script type=\"application/json\" ...> and parsing it in custom-script.js, or (b) escaping < (e.g., replace < with \\u003c) in the serialized JSON before embedding it in HTML.

Copilot uses AI. Check for mistakes.
Comment thread customize-report.js Outdated
Comment on lines +106 to +108
function createNavDataScript(currentPageDir, navEntries) {
const navData = navEntries.map(({ label, relativeDir, reportPageDir }) => ({
id: relativeDir || 'home',
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Using relativeDir as an id can produce values containing path separators (e.g., foo/bar) or platform-specific separators if it ever leaks through, which makes the id awkward to consume and inconsistent with the intended stable identifiers. Either sanitize/normalize the id (e.g., slugify and replace separators) or drop id entirely if it’s not used by the client script.

Suggested change
function createNavDataScript(currentPageDir, navEntries) {
const navData = navEntries.map(({ label, relativeDir, reportPageDir }) => ({
id: relativeDir || 'home',
function formatNavId(relativeDir) {
if (!relativeDir) {
return 'home';
}
const normalizedDir = relativeDir.split(path.sep).join('/').replace(/[\\/]+/g, '/');
const sanitizedId = normalizedDir
.split('/')
.filter(Boolean)
.map((segment) => segment.replace(/[^A-Za-z0-9_-]+/g, '-').replace(/^-+|-+$/g, ''))
.filter(Boolean)
.join('-');
return sanitizedId || 'home';
}
function createNavDataScript(currentPageDir, navEntries) {
const navData = navEntries.map(({ label, relativeDir, reportPageDir }) => ({
id: formatNavId(relativeDir),

Copilot uses AI. Check for mistakes.
Comment thread custom-script.js
Comment on lines +147 to +157
function hideLegacyReportMenu() {
let changed = false;

document.querySelectorAll('button').forEach((button) => {
if (button.dataset.mesheryLegacyNavHidden === 'true') {
return;
}

if (normalizeText(button.textContent) !== LEGACY_NAV_BUTTON_TEXT) {
return;
}
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Because applyBranding() is typically triggered multiple times (often via a MutationObserver in this kind of customization script), scanning all buttons and panels each time can become expensive on large Allure pages. Consider adding a single page-level guard (e.g., a module-scoped boolean like legacyNavHiddenOnce) and early-return once the legacy menu has been hidden, and/or narrowing the selectors to the known navigation container instead of querying the entire document each time.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 0611949.

hideLegacyReportMenu() now uses a module-scoped legacyNavHiddenOnce guard and exits early after the legacy nav has been hidden, so applyBranding() no longer keeps rescanning the full page on later observer runs.

Screenshot: updated navigation

Comment thread custom-style.css Outdated
Comment on lines +35 to +39
.meshery-report-nav__link:hover,
.meshery-report-nav__link:focus-visible {
background: var(--bg-control-secondary-medium, rgba(148, 163, 184, 0.2));
border-color: var(--on-border-primary, rgba(100, 116, 139, 0.4));
outline: none;
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Removing the focus outline on :focus-visible without providing a clearly visible alternative focus indicator can harm keyboard accessibility. Consider replacing outline: none; with an explicit focus ring (e.g., outline or box-shadow) that meets contrast requirements, so focused nav links remain clearly discernible.

Suggested change
.meshery-report-nav__link:hover,
.meshery-report-nav__link:focus-visible {
background: var(--bg-control-secondary-medium, rgba(148, 163, 184, 0.2));
border-color: var(--on-border-primary, rgba(100, 116, 139, 0.4));
outline: none;
.meshery-report-nav__link:hover {
background: var(--bg-control-secondary-medium, rgba(148, 163, 184, 0.2));
border-color: var(--on-border-primary, rgba(100, 116, 139, 0.4));
}
.meshery-report-nav__link:focus-visible {
background: var(--bg-control-secondary-medium, rgba(148, 163, 184, 0.2));
border-color: var(--on-border-primary, rgba(100, 116, 139, 0.4));
outline: 2px solid var(--bg-support-capella, #1d4ed8);
outline-offset: 2px;

Copilot uses AI. Check for mistakes.
@winkletinkle
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/meshery/qa/sessions/65103193-fc41-4af9-99cc-42efc81b6faa

Co-authored-by: winkletinkle <182693777+winkletinkle@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Apr 15, 2026

@copilot apply changes based on the comments in this thread

Addressed in 0611949.

Applied the review-thread fixes: escaped the injected nav JSON before embedding it in the inline script, removed the unused nav item id, added a one-time guard around legacy menu hiding, and restored a visible keyboard focus ring for nav links.

Screenshot: updated navigation

Copy link
Copy Markdown
Contributor

@yi-nuo426 yi-nuo426 left a comment

Choose a reason for hiding this comment

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

@copilot don't change this website's colors.

Copilot AI and others added 2 commits April 15, 2026 22:46
Agent-Logs-Url: https://github.com/meshery/qa/sessions/88b6f120-ebb7-43e1-9562-33fe68161293

Co-authored-by: yi-nuo426 <218099172+yi-nuo426@users.noreply.github.com>
Agent-Logs-Url: https://github.com/meshery/qa/sessions/88b6f120-ebb7-43e1-9562-33fe68161293

Co-authored-by: yi-nuo426 <218099172+yi-nuo426@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Apr 15, 2026

@copilot apply changes based on the comments in this thread

Addressed in eb9e08e.

The shared nav now keeps the existing site/report palette instead of introducing a new accent-colored active state, while the earlier review-thread fixes remain in place.

Screenshot: updated shared navigation

@copilot don't change this website's colors.

Addressed in eb9e08e.

The shared nav no longer changes the website’s colors; the active tab now uses the existing neutral page tones instead of an accent fill.

Screenshot: updated shared navigation

Copilot AI requested a review from yi-nuo426 April 15, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace existing navigation menu

7 participants