Replace per-report menu with shared cross-dashboard navigation#52
Replace per-report menu with shared cross-dashboard navigation#52
Conversation
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>
There was a problem hiding this comment.
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.mesheryReportNavinto 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.
| 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`; | ||
| } |
There was a problem hiding this comment.
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.
| function createNavDataScript(currentPageDir, navEntries) { | ||
| const navData = navEntries.map(({ label, relativeDir, reportPageDir }) => ({ | ||
| id: relativeDir || 'home', |
There was a problem hiding this comment.
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.
| 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), |
| 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; | ||
| } |
There was a problem hiding this comment.
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.
| .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; |
There was a problem hiding this comment.
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.
| .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 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>
Addressed in Applied the review-thread fixes: escaped the injected nav JSON before embedding it in the inline script, removed the unused nav item |
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>
Addressed in 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.
Addressed in 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. |



Uh oh!
There was an error while loading. Please reload this page.